<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 6, 2016, at 10:27 AM, Eric Christopher <<a href="mailto:echristo@gmail.com" class="">echristo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Jan 6, 2016 at 10:04 AM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">+aprantl<br class=""><br class="">> On 2016-Jan-06, at 09:46, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank" class="">tejohnson@google.com</a>> wrote:<br class="">><br class="">> +dexonsmith,echristo,blaikie who are way more familiar with debug info than I<br class="">> +pcc who also found this and contacted me off-list with a possible solution<br class="">><br class="">> See also<span class="Apple-converted-space"> </span><a href="https://llvm.org/bugs/show_bug.cgi?id=26037" rel="noreferrer" target="_blank" class="">https://llvm.org/bugs/show_bug.cgi?id=26037</a><span class="Apple-converted-space"> </span>that I updated<br class="">> with a test case and more detailed analysis.<br class="">><br class="">> The issue is essentially that we no longer pull in a DISubprogram on<br class="">> an LTO link with this patch because the associated function was not<br class="">> linked in. However, it had been referenced by a DIImportedEntity which<br class="">> is leaving behind a DIImportedEntity with no entity:<br class="">><br class="">> !11 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !8, line: 2)<br class="">><br class="">> which in turn causes an assert during debug generation. The 3<br class="">> solutions I mention there are:<br class="">><br class="">> 1) Change findNeededSubprograms to look at entities and conservatively<br class="">> remove any subprograms reached from there from the UnneededSubprograms<br class="">> list<br class="">> 2) Remove any DIImportedEntity that no longer have an entity<br class="">> 3) Change the debug generation code to handle DIImportedEntity with a<br class="">> null entity<br class="">><br class="">> pcc has a tentative solution that implements 3) above.<br class="">><br class="">> Debug info experts - what is the right thing to do here? Is it legal<br class="">> to have a DIImportedEntity with no entity? If so, pcc's solution<br class="">> (solution 3 above) is the right one. Otherwise I should implement 1)<br class="">> or 2).<br class=""><br class="">My intuition says that (2) is the correct solution, but Adrian would know best.<br class=""><br class=""></blockquote><div class=""><br class=""></div><div class="">It's a bit iffy:</div><div class=""><br class=""></div><div class="">3.2.3 Imported (or Renamed) Declaration Entries Some languages support the concept of importing into or making accessible in a given unit declarations made in a different module or scope. An imported declaration may sometimes be given another name. An imported declaration is represented by one or more debugging information entries with the tag DW_TAG_imported_declaration. When an overloaded entity is imported, there is one imported declaration entry for each overloading.<br class=""></div><div class=""><br class=""></div><div class="">which means that we're probably looking at 1 as we care about declarations for entities rather than defining declarations. That said, unless we change the metadata to be a declaration of a subprogram rather than a defining declaration we're going to have a bad time further down the line.</div></div></div></div></blockquote><div><br class=""></div>Right. Dropping the imported entity because it’s not in the current translation unit would change the overload resolution in the debugger. <br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">In the meantime, if (3) isn't too intrusive, it would be worth doing temporarily to unbreak the bot...<br class=""></blockquote><div class=""><br class=""></div><div class="">This is absolutely ok as long as it's temporary while we figure this out.</div></div></div></div></blockquote><div><br class=""></div>Agreed.<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""><br class=""></div><div class="">-eric</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">><br class="">> Thanks,<br class="">> Teresa<br class="">><br class="">><br class="">> On Tue, Jan 5, 2016 at 4:24 PM, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com" target="_blank" class="">ahmed.bougacha@gmail.com</a>> wrote:<br class="">>> On Fri, Dec 18, 2015 at 9:51 AM, Teresa Johnson via llvm-commits<br class="">>> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">>>> Author: tejohnson<br class="">>>> Date: Fri Dec 18 11:51:37 2015<br class="">>>> New Revision: 256003<br class="">>>><br class="">>>> URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=256003&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=256003&view=rev</a><br class="">>>> Log:<br class="">>>> [ThinLTO/LTO] Don't link in unneeded metadata<br class="">>>><br class="">>>> Summary:<br class="">>>> Third patch split out from<span class="Apple-converted-space"> </span><a href="http://reviews.llvm.org/D14752" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D14752</a>.<br class="">>>><br class="">>>> Only map in needed DISubroutine metadata (imported or otherwise linked<br class="">>>> in functions and other DISubroutine referenced by inlined instructions).<br class="">>>> This is supported for ThinLTO, LTO and llvm-link --only-needed, with<br class="">>>> associated tests for each one.<br class="">>>><br class="">>>> Depends on D14838.<br class="">>>><br class="">>>> Reviewers: dexonsmith, joker.eph<br class="">>>><br class="">>>> Subscribers: davidxl, llvm-commits, joker.eph<br class="">>>><br class="">>>> Differential Revision:<span class="Apple-converted-space"> </span><a href="http://reviews.llvm.org/D14843" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D14843</a><br class="">>>><br class="">>>> Added:<br class="">>>> llvm/trunk/test/Linker/Inputs/only-needed-debug-metadata.ll<br class="">>>> llvm/trunk/test/Linker/only-needed-debug-metadata.ll<br class="">>>> Modified:<br class="">>>> llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h<br class="">>>> llvm/trunk/lib/Linker/IRMover.cpp<br class="">>>> llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp<br class="">>>> llvm/trunk/test/Linker/thinlto_funcimport_debug.ll<br class="">>>> llvm/trunk/test/tools/gold/X86/Inputs/linkonce-weak.ll<br class="">>>> llvm/trunk/test/tools/gold/X86/linkonce-weak.ll<br class="">>>><br class="">>>> Modified: llvm/trunk/lib/Linker/IRMover.cpp<br class="">>>> URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=256003&r1=256002&r2=256003&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=256003&r1=256002&r2=256003&view=diff</a><br class="">>>> ==============================================================================<br class="">>>> --- llvm/trunk/lib/Linker/IRMover.cpp (original)<br class="">>>> +++ llvm/trunk/lib/Linker/IRMover.cpp Fri Dec 18 11:51:37 2015<br class="">>>> @@ -487,6 +495,16 @@ class IRLinker {<br class="">>>><br class="">>>> void linkNamedMDNodes();<br class="">>>><br class="">>>> + /// Populate the UnneededSubprograms set with the DISubprogram metadata<br class="">>>> + /// from the source module that we don't need to link into the dest module,<br class="">>>> + /// because the functions were not imported directly or via an inlined body<br class="">>>> + /// in an imported function.<br class="">>>> + void findNeededSubprograms(ValueToValueMapTy &ValueMap);<br class="">>><br class="">>> Hey Teresa,<br class="">>><br class="">>> This has been causing bootstrap assertion failures, I think because a<br class="">>> DISubprogram referenced by DIImportedEntity should also be considered<br class="">>> needed.<br class="">>><br class="">>> I filed PR26037, could you please have a look?<br class="">>><br class="">>> Thanks!<br class="">>> -Ahmed<br class="">><br class="">><br class="">><br class="">> --<br class="">> Teresa Johnson | Software Engineer |<span class="Apple-converted-space"> </span><a href="mailto:tejohnson@google.com" target="_blank" class="">tejohnson@google.com</a><span class="Apple-converted-space"> </span>| 408-460-2413</blockquote></div></div></div></blockquote></div><br class=""></body></html>