llvm dev - Nov 2021 - [DebugInfo] Bugs when emitting debug info with inlined functions

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/20211014/790eb176/attachment-0001.html>

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>

I have another way to fix these bugs in https://reviews.llvm.org/D112337
and it is much simpler. Could someone please take a look?

On Fri, Oct 15, 2021 at 6:59 PM Ellis Hoag <ellis.sparky.hoag at
gmail.com>
wrote:
> 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/20211101/87cd2386/attachment.html>