r184276 - Debug Info: support for gdwarf-2 gdwarf-3 gdwarf-4

[David Blaikie]

On Wed, Jul 3, 2013 at 5:31 PM, Eric Christopher <echristo at gmail.com> wrote:
> On Wed, Jul 3, 2013 at 5:24 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Jul 2, 2013 5:40 PM, "Eric Christopher" <echristo at gmail.com> wrote:
>>>
>>> I see you went ahead and committed these (the mailing list is down,
>>> but I saw the updates via phabricator).
>>>
>>> The discussion may have faltered, but given that there was
>>> disagreement on the original patch I would think that it's your
>>> responsibility to ping the thread before committing.
>>>
>>> I've spoken with Dan again and we've chatted about the module flag
>>> versus in the compile unit and the rationale for the direction you've
>>> chosen. Overall while I think it should be the other way
>>
>> Could we try to capture some/more of the rationale in both directions
>> actually in email rather than (or at least in addition to) offline
>> conversations so we can refer to it later. As it stands it seems were still
>> lacking any of that conversation in an archived form.
>
> *nod*
>
> Basically to summarize it's pretty much what you said above: when
> we're linking, we a) have full control over what we emit, and b) it's
> a specialized enough use case that following what most debug info does
> isn't as compelling an argument. I do think being able to limit
> features on a per-cu basis is worth it, but at this point it's
> probably a fine point rather than absolute correctness.

I'm curious what the value in this might be/what you have in mind -
how/when/where might a per-cu choice of DWARF version support be used?

>
>>
>>> it's not
>>> overly damaging to llvm for it to be the direction that you've already
>>> written the code and can always be changed later if we find it too
>>> limiting.
>>>
>>> That said, I do believe that the warning should go away and that you
>>> should fix the code (I guess during module linking since you've gone
>>> this direction) so that it sorts out the values and re-sets the module
>>> flag to the minimum of all modules.
>>
>> What's the reasoning for this choice?it would seem to me that the maximum
>> would make a fair bit of sense, though I admit it does seem somewhat
>> arbitrary either way.
>>
>
> The idea is that whomever decided to limit their debug info to, say,
> dwarf-2 should win overall because they theoretically know better
> (since they specified an option)

But all the dwarf version selections might have been specified by
options, no? Where does the assumption come from that the lowest
version is the only explicit one?

The only actual use-case/example I remember being given was from Dan,
I think, where someone might've shipped an LTO-able static library (a
bunch of bitcode) with debug info (thus a predefined/fixed debug info
version in that module) & then a user works day-to-day linking that
with their client code & perhaps they sometimes need no debug info,
sometimes dwarf-2, sometimes dwarf-4, etc (or there are multiple
clients, some who always use 2, etc).

In that case neither min nor max is going to capture the scenario
correctly - I suppose maybe anyone shipping such a lib should be
advised to always ship it with the max dwarf version for the version
of the compiler (since this bitcode library would be precisely version
locked to the compiler anyway, due to the lack of debug info
versioning) in which case 'min' would be correct, yes. Is this what
you had in mind? Except that the default debug info version isn't
'max' at the moment (at least not on darwin) so it seems easy for
someone to ship this with the 'default' version but for some clients
to want the 'max' version.

Just to dredge up some more of the in-person conversation from months
ago that I barely remember - the alternative was to have an argument
to the linker, yes? (can't remember if you spoke in favor of that at
some point or not... ) which seems ugly because a non-LTO linker
wouldn't need this information & it'd be a "value add" that's not
strictly necessary (in a non-LTO case you'd get whatever DWARF version
you built each object file with separately, none of them would be
upgraded or downgraded).

Yeah, so I can see trying to implicitly merge dwarf version choice
might hurt users in either direction. At least preserving the version
per-CU makes LTO strictly "no worse". The only other way to make LTO
strictly no-worse (& I think /better/) would be to have a flag passed
to the linker. (this is not meant to be an authoritative observation -
I'm totally open to being corrected on any of this if I've
misunderstood something or come to a bogus conclusion)

> what the capabilities of the system
> are. i.e. It's the conservative choice to make.
>
> -eric
>
>>> This will mean that a number of
>>> more advanced features may get turned off, but it's the conservative
>>> answer.
>>>
>>> -eric
>>>
>>>
>>>
>>> On Thu, Jun 20, 2013 at 4:48 PM, Manman Ren <mren at apple.com> wrote:
>>> >
>>> > On Jun 19, 2013, at 4:28 PM, Eric Christopher wrote:
>>> >
>>> >>>> I don't think a module flag is the way we want to go about this, I
>>> >>>> think it should be an attribute on the compile unit created.
>>> >>>
>>> >>> Hmm, I could've sworn we (Eric, Dan, and myself) in person a couple of
>>> >>> weeks ago & decided that a module flag was the way to go - but I'm not
>>> >>> 100% sure that's how the conversation concluded & I'm also not sure I
>>> >>> can reconstruct all the arguments.
>>> >>>
>>> >>> I think it went something like: we'll need the module-level
>>> >>> functionality anyway, and it'll be able to report
>>> >>> errors/incompatibilities in a uniform way that users will be used to,
>>> >>> whereas if we did this down in debug info we wouldn't be able to use
>>> >>> such error reporting if there are "incompatible" dwarf version
>>> >>> requests. (I suppose there might not be - in which case it'd be "min"
>>> >>> or "max" essentially and even then, presumably, having the common
>>> >>> module functionality able to handle that for us seems reasonable/nice)
>>> >>>
>>> >>
>>> >> Right, but that pretty much contradicts how linkers deal with them
>>> >> today - it's perfectly compatible to link them without warning.
>>> >>
>>> >>>> Few constraints there:
>>> >>>>
>>> >>>> a) this means that we just check the compile unit as we create up the
>>> >>>> DIE and we don't have to check the module.
>>> >>>
>>> >>> I'm not sure that's a huge change - and we'd still have to have some
>>> >>> code resolve the difference between different modules & presumably
>>> >>> rewrite the compile unit of each one to bring it up to whatever the
>>> >>> One True dwarf version is that we'll be using for this build, no? Or
>>> >>> are you expecting to generate some DWARF-N for some compile units in
>>> >>> the same compilation and DWARF-M for others?
>>> >>
>>> >> Only at LTO time and only up to a minimum of 3. Otherwise, I'm
>>> >> perfectly fine with generating differing values of dwarf for each
>>> >> compile unit.
>>> >>
>>> >>> I'm not sure how we settled on this issue - whether the module
>>> >>> handling should just bump everything up to the "minimum compatible"
>>> >>> version (floor of DWARF-4 or whatever, otherwise the max of all the
>>> >>> specified versions from the different CUs).
>>> >>>
>>> >>>> c) at LTO time we'll likely want to constrain to a minimum dwarf
>>> >>>> version that contains ref addr so that we can minimize, but this is
>>> >>>> by
>>> >>>> no means required,
>>> >>>
>>> >>> Fair enough, so maybe the 'floor' above isn't required.
>>> >>>
>>> >>
>>> >> *nod* Basically the way we're doing LTO is going to require
>>> >> inter-compile unit references. Other than that there's no real need to
>>> >> do any munging of the value at all. Hence me figuring to just leave it
>>> >> on each compile unit and keep the logic on what to emit in the compile
>>> >> unit itself.
>>> >>
>>> >> It probably comes down to simple preference really there's nothing
>>> >> compelling about either way unless we want to explicitly warn people
>>> >> and that'd be creating a new warning for "little" reason and it might
>>> >> be surprising for people that were previously getting no warnings with
>>> >> normal compilation moving to LTO. That said, maybe they'll be
>>> >> surprised when they get inter-compile unit references.
>>> > Any conclusion here? I am okay with either choice, and about to modify
>>> > the backend.
>>> >
>>> > Manman
>>> >
>>> >>
>>> >> *shrug*
>>> >>
>>> >> -eric
>>> >>
>>> >>
>>> >>>> d) it matches how linkers actually link debug information
>>> >>>>
>>> >>>> Also:
>>> >>>>
>>> >>>>> +  else if (Opts.getDebugInfo() != CodeGenOptions::NoDebugInfo)
>>> >>>>> +    // Default Dwarf version is 3 if we are generating debug
>>> >>>>> information.
>>> >>>>> +    Opts.DwarfVersion = 3;
>>> >>>>>
>>> >>>>
>>> >>>> We want 4 here I believe, or even a level for "latest" with all of
>>> >>>> the
>>> >>>> bells and whistles. 4 is a likely sufficient start though since we'd
>>> >>>> have to define what "all the bells and whistles" means :) Or, as an
>>> >>>> alternate question, "why 3?"
>>> >>>>
>>> >>>> Thanks!
>>> >>>>
>>> >>>> -eric
>>> >>>> _______________________________________________
>>> >>>> cfe-commits mailing list
>>> >>>> cfe-commits at cs.uiuc.edu
>>> >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> >