[DebugInfo] Add source attributes for function declaration on behalf of owner CU

[evgeny777]

Summary

There seems to be a bug in DwarfUnit which one can easily step on when building boost examples with full LTO.
It causes template function declaration inside a class to have invalid DW_AT_decl_file attribute and GNU
objdump and addr2line tools to report multiple failures while processing such executables.

Assume we have following C++ structure:

struct S {
  int foo;
  template< class T > T squared(T value) { return value * value; }
};

Even though this structure contains two elements, debug information for it will contain only one,
because squared may not be used anywhere in the program and so may not even be compiled.

!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, ....)
!18 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "S", ...., elements !19, ....)
!19 = !{!20}
!20 = !DIDerivedType(tag: DW_TAG_member, name: "foo", ....)

If some CU in a program is using S::squared two records will be added to debug information: definition and declaration
The latter will reference struct S via scope field:

!37 = distinct !DISubprogram(name: "squared<int>", unit !2, declaration !38, .....)
!38 = distinct !DISubprogram(name: "squared<int>", scope !18, ....)

The struct S could be used in multiple CUs, some of them may use squared and some may not. When full LTO is used
IRMover will combine debug information from all CUs merging the types and DIE corresponding to declaration of squared
my be processed by DwarfUnit instance corresponding to CU#1 and DIE corresponding to its scope (struct S) may be owned
by CU#2. In such case if we add source line attributes for declaration on behalf of CU#1 we'll likely break the debug info,
because file ids in CU#1 and CU#2 don't match.

This patch fixes the issue by adding debug info for declaration on behalf of owning CU.

[dblaikie]

I think the point where we lookup the other CU and cross over to it should probably be earlier - imagine if all attributes could potentially be CU-local (eg: we could produce a separate string_offsets section for each CU and so have separate strx indexes for each CU) & we should probably have a solution that would be correct in that situation. Does that make sense?

Also, it's helpful to include example source code in the IR file to demonstrate roughly what the LLVM IR metadata is representing/how it was produced in case it needs to be updated at a later date (it's a bit hard to manually edit the LLVM IR).

& FWIW there are 3 cases I know of where this kind of lazy-attachment occurs:

1. Member function template specializations
2. Nested classes
3. Implicit special members (copy ctor, dtor, etc)

it /might/ be easier to demonstrate with one of the other two? But it'll be easier for me to understand once I see the source/reproduction steps written into the test case & I might be able to suggest simplifications along those lines.

[evgeny777]

I think the point where we lookup the other CU and cross over to it should probably be earlier - imagine if all attributes could potentially be CU-local (eg: we could produce a separate string_offsets section for each CU and so have separate strx indexes for each CU) & we should probably have a solution that would be correct in that situation. Does that make sense?

This probably makes sense, I moved this lookup earlier, so now all subprogram attributes are applied on behalf of owner unit. This should address the problem you described.

Also, it's helpful to include example source code in the IR file to demonstrate roughly what the LLVM IR metadata is representing/how it was produced in case it needs to be updated at a later date (it's a bit hard to manually edit the LLVM IR).

I've added it as comments to IR file and also uploading source archive in case you want to try it yourself

dist.tbz2602 BDownload

[dblaikie]

test/DebugInfo/X86/template_function_decl.ll

12 ↗

(On Diff #188104)

Could probably remove the parameter and logic in this (just "template<typename T> void templ() { }")

30 ↗

(On Diff #188104)

What purpose does S2 serve in this test case?

[evgeny777]

test/DebugInfo/X86/template_function_decl.ll

30 ↗

(On Diff #188104)

I need inc2.h to be in debugging information. Including it before inc1.h in bar.cpp causes inc1.h to have identifier 3 in bar.cpp CU. Such identifier doesn't exist in foo.cpp, that's why you get DW_AT_decl_file (0x3) in llvm-dwarfdump output

[evgeny777]

Shortened test case a little

[dblaikie]

Here's a bit of a smaller test that might make it clearer (could add some comments to help explain why each part is present):

$ cat inc1.h
struct S {
  template <typename T> static void tmpfn() {}
};
$ cat inc2.h
extern int x;
int x;
$ cat foo.cpp
#include "inc1.h"
S s;
blaikie@blaikie-linux2:~/dev/scratch$ cat bar.cpp
#include "inc1.h"
#include "inc2.h"
void f3() { S::tmpfn<int>(); }

inc2.h in bar.cpp is, as you said, to increment the file number there so the file numbers in each CU don't line up coincidentally (it could contain any entity - an int seems the simplest)

Made the member function static and void - less DWARF to be emitted that way - no need for variables, etc.

I'll test a couple of related cases (implicit special members and nested classes) here in a moment & see if they have any related bugs.

[dblaikie]

I'll test a couple of related cases (implicit special members and nested classes) here in a moment & see if they have any related bugs.

Yep, if you add a nested type you can see a similar bug:

$ cat inc1.h
struct S {
  struct Nested {};
};
$ cat bar.cpp
#include "inc1.h"
#include "inc2.h"
void f3() {
  S::Nested n;
}
$ clang++-tot -emit-llvm foo.cpp bar.cpp -g -c && ~/dev/llvm/build/default/bin/llvm-link foo.bc bar.bc -o foobar.bc && clang++-tot foobar.bc -c && llvm-dwarfdump-tot -v -debug-info foobar.o | grep "strp.*\"Nested\"" -A2 -B2
0x00000051:     DW_TAG_structure_type [6]  
                DW_AT_calling_convention [DW_FORM_data1]      (DW_CC_pass_by_value)
                DW_AT_name [DW_FORM_strp]     ( .debug_str[0x000000a7] = "Nested")
                DW_AT_byte_size [DW_FORM_data1]       (0x01)
                DW_AT_decl_file [DW_FORM_data1]       (0x03)

Probably to be fixed by similarly switching over to the appropriate unit just before "createAndAddDIE" in "getOrCreateTypeDIE" (maybe splitting off the tail of getOrCreateTypeDIE into an implementation function that can be called here). Does that make sense?

llvm/trunk/include/llvm/CodeGen/DIE.h

public:
   /// this DIE hasn't been added to a unit DIE.
   const DIE *getUnitDie() const;
 
   /// Climb up the parent chain to get the compile unit or type unit that this
   /// DIE belongs to.
   ///
   /// \returns the DIEUnit that represents the compile or type unit that owns
   /// this DIE, or NULL if this DIE hasn't been added to a unit DIE.
-  const DIEUnit *getUnit() const;
+  DIEUnit *getUnit() const;
 
   void setOffset(unsigned O) { Offset = O; }
   void setSize(unsigned S) { Size = S; }
 
   /// Add a child to the DIE.
   DIE &addChild(DIE *Child) {
     assert(!Child->getParent() && "Child should be orphaned");
     Child->Owner = this;

llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp

while (p) {
     if (p->getTag() == dwarf::DW_TAG_compile_unit ||
         p->getTag() == dwarf::DW_TAG_type_unit)
       return p;
     p = p->getParent();
   }
   return nullptr;
 }
 
-const DIEUnit *DIE::getUnit() const {
+DIEUnit *DIE::getUnit() const {
   const DIE *UnitDie = getUnitDie();
   if (UnitDie)
     return UnitDie->Owner.dyn_cast<DIEUnit*>();
   return nullptr;
 }
 
 DIEValue DIE::findAttribute(dwarf::Attribute Attribute) const {
   // Iterate through all the attributes until we find the one we're

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal) {
   // DW_TAG_inlined_subroutine may refer to this DIE.
   DIE &SPDie = createAndAddDIE(dwarf::DW_TAG_subprogram, *ContextDIE, SP);
 
   // Stop here and fill this in later, depending on whether or not this
   // subprogram turns out to have inlined instances or not.
   if (SP->isDefinition())
     return &SPDie;
 
-  applySubprogramAttributes(SP, SPDie);
+  static_cast<DwarfUnit *>(SPDie.getUnit())
+      ->applySubprogramAttributes(SP, SPDie);
   return &SPDie;
 }
 
 bool DwarfUnit::applySubprogramDefinitionAttributes(const DISubprogram *SP,
                                                     DIE &SPDie) {
   DIE *DeclDie = nullptr;
   StringRef DeclLinkageName;
   if (auto *SPDecl = SP->getDeclaration()) {

llvm/trunk/test/DebugInfo/X86/template_function_decl.ll

+; Checks that DW_AT_decl_file attribute is correctly set for
+; template function declaration. When using full LTO we likely
+; end up having multiple CUs in a single DWARF file, and it is
+; possible that we add template function declaration DIE from
+; CU#1 to structure type DIE from CU#2. In such case we should
+; add source line attributes on behalf of CU#2, not CU#1
+
+; Example source code used to generate this sample (C++)
+; // File inc1.h
+; struct S {
+;   template <typename T> static void tmpfn() {}
+; };
+;
+; // File: inc2.h
+; extern int x;
+; int x;
+;
+; // File: foo.cpp
+; #include "inc1.h"
+; S s;
+;
+; // File: bar.cpp
+; #include "inc1.h"
+; #include "inc2.h"
+; void f3() { S::tmpfn<int>(); }
+
+; RUN: llc -filetype=obj %s -o %t
+; RUN: llvm-dwarfdump %t -o - | FileCheck %s
+
+; CHECK:       DW_AT_linkage_name ("_ZN1S5tmpfnIiEEvv")
+; CHECK-NEXT:  DW_AT_name ("tmpfn<int>")
+; CHECK-NEXT:  DW_AT_decl_file ("{{.*}}inc1.h")
+; CHECK-NEXT:  DW_AT_decl_line (2)
+
+source_filename = "ld-temp.o"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.S = type { i8 }
+
+$_ZN1S5tmpfnIiEEvv = comdat any
+
+@s = dso_local global %struct.S zeroinitializer, align 1, !dbg !0
+@x = dso_local global i32 0, align 4, !dbg !8
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @_Z2f3v() !dbg !21 {
+  call void @_ZN1S5tmpfnIiEEvv(), !dbg !24
+  ret void, !dbg !25
+}
+
+; Function Attrs: noinline nounwind optnone uwtable
+define weak_odr dso_local void @_ZN1S5tmpfnIiEEvv() comdat align 2 !dbg !26 {
+  ret void, !dbg !30
+}
+
+!llvm.dbg.cu = !{!2, !10}
+!llvm.ident = !{!15, !15}
+!llvm.module.flags = !{!16, !17, !18, !19, !20}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "s", scope: !2, file: !3, line: 2, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 9.0.0 (trunk 354767)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None)
+!3 = !DIFile(filename: "foo.cpp", directory: "/home/evgeny/work/cpp_lexer/sample2")
+!4 = !{}
+!5 = !{!0}
+!6 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S", file: !7, line: 1, size: 8, flags: DIFlagTypePassByValue | DIFlagTrivial, elements: !4, identifier: "_ZTS1S")
+!7 = !DIFile(filename: "./inc1.h", directory: "/home/evgeny/work/cpp_lexer/sample2")
+!8 = !DIGlobalVariableExpression(var: !9, expr: !DIExpression())
+!9 = distinct !DIGlobalVariable(name: "x", scope: !10, file: !13, line: 2, type: !14, isLocal: false, isDefinition: true)
+!10 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !11, producer: "clang version 9.0.0 (trunk 354767)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !12, nameTableKind: None)
+!11 = !DIFile(filename: "bar.cpp", directory: "/home/evgeny/work/cpp_lexer/sample2")
+!12 = !{!8}
+!13 = !DIFile(filename: "./inc2.h", directory: "/home/evgeny/work/cpp_lexer/sample2")
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!15 = !{!"clang version 9.0.0 (trunk 354767)"}
+!16 = !{i32 2, !"Dwarf Version", i32 4}
+!17 = !{i32 2, !"Debug Info Version", i32 3}
+!18 = !{i32 1, !"wchar_size", i32 4}
+!19 = !{i32 1, !"ThinLTO", i32 0}
+!20 = !{i32 1, !"EnableSplitLTOUnit", i32 0}
+!21 = distinct !DISubprogram(name: "f3", linkageName: "_Z2f3v", scope: !11, file: !11, line: 3, type: !22, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !10, retainedNodes: !4)
+!22 = !DISubroutineType(types: !23)
+!23 = !{null}
+!24 = !DILocation(line: 3, column: 13, scope: !21)
+!25 = !DILocation(line: 3, column: 30, scope: !21)
+!26 = distinct !DISubprogram(name: "tmpfn<int>", linkageName: "_ZN1S5tmpfnIiEEvv", scope: !6, file: !7, line: 2, type: !22, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !10, templateParams: !28, declaration: !27, retainedNodes: !4)
+!27 = !DISubprogram(name: "tmpfn<int>", linkageName: "_ZN1S5tmpfnIiEEvv", scope: !6, file: !7, line: 2, type: !22, scopeLine: 2, flags: DIFlagPrototyped | DIFlagStaticMember, spFlags: 0, templateParams: !28)
+!28 = !{!29}
+!29 = !DITemplateTypeParameter(name: "T", type: !14)
+!30 = !DILocation(line: 2, column: 46, scope: !26)
+