<div dir="ltr">I have another way to fix these bugs in <a href="https://reviews.llvm.org/D112337">https://reviews.llvm.org/D112337</a> and it is much simpler. Could someone please take a look?<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 15, 2021 at 6:59 PM Ellis Hoag <<a href="mailto:ellis.sparky.hoag@gmail.com">ellis.sparky.hoag@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>I've just uploaded <a href="https://reviews.llvm.org/D111916" target="_blank">https://reviews.llvm.org/D111916</a>. 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.<br></div><div><br></div><div>Ellis<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 2:26 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 2:19 PM Ellis Hoag <<a href="mailto:ellis.sparky.hoag@gmail.com" target="_blank">ellis.sparky.hoag@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Sure, I'll try it out!</div><div><br></div><div>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.<br></div></div></blockquote><div><br>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.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 2:13 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">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.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 2:12 PM Ellis Hoag <<a href="mailto:ellis.sparky.hoag@gmail.com" target="_blank">ellis.sparky.hoag@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>/maybe/ though I'm not sure the DIE APIs support removing children, do they?</div></blockquote><div><br></div><div>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.</div><div><br></div><div>Ellis<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 11:49 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 10:30 AM Ellis Hoag <<a href="mailto:ellis.sparky.hoag@gmail.com" target="_blank">ellis.sparky.hoag@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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.</blockquote><div><br></div><div>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.</div></div></blockquote><div><br></div><div>/maybe/ though I'm not sure the DIE APIs support removing children, do they?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>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.</div></blockquote><div><br></div><div>I just tested function local types and they do have the same problem.</div><div><br></div><div><span style="font-family:monospace">__attribute__((always_inline)) inline<br>int foo() { struct S {int a;}; S s; return s.a; }<br>int bar() { return 1 + foo(); }</span></div></div></blockquote><div><br>Thanks, good to confirm!<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><span style="font-family:monospace"> </span></div><div><br></div><div>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.</div></div></blockquote><div><br></div><div>Fair point!<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div> <br></div><div><br></div><div>Thanks, Ellis</div><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 13, 2021 at 8:29 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Wed, Oct 13, 2021 at 4:20 PM Ellis Hoag via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi all,<br><br>In recent weeks I've been looking into fixing a few bugs in the Dwarf emitted by LLVM related to when functions are inlined.<br>* <a href="https://bugs.llvm.org/show_bug.cgi?id=30637" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=30637</a> (static variables)<br> * Fixed in <a href="https://reviews.llvm.org/D108492" target="_blank">https://reviews.llvm.org/D108492</a> (in review)<br>* <a href="https://bugs.llvm.org/show_bug.cgi?id=52159" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=52159</a> (imported declaration)<br> * Fixed in <a href="https://reviews.llvm.org/D110294" target="_blank">https://reviews.llvm.org/D110294</a> (in review)<br>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.<br><br>Here is a concrete example from <a href="https://bugs.llvm.org/show_bug.cgi?id=52159" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=52159</a><br>```<br>namespace ns {<br>inline __attribute__((always_inline))<br>void foo() { int a = 4; }<br>}<br><br>void goo() {<br> using ns::foo;<br> foo();<br>}<br>```<br><br>This produces an imported declaration DIE that references an empty SP DIE even though there already exists one for foo.<br>```<br>// Abstract origin<br>0x0000002f: DW_TAG_subprogram<br> DW_AT_linkage_name ("_ZN2ns3fooEv")<br> DW_AT_name ("foo")<br>// Empty concrete <br>0x00000047: DW_TAG_subprogram<br>0x00000048: NULL<br>// Import declaration<br>0x00000069: DW_TAG_imported_declaration<br> DW_AT_import (0x00000047)<br>```<br><br>One fix is to reference the abstract origin DIE.<br>```<br>0x00000069: DW_TAG_imported_declaration<br> DW_AT_import (0x0000002f)<br>```<br><br>Another fix is to do what gcc does and fill out a specification DIE that the import references.<br>```<br>// Import declaration<br>0x0000004f: DW_TAG_imported_declaration<br> DW_AT_import (0x00000063)<br>// Specification <br>0x00000063: DW_TAG_subprogram<br> DW_AT_name ("foo")<br> DW_AT_declaration (true)<br>// Abstract origin <br>0x00000070: DW_TAG_subprogram<br> DW_AT_specification (0x00000063 "_ZN2ns3fooEv")<br>```<br><br>Since I'm relatively new to debug info, I thought I'd ask some questions on the mailing list.<br>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?<br></div></blockquote><div><br>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.<br><br>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.<br><br>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?<br><br>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.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">2. Would it be better to follow gcc's solution and fill out specifications in these cases?<br></div></blockquote><div><br>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)<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">3. Are they more known cases/bugs to consider?</div></blockquote><div><br>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.<br><br>Thanks for looking into this - sorry it's all a bit thorny, though.<br><br>- Dave<br> </div></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div>
</blockquote></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div>