[llvm-dev] [DebugInfo] Bugs when emitting debug info with inlined functions
Ellis Hoag via llvm-dev
llvm-dev at lists.llvm.org
Fri Oct 15 18:59:35 PDT 2021
I've just uploaded https://reviews.llvm.org/D111916. I haven't seen any
problems so far and it seems to solve all the bugs I am aware of. Let me
know what you think.
Ellis
On Thu, Oct 14, 2021 at 2:26 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Thu, Oct 14, 2021 at 2:19 PM Ellis Hoag <ellis.sparky.hoag at gmail.com>
> wrote:
>
>> Sure, I'll try it out!
>>
>> I'm not sure if we will need to remove attributes? We just need to move
>> children from the SP DIE and possibly remove the whole SP DIE if it's empty.
>>
>
> I don't think we'll need to remove attributes - we'll keep doing the thing
> the code currently does, delaying adding the name/file/line attributes to
> the SP DIE until the end of the module then deciding where to put them
> based on whether there's an abstract origin or not.
>
>
>>
>> On Thu, Oct 14, 2021 at 2:13 PM David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> Could be worth a shot - I think the attribute removal would be more
>>> difficult due to abbreviations, etc (maybe the same is true of children,
>>> when setting the CHILDREN property of the abbrev, I'm not sure) - worth a
>>> go, at least. Can write down why it's not feasible if you/we discover some
>>> reason that's infeasible.
>>>
>>> On Thu, Oct 14, 2021 at 2:12 PM Ellis Hoag <ellis.sparky.hoag at gmail.com>
>>> wrote:
>>>
>>>> /maybe/ though I'm not sure the DIE APIs support removing children, do
>>>>> they?
>>>>>
>>>>
>>>> No the API doesn't support removing children or changing the parent
>>>> node. I don't see anything obvious reason why we can't add this API, but it
>>>> might be too disruptive for what we are trying to accomplish.
>>>>
>>>> Ellis
>>>>
>>>> On Thu, Oct 14, 2021 at 11:49 AM David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, Oct 14, 2021 at 10:30 AM Ellis Hoag <
>>>>> ellis.sparky.hoag at gmail.com> wrote:
>>>>>
>>>>>> For these other cases (variables, imported declarations, maybe
>>>>>>> function local types too?) - I think, at least consistent with the current
>>>>>>> design, we have to defer producing them until we get to the end - the same
>>>>>>> as for the name/decl file/line attributes, etc.
>>>>>>
>>>>>>
>>>>>> Instead of deferring those DIEs, I wonder if we can do some kind of
>>>>>> fixup. At the end of the module, we can search through the SPs with
>>>>>> abstract origins to see if they have a concrete DIE with children
>>>>>> (variables, types, imported declarations). Then move those DIEs to the
>>>>>> abstract origin. This would be a simple solution because we can just assume
>>>>>> the concrete function will exist and never be inlined, but at the end we
>>>>>> fix the inlined cases. I suspect this solution won't work for the same
>>>>>> reason we don't want to create DIEs without parents, but let me know what
>>>>>> you think of this.
>>>>>>
>>>>>
>>>>> /maybe/ though I'm not sure the DIE APIs support removing children, do
>>>>> they?
>>>>>
>>>>>
>>>>>> Function local types /probably/ have a similar problem. Maybe
>>>>>>> call_site tags too? (since they reference function declarations to say
>>>>>>> which function the call_site is calling) Perhaps some other stuff - but
>>>>>>> those are ones that come to my mind at least.
>>>>>>>
>>>>>>
>>>>>> I just tested function local types and they do have the same problem.
>>>>>>
>>>>>> __attribute__((always_inline)) inline
>>>>>> int foo() { struct S {int a;}; S s; return s.a; }
>>>>>> int bar() { return 1 + foo(); }
>>>>>>
>>>>>
>>>>> Thanks, good to confirm!
>>>>>
>>>>>
>>>>>>
>>>>>> I don't think call site tags have the problem because if they exist
>>>>>> then the function was not inlined in that location so there must be a
>>>>>> concrete definition.
>>>>>>
>>>>>
>>>>> Fair point!
>>>>>
>>>>> - Dave
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks, Ellis
>>>>>>
>>>>>> On Wed, Oct 13, 2021 at 8:29 PM David Blaikie <dblaikie at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> On Wed, Oct 13, 2021 at 4:20 PM Ellis Hoag via llvm-dev <
>>>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> In recent weeks I've been looking into fixing a few bugs in the
>>>>>>>> Dwarf emitted by LLVM related to when functions are inlined.
>>>>>>>> * https://bugs.llvm.org/show_bug.cgi?id=30637 (static variables)
>>>>>>>> * Fixed in https://reviews.llvm.org/D108492 (in review)
>>>>>>>> * https://bugs.llvm.org/show_bug.cgi?id=52159 (imported
>>>>>>>> declaration)
>>>>>>>> * Fixed in https://reviews.llvm.org/D110294 (in review)
>>>>>>>> In these bugs `getOrCreateSubprogramDIE()` is used to find the SP
>>>>>>>> DIE that will become the parent of a GV DIE or the reference of an imported
>>>>>>>> declaration DIE. The problem is if the function is inlined and removed, the
>>>>>>>> concrete SP DIE will not be created and `getOrCreateSubprogramDIE()` will
>>>>>>>> return an empty DIE. Instead, I think we should be using the abstract
>>>>>>>> origin DIE of the SP which is created after processing an inlined scope.
>>>>>>>>
>>>>>>>> Here is a concrete example from
>>>>>>>> https://bugs.llvm.org/show_bug.cgi?id=52159
>>>>>>>> ```
>>>>>>>> namespace ns {
>>>>>>>> inline __attribute__((always_inline))
>>>>>>>> void foo() { int a = 4; }
>>>>>>>> }
>>>>>>>>
>>>>>>>> void goo() {
>>>>>>>> using ns::foo;
>>>>>>>> foo();
>>>>>>>> }
>>>>>>>> ```
>>>>>>>>
>>>>>>>> This produces an imported declaration DIE that references an empty
>>>>>>>> SP DIE even though there already exists one for foo.
>>>>>>>> ```
>>>>>>>> // Abstract origin
>>>>>>>> 0x0000002f: DW_TAG_subprogram
>>>>>>>> DW_AT_linkage_name ("_ZN2ns3fooEv")
>>>>>>>> DW_AT_name ("foo")
>>>>>>>> // Empty concrete
>>>>>>>> 0x00000047: DW_TAG_subprogram
>>>>>>>> 0x00000048: NULL
>>>>>>>> // Import declaration
>>>>>>>> 0x00000069: DW_TAG_imported_declaration
>>>>>>>> DW_AT_import (0x00000047)
>>>>>>>> ```
>>>>>>>>
>>>>>>>> One fix is to reference the abstract origin DIE.
>>>>>>>> ```
>>>>>>>> 0x00000069: DW_TAG_imported_declaration
>>>>>>>> DW_AT_import (0x0000002f)
>>>>>>>> ```
>>>>>>>>
>>>>>>>> Another fix is to do what gcc does and fill out a specification DIE
>>>>>>>> that the import references.
>>>>>>>> ```
>>>>>>>> // Import declaration
>>>>>>>> 0x0000004f: DW_TAG_imported_declaration
>>>>>>>> DW_AT_import (0x00000063)
>>>>>>>> // Specification
>>>>>>>> 0x00000063: DW_TAG_subprogram
>>>>>>>> DW_AT_name ("foo")
>>>>>>>> DW_AT_declaration (true)
>>>>>>>> // Abstract origin
>>>>>>>> 0x00000070: DW_TAG_subprogram
>>>>>>>> DW_AT_specification (0x00000063 "_ZN2ns3fooEv")
>>>>>>>> ```
>>>>>>>>
>>>>>>>> Since I'm relatively new to debug info, I thought I'd ask some
>>>>>>>> questions on the mailing list.
>>>>>>>> 1. A simple solution to this class of bugs is to reference the
>>>>>>>> abstract origin SP DIE where appropriate. Do we have any rules for when to
>>>>>>>> reference the abstract origin if it exists?
>>>>>>>>
>>>>>>>
>>>>>>> That's the difficult problem here - the abstract origin only exists
>>>>>>> if there's at least one instance of the function being inlined - and we
>>>>>>> don't know that until we've processed all the functions. The way this
>>>>>>> was/is currently implemented is to create the concrete definition
>>>>>>> subprogram when we see the concrete function definition, but not fill out
>>>>>>> the attributes that would be inherited from an abstract origin if there was
>>>>>>> one - then wait until we get to the end of the module and if we've created
>>>>>>> an abstract origin (because an inlined instance was found/produced), then
>>>>>>> we add the abstract_origin attribute to the concrete definition subprogram,
>>>>>>> and if we haven't created an abstract origin (because there were no inlined
>>>>>>> instances) then we put the attributes (name, decl file/line/etc) on the
>>>>>>> concrete definition without creating an abstract origin.
>>>>>>>
>>>>>>> For these other cases (variables, imported declarations, maybe
>>>>>>> function local types too?) - I think, at least consistent with the current
>>>>>>> design, we have to defer producing them until we get to the end - the same
>>>>>>> as for the name/decl file/line attributes, etc.
>>>>>>>
>>>>>>> Though deferring them presents other challenges - we can't reference
>>>>>>> them if they're deferred, so if some other debug info like the type of a
>>>>>>> function parameter has to be produced, how do we produce that if we have
>>>>>>> deferred the type production?
>>>>>>>
>>>>>>> We can't/shouldn't/it's difficult to create the DIEs but to defer
>>>>>>> attaching those DIEs to the DIE tree - because there's some logic that uses
>>>>>>> the DIE parent chain/which CU a DIE is in to determine certain issues of
>>>>>>> encoding (whether CU-local references can be used or the like). Maybe we
>>>>>>> can get rid of that constraint (at which point we could create DIEs but
>>>>>>> defer adding them to the DIE tree) but if I recall correctly it's pretty
>>>>>>> deeply embedded/important.
>>>>>>>
>>>>>>>
>>>>>>>> 2. Would it be better to follow gcc's solution and fill out
>>>>>>>> specifications in these cases?
>>>>>>>>
>>>>>>>
>>>>>>> I guess that doesn't address the function-local static variable
>>>>>>> case, or the case of an imported declaration being inside a subprogram? (we
>>>>>>> could in theory put those in a declaration DIE, but it'd probably be extra
>>>>>>> confusing to consumers)
>>>>>>>
>>>>>>>
>>>>>>>> 3. Are they more known cases/bugs to consider?
>>>>>>>>
>>>>>>>
>>>>>>> Function local types /probably/ have a similar problem. Maybe
>>>>>>> call_site tags too? (since they reference function declarations to say
>>>>>>> which function the call_site is calling) Perhaps some other stuff - but
>>>>>>> those are ones that come to my mind at least.
>>>>>>>
>>>>>>> Thanks for looking into this - sorry it's all a bit thorny, though.
>>>>>>>
>>>>>>> - Dave
>>>>>>>
>>>>>>>
>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211015/b112000c/attachment.html>
More information about the llvm-dev
mailing list