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

[David Blaikie via cfe-dev]

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.

>  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
> > > > >
> > > > >
> > > > >