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

Wed Jan 6 10:38:29 PST 2016

> On Jan 6, 2016, at 10:27 AM, Eric Christopher <echristo at gmail.com> wrote:
> 
> 
> 
> On Wed, Jan 6, 2016 at 10:04 AM Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
> +aprantl
> 
> > On 2016-Jan-06, at 09:46, Teresa Johnson <tejohnson at google.com <mailto:tejohnson at google.com>> wrote:
> >
> > +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 <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).
> 
> My intuition says that (2) is the correct solution, but Adrian would know best.
> 
> 
> It's a bit iffy:
> 
> 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.
> 
> 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.

Right. Dropping the imported entity because it’s not in the current translation unit would change the overload resolution in the debugger. 
>  
> In the meantime, if (3) isn't too intrusive, it would be worth doing temporarily to unbreak the bot...
> 
> This is absolutely ok as long as it's temporary while we figure this out.

Agreed.
> 
> -eric
>  
> 
> >
> > Thanks,
> > Teresa
> >
> >
> > On Tue, Jan 5, 2016 at 4:24 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com <mailto: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 <mailto: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 <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 <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 <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 <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 <mailto:tejohnson at google.com> | 408-460-2413

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160106/e5778a77/attachment.html>