[cfe-dev] Make standalone-debug default based on glldb tuning

[David Blaikie via cfe-dev]

On Fri, Apr 12, 2019 at 11:05 AM <paul.robinson at sony.com> wrote:
>
>
>
> > -----Original Message-----
> > From: David Blaikie [mailto:dblaikie at gmail.com]
> > Sent: Friday, April 12, 2019 1:40 PM
> > To: Robinson, Paul
> > Cc: Adrian Prantl; Clang Dev; Douglas Katzman
> > Subject: Re: Make standalone-debug default based on glldb tuning
> >
> > On Fri, Apr 12, 2019 at 10:30 AM <paul.robinson at sony.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: David Blaikie [mailto:dblaikie at gmail.com]
> > > > Sent: Friday, April 12, 2019 12:43 PM
> > > > To: Robinson, Paul
> > > > Cc: Adrian Prantl; Clang Dev; Douglas Katzman
> > > > Subject: Re: Make standalone-debug default based on glldb tuning
> > > >
> > > > On Fri, Apr 12, 2019 at 9:38 AM <paul.robinson at sony.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: David Blaikie [mailto:dblaikie at gmail.com]
> > > > > > Sent: Friday, April 12, 2019 12:06 PM
> > > > > > To: Robinson, Paul
> > > > > > Cc: Adrian Prantl; Clang Dev; Douglas Katzman
> > > > > > Subject: Re: Make standalone-debug default based on glldb tuning
> > > > > >
> > > > > > On Fri, Apr 12, 2019 at 8:45 AM <paul.robinson at sony.com> wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: aprantl at apple.com [mailto:aprantl at apple.com]
> > > > > > > > Sent: Friday, April 12, 2019 11:21 AM
> > > > > > > > To: David Blaikie
> > > > > > > > Cc: Clang Dev; Robinson, Paul; Douglas Katzman
> > > > > > > > Subject: Re: Make standalone-debug default based on glldb
> > tuning
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > On Apr 12, 2019, at 8:18 AM, David Blaikie
> > <dblaikie at gmail.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > I realized recently (as folks at Google have started
> > > > experimenting
> > > > > > > > > with LLDB) that the platform default for debugger tuning and
> > the
> > > > > > > > > default for -fstandalone-debug are both independent. This
> > seems
> > > > a
> > > > > > bit
> > > > > > > > > wrong to me because enabling -glldb doesn't enable -
> > fstandalone-
> > > > > > debug.
> > > > > > > > >
> > > > > > > > > I'm wondering if everyone's on board with removing
> > > > > > > > > ToolChain::GetDefaultStandaloneDebug, and using
> > > > > > > > > ToolChain::getDefaultDebuggerTuning() to select the default
> > > > > > standalone
> > > > > > > > > debug state?
> > > > > > > > >
> > > > > > > > > Then on the command line, I'd kind of expect one to override
> > the
> > > > > > > > > other, so things like "-glldb -fno-standalone-debug" is
> > > > respected
> > > > > > > > > (when it comes to hopefully fixing LLDB to cope with
> > standalone
> > > > > > debug
> > > > > > > > > input).
> > > > > > > > >
> > > > > > > > > Sound good to everyone? If so I'll make a patch & send it
> > for
> > > > > > review.
> > > > > > > > > (well, I'll probably work on the patch now anyway)
> > > > > > > >
> > > > > > > > Since this doesn't change anything for platforms that default
> > to -
> > > > > > glldb,
> > > > > > > > I'm fine with this change.
> > > > > > > >
> > > > > > > > -- adrian
> > > > > > >
> > > > > > > I'm somewhat surprised LLDB would want -fstandalone-debug,
> > > > independent
> > > > > > > of the target.  It would be a source of unnecessary bloat on
> > ELF-
> > > > using
> > > > > > > targets, no?
> > > > > >
> > > > > > LLDB fundamentally (at the moment) doesn't support -fstandalone-
> > debug,
> > > > > > no matter the file format.
> > > > > >
> > > > > > As far as I understand it: LLDB constructs ASTs from DWARF on a
> > > > > > per-binary level (so, .so or executable). -fstandalone-debug
> > creates
> > > > > > "impossible" DWARF (eg: if one base class's vtable (& thus DWARF
> > > > > > definition) is in one .so, but a derived class is in another .so -
> > > > > > then the DWARF in the second .so contains a definition that
> > derives
> > > > > > from a declaration (& the DWARF consumer would have to go look in
> > the
> > > > > > other .so to stitch it up to a definition).
> > > > >
> > > > > Uh. Am I misunderstanding? Somehow I thought -fstandalone-debug
> > meant
> > > > > "full" which means the base class ought not to be a declaration in
> > > > > the second .so?  Or is that pruning not based on the full/limited
> > > > > distinction?
> > > >
> > > > Sorry, I misspoke - the default ("no standalone debug") creates the
> > > > DWARF that doesn't correspond to source in that you can see classes
> > > > derived from declarations and the like. LLDB won't go searching for a
> > > > definition that matches the declaration outside the scope of a single
> > > > blob of DWARF (either a single object or single dsym, I guess).
> > > > Turning on standalone debug avoids LLDB encountering these
> > > > "impossible" situations.
> > >
> >
> > (bonus data: oh, apparently FreeBSD still defaults to GDB tuning - but
> > also want to opt-in to -fstandalone-debug so FreeBSD binaries are
> > usable with LLDB out of the box, even if it's not the default - and
> > they mention in ToolChains/FreeBSD.h that 'dtrace' has some issues
> > with this kind of DWARF too - non-specific as to what those issues
> > are)
> >
> > > Okay then! So the story is, LLDB doesn't want to chase after things
> > > in other objects, which on Darwin is either expensive (in a .o case)
> > > or moot (when dsymutil has done it all ahead of time).  Which leaves
> > > me wondering about the ELF case; LLDB also doesn't want to troll
> > > through other CUs even when they are in the same executable?
> >
> > As I gave in the example/Adrian reiterated - it's cross-file (so
> > crossing a binary boundary, from one .so to another, for instance -
> > where the DWARF isn't merged across that boundary) rather than
> > cross-object, that's the issue.
>
> Building everything -fstandalone-debug still feels like it would be
> a big size hit to compensate for this, unless you also use type units.
> Which have their own size issues... but if LLDB is resisting the
> cross-loadfile fix, I guess it's a relatively small change to the
> compiler and users have the full set of options to undo it if they
> prefer.

Oh, sure, it's a huge hit - but it reflects the reality today &
decisions that've been made for many years now.

Fundamental differences of opinion... for a while LLDB would suggest
this was a bug in the compiler and then improved somewhat:
http://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20150921/023143.html

Somewhere between Adrian's (no one wrote a patch) & your perspective.
Yes, LLDB developers were pretty resistant to the idea that this was
even a bug in the debugger & a non-issue for a few reasons on MacOS
(the feature wasn't usable because platform libraries didn't ship with
debug info, so code using those libraries couldn't be built
non-standalone) - so they're certainly not working to fix this any
time soon & no one else has found it to be a priority. It'll probably
be a priority for Google in the near future - but that's still a fair
bit of work to do (there is no simple patch/prototype here - I don't
know how much work it will be, but I assume non-trivial to get the
desired behavior & preserve performance, etc).

- Dave


> --paulr
>
> >
> > >  That
> > > strikes me as a real inadequacy in LLDB, not something that the
> > > compiler should compensate for. Especially once you upgrade to v5
> > > and get the spiffy new index (thanks to Pavel).
> >
> > I still think it's a significant inadequacy in LLDB & mentioned this
> > at the time it was discovered/worked around. But it's stuck around for
> > a while and means using -fno-standalone-debug with LLDB is not
> > practical on any platform. So tuning for LLDB should imply
> > -fstandalone-debug, which it currently doesn't.
> >
> > (that said, tuning for other debuggers shouldn't turn it off if a user
> > opted into it (eg: "-fstandalone-debug -ggdb" should still produce
> > standalone debug info) because there are other reasons (as the name
> > implies - if you're really only building a subset of objects with
> > debug info) you might have enabled it, so -ggdb shouldn't override
> > that choice)
> >
> > So my current proposed patch is quite small:
> >
> > -  bool NeedFullDebug = Args.hasFlag(options::OPT_fstandalone_debug,
> > -                                    options::OPT_fno_standalone_debug,
> > -                                    TC.GetDefaultStandaloneDebug());
> > +  bool NeedFullDebug = Args.hasFlag(
> > +      options::OPT_fstandalone_debug, options::OPT_fno_standalone_debug,
> > +      DebuggerTuning == llvm::DebuggerKind::LLDB ||
> > +          TC.GetDefaultStandaloneDebug());
> >
> > > So I'm unconvinced that the basis of the -fstandalone-debug default
> > > should be changed.
> >
> > Seems reasonable to change the default when -glldb is specified, given
> > LLDB can't cope with that DWARF (& has made that an explicit choice
> > for years now). I still hope the bug will be addressed (likely as
> > Google starts to look at transitioning to LLDB but not wanting to take
> > the object size hit of disabling -fstandalone-debug).
> >
> > > FTR, I completely get the scenario Adrian describes where some
> > > library has no debug info and the normal (limited) case fails to
> > > provide a full definition. We've had complaints about not being
> > > able to debug some STL-derived classes.
> >
> > Have you considered shipping STL binaries with debug info? (that's
> > what I use on Ubuntu - there's separate -dbg variant packages for
> > libstdc++ that include the DWARF for the library) - GCC has the same
> > issue, though in a lesser form (it's not as aggressive as LLVM is -
> > for LLVM, it'll home/assume std::basic_string<char> (using an explicit
> > instantiation declaration/definition) is available elsewhere, GCC
> > won't - but both will assume that std::basic_ifstream<char> is 'homed'
> > (using the vtable location)).
> >
> > > Licensees are not too
> > > happy about the extra size with the full-info workaround.
> > > --paulr
> > >
> > > >
> > > > >
> > > > > >
> > > > > > (FreeBSD defaults to -fstandalone-debug because it defaults to
> > LLDB as
> > > > > > the debugger - Adrian's reply makes me wonder if that's clear, so
> > > > > > figured I'd mention that)
> > > > > >
> > > > > > > --paulr
> > > > > > >
> > > > > > >
> > > > > > >