Debug info questy (was Re: [llvm] r256003 - [ThinLTO/LTO] Don't link in unneeded metadata)

[Teresa Johnson via llvm-commits]

+dexonsmith,echristo,blaikie who are way more familiar with debug info than I
+pcc who also found this and contacted me off-list with a possible solution

See also https://llvm.org/bugs/show_bug.cgi?id=26037 that I updated
with a test case and more detailed analysis.

The issue is essentially that we no longer pull in a DISubprogram on
an LTO link with this patch because the associated function was not
linked in. However, it had been referenced by a DIImportedEntity which
is leaving behind a DIImportedEntity with no entity:

!11 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !8, line: 2)

which in turn causes an assert during debug generation. The 3
solutions I mention there are:

1) Change findNeededSubprograms to look at entities and conservatively
remove any subprograms reached from there from the UnneededSubprograms
list
2) Remove any DIImportedEntity that no longer have an entity
3) Change the debug generation code to handle DIImportedEntity with a
null entity

pcc has a tentative solution that implements 3) above.

Debug info experts - what is the right thing to do here? Is it legal
to have a DIImportedEntity with no entity? If so, pcc's solution
(solution 3 above) is the right one. Otherwise I should implement 1)
or 2).

Thanks,
Teresa


On Tue, Jan 5, 2016 at 4:24 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
> On Fri, Dec 18, 2015 at 9:51 AM, Teresa Johnson via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: tejohnson
>> Date: Fri Dec 18 11:51:37 2015
>> New Revision: 256003
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=256003&view=rev
>> Log:
>> [ThinLTO/LTO] Don't link in unneeded metadata
>>
>> Summary:
>> Third patch split out from http://reviews.llvm.org/D14752.
>>
>> Only map in needed DISubroutine metadata (imported or otherwise linked
>> in functions and other DISubroutine referenced by inlined instructions).
>> This is supported for ThinLTO, LTO and llvm-link --only-needed, with
>> associated tests for each one.
>>
>> Depends on D14838.
>>
>> Reviewers: dexonsmith, joker.eph
>>
>> Subscribers: davidxl, llvm-commits, joker.eph
>>
>> Differential Revision: http://reviews.llvm.org/D14843
>>
>> Added:
>>     llvm/trunk/test/Linker/Inputs/only-needed-debug-metadata.ll
>>     llvm/trunk/test/Linker/only-needed-debug-metadata.ll
>> Modified:
>>     llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h
>>     llvm/trunk/lib/Linker/IRMover.cpp
>>     llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
>>     llvm/trunk/test/Linker/thinlto_funcimport_debug.ll
>>     llvm/trunk/test/tools/gold/X86/Inputs/linkonce-weak.ll
>>     llvm/trunk/test/tools/gold/X86/linkonce-weak.ll
>>
>> Modified: llvm/trunk/lib/Linker/IRMover.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=256003&r1=256002&r2=256003&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Linker/IRMover.cpp (original)
>> +++ llvm/trunk/lib/Linker/IRMover.cpp Fri Dec 18 11:51:37 2015
>> @@ -487,6 +495,16 @@ class IRLinker {
>>
>>    void linkNamedMDNodes();
>>
>> +  /// Populate the UnneededSubprograms set with the DISubprogram metadata
>> +  /// from the source module that we don't need to link into the dest module,
>> +  /// because the functions were not imported directly or via an inlined body
>> +  /// in an imported function.
>> +  void findNeededSubprograms(ValueToValueMapTy &ValueMap);
>
> Hey Teresa,
>
> This has been causing bootstrap assertion failures, I think because a
> DISubprogram referenced by DIImportedEntity should also be considered
> needed.
>
> I filed PR26037, could you please have a look?
>
> Thanks!
> -Ahmed



-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413