Page MenuHomePhabricator

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

Authored by evgeny777 on Feb 21 2019, 11:59 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

evgeny777 created this revision.Feb 21 2019, 11:59 PM
asl added a subscriber: asl.Feb 22 2019, 12:01 AM

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.

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

dblaikie added inline comments.Feb 25 2019, 5:14 PM
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 marked an inline comment as done.Feb 25 2019, 10:16 PM
evgeny777 added inline comments.
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 updated this revision to Diff 188340.Feb 26 2019, 4:19 AM

Shortened test case a little

dblaikie accepted this revision.Feb 26 2019, 4:16 PM

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.

This revision is now accepted and ready to land.Feb 26 2019, 4:16 PM

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?

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2019, 6:46 AM