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

[via cfe-dev]

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