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

[Ahmed Bougacha via llvm-commits]

I'm sorry, but I spoke too soon!
It turns out, this fixes llc, but later in the build, other binaries
hit a similar problem, from elsewhere though:

    frame #9: 0x0000000102c07984
libLTO.dylib`llvm::DIScope::getRawFile(this=0x0000000000000000) const
+ 36 at DebugInfoMetadata.h:430
    frame #10: 0x0000000102c07875
libLTO.dylib`llvm::DIScope::getFile(this=0x0000000000000000) const +
21 at DebugInfoMetadata.h:415
    frame #11: 0x0000000102beba25
libLTO.dylib`llvm::DIScope::getFilename(this=0x0000000000000000) const
+ 21 at DebugInfoMetadata.h:503
    frame #12: 0x0000000103907f06
libLTO.dylib`llvm::DwarfCompileUnit::constructImportedEntityDIE(this=0x00000001e5847090,
Module=0x000000014fc5a5a8) + 614 at DwarfCompileUnit.cpp:659
    frame #13: 0x00000001039151cb
libLTO.dylib`llvm::DwarfDebug::constructAndAddImportedEntityDIE(this=0x00000001c8fcbc00,
TheCU=0x00000001e5847090, N=0x000000014fc5a5a8) + 91 at
DwarfDebug.cpp:456

Looks like it's the scope that's missing this time:

(lldb) f 12
frame #12: 0x0000000103907f06
libLTO.dylib`llvm::DwarfCompileUnit::constructImportedEntityDIE(this=0x00000001e5847090,
Module=0x000000014fc5a5a8) + 614 at DwarfCompileUnit.cpp:659
   656  else
   657    EntityDie = getDIE(Entity);
   658  assert(EntityDie);
-> 659  addSourceLine(*IMDie, Module->getLine(),
Module->getScope()->getFilename(),
   660                Module->getScope()->getDirectory());
   661  addDIEEntry(*IMDie, dwarf::DW_AT_import, *EntityDie);
   662  StringRef Name = Module->getName();
(lldb) p ::Module->dump()
<0x14fc5a5a8> = !DIImportedEntity(tag: DW_TAG_imported_module, scope:
null, entity: <0x154545c78>, line: 154)

Sorry about missing that!
-Ahmed


On Wed, Jan 6, 2016 at 4:11 PM, Teresa Johnson <tejohnson at google.com> wrote:
> Great, thanks for checking. Committed at r257000.
> Teresa
>
> On Wed, Jan 6, 2016 at 2:22 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
>> On Wed, Jan 6, 2016 at 1:33 PM, Teresa Johnson <tejohnson at google.com> wrote:
>>> Here is the change I made which fixes the test case. I can add some
>>> comments and a test case, but let me know if this looks ok otherwise.
>>> As Eric noted, the DISubroutine for a() is marked as a definition,
>>> which it isn't actually, but this was the behavior before this patch
>>> and seems to work ok.
>>
>> FWIW, this patch [0] fixes the issue [1] for me.
>>
>> Thanks for taking care of this Teresa!
>> -Ahmed
>>
>> [0] on r256768 and with a fix for the unrelated PR25919
>> [1] llc build, OS X, -O2 -flto -g
>>
>>> Thanks,
>>> Teresa
>>>
>>> diff --git a/lib/Linker/IRMover.cpp b/lib/Linker/IRMover.cpp
>>> index 309690f..9900ecc 100644
>>> --- a/lib/Linker/IRMover.cpp
>>> +++ b/lib/Linker/IRMover.cpp
>>> @@ -1211,6 +1211,12 @@ void
>>> IRLinker::findNeededSubprograms(ValueToValueMapTy &ValueMap) {
>>>    for (unsigned I = 0, E = CompileUnits->getNumOperands(); I != E; ++I) {
>>>      auto *CU = cast<DICompileUnit>(CompileUnits->getOperand(I));
>>>      assert(CU && "Expected valid compile unit");
>>> +    SmallPtrSet<DISubprogram *, 8> ImportedEntitySPs;
>>> +    for (auto *IE : CU->getImportedEntities()) {
>>> +      assert(IE->getEntity());
>>> +      if (auto *SP = dyn_cast<DISubprogram>(IE->getEntity()))
>>> +        ImportedEntitySPs.insert(SP);
>>> +    }
>>>      for (auto *Op : CU->getSubprograms()) {
>>>        // Unless we were doing function importing and deferred metadata linking,
>>>        // any needed SPs should have been mapped as they would be reached
>>> @@ -1218,7 +1224,7 @@ void
>>> IRLinker::findNeededSubprograms(ValueToValueMapTy &ValueMap) {
>>>        // function bodies, or from DILocation on inlined instructions).
>>>        assert(!(ValueMap.MD()[Op] && IsMetadataLinkingPostpass) &&
>>>               "DISubprogram shouldn't be mapped yet");
>>> -      if (!ValueMap.MD()[Op])
>>> +      if (!ValueMap.MD()[Op] && !ImportedEntitySPs.count(Op))
>>>          UnneededSubprograms.insert(Op);
>>>      }
>>>    }
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413