[dsymutil] Skip DW_AT_sibling attributes.

[JDevlieghere]

Summary

Following DW_AT_sibling attributes completely defeats the pruning pass.
Although we don't generate the DW_AT_sibling attribute we should still
handle it correctly.

This is also the reason I don't have a test case for this, I'm not sure how we
can test this without checking in a non-clang generated binary.

[probinson]

I'm not really familiar with dsymutil's use cases; does it operate on .o files? If so, maybe you can generate a test with gcc -g -S and reduce the resulting assembler by hand? If the assembler isn't ridiculously long.

[aprantl]

I dusted off my PowerBook G4 and compiled

struct A { int a; } a;
struct B { int b; } b;

which outputs:

 File: sibling.o (ppc)
----------------------------------------------------------------------
.debug_info contents:

0x00000000: Compile Unit: length = 0x000000ba  version = 0x0002  abbr_offset = 0x00000000  addr_size = 0x04  (next CU at 0x000000be)

0x0000000b: TAG_compile_unit [1] *
             AT_producer( "GNU C 4.0.1 (Apple Computer, Inc. build 5370)" )
             AT_language( DW_LANG_C89 )
             AT_name( "/tmp/sibling.c" )
             AT_low_pc( 0x00000000 )
             AT_high_pc( 0x00000000 )
             AT_stmt_list( 0x00000000 )

0x00000056:     TAG_structure_type [2]  
                 AT_name( "A" )
                 AT_byte_size( 0x00 )
                 AT_decl_file( "/tmp/sibling.c" )
                 AT_decl_line( 1 )

0x0000005c:     TAG_structure_type [2]  
                 AT_name( "B" )
                 AT_byte_size( 0x00 )
                 AT_decl_file( "/tmp/sibling.c" )
                 AT_decl_line( 3 )

0x00000062:     TAG_array_type [3] *
                 AT_type( {0x0000006d} ( int ) )
                 AT_sibling( {0x0000006d} )

0x0000006b:         TAG_subrange_type [4]  

0x0000006c:         NULL

0x0000006d:     TAG_base_type [5]  
                 AT_byte_size( 0x04 )
                 AT_encoding( DW_ATE_signed )
                 AT_name( "int" )

0x00000074:     TAG_variable [6]  
                 AT_name( "__CFConstantStringClassReference" )
                 AT_type( {0x00000062} ( int[] ) )
                 AT_external( 0x01 )
                 AT_artificial( 0x01 )
                 AT_declaration( 0x01 )

0x0000009d:     TAG_variable [7]  
                 AT_name( "A" )
                 AT_decl_file( "/tmp/sibling.c" )
                 AT_decl_line( 1 )
                 AT_type( {0x00000056} ( A ) )
                 AT_external( 0x01 )
                 AT_location( [0x00000000] )

0x000000ad:     TAG_variable [7]  
                 AT_name( "B" )
                 AT_decl_file( "/tmp/sibling.c" )
                 AT_decl_line( 3 )
                 AT_type( {0x0000005c} ( B ) )
                 AT_external( 0x01 )
                 AT_location( [0x00000000] )

0x000000bd:     NULL
...

Would that work as a testcase? Do you need it to be i386 instead?

[JDevlieghere]

In D43439#1017376, @aprantl wrote:

Would that work as a testcase? Do you need it to be i386 instead?

Thanks Adrian, I think this should be fine!

[JDevlieghere]

Thanks to Adrian's sample I found out that we already handle this case correctly, because DW_AT_sibling has an FC_Reference class form value. Hence, the additional check is redundant.

I'll still commit the test case, once D43760 has been landed.

[JDevlieghere]

Forget what I said before, I had misread part of the code and the other test only succeeded accidentally.

Added proper test (indeed, it fails without this change)

Thanks Adrian for compiling this!

[davide]

LGTM modulo comment. Adrian?

llvm/test/tools/dsymutil/PowerPC/sibling.test

4 ↗

(On Diff #136079)

Can you also please mention the compiler we're using to build & the cmdline?

[aprantl]

llvm/test/tools/dsymutil/PowerPC/sibling.test

4 ↗

(On Diff #136079)

That would be Apple-GCC 4.0.1 (build 5370). The command line was g++ -g -c sibling.cpp -o sibling.o

llvm/trunk/test/tools/dsymutil/PowerPC/sibling.test

 # Verify that we don't follow DW_AT_sibling references.
 #
 # Source:
-#   struct A { int a; } a;
-#   struct B { int b; } b;
+#   struct A { };
+#   struct B { };
+#
+#   struct C {
+#     A a;
+#   };
+#
+#   B b;
+#   C c;
+#
+# Compiled with Apple-GCC 4.0.1 (build 5370):
+#   g++ -g -c sibling.cpp -o sibling.o
 
 # RUN: llvm-dsymutil -arch ppc -f -oso-prepend-path=%p/../Inputs/ -y %s -o - | llvm-dwarfdump -debug-info - | FileCheck %s
 
-# CHECK: DW_TAG_variable
-# CHECK-NEXT: DW_AT_name ("a")
-# CHECK-NOT: DW_AT_name ("b")
+# CHECK: DW_TAG_structure_type
+# CHECK-NEXT: DW_AT_name  ("A")
+# CHECK: DW_TAG_structure_type
+# CHECK-NEXT: DW_AT_name  ("C")
+
+# CHECK-NOT: DW_AT_name ("B")
 
 ---
 triple:          'ppc-apple-darwin'
 objects:
   - filename:        sibling.o
     timestamp:       1518197670
     symbols:
-      - { sym: _a, objAddr: 0x00000000000000cb, binAddr: 0x0000000100000FA0, size: 0x00000010 }
+      - { sym: _c, objAddr: 0x000000000000017e, binAddr: 0x0000000100000FA0, size: 0x00000010 }
 ...

llvm/trunk/tools/dsymutil/DwarfLinker.cpp

void DwarfLinker::keepDIEAndDependencies(RelocationManager &RelocMgr,
   DWARFDataExtractor Data = Unit.getDebugInfoExtractor();
   const auto *Abbrev = Die.getAbbreviationDeclarationPtr();
   uint32_t Offset = Die.getOffset() + getULEB128Size(Abbrev->getCode());
 
   // Mark all DIEs referenced through attributes as kept.
   for (const auto &AttrSpec : Abbrev->attributes()) {
     DWARFFormValue Val(AttrSpec.Form);
 
-    if (!Val.isFormClass(DWARFFormValue::FC_Reference)) {
+    if (!Val.isFormClass(DWARFFormValue::FC_Reference) ||
+        AttrSpec.Attr == dwarf::DW_AT_sibling) {
       DWARFFormValue::skipValue(AttrSpec.Form, Data, &Offset,
                                 Unit.getFormParams());
       continue;
     }
 
     Val.extractValue(Data, &Offset, Unit.getFormParams(), &Unit);
     CompileUnit *ReferencedCU;
     if (auto RefDie =
unsigned DwarfLinker::DIECloner::cloneDieReferenceAttribute(
   DIE *NewRefDie = nullptr;
   CompileUnit *RefUnit = nullptr;
   DeclContext *Ctxt = nullptr;
 
   DWARFDie RefDie =
       resolveDIEReference(Linker, CompileUnits, Val, U, InputDIE, RefUnit);
 
   // If the referenced DIE is not found,  drop the attribute.
-  if (!RefDie)
+  if (!RefDie || AttrSpec.Attr == dwarf::DW_AT_sibling)
     return 0;
 
   unsigned Idx = RefUnit->getOrigUnit().getDIEIndex(RefDie);
   CompileUnit::DIEInfo &RefInfo = RefUnit->getInfo(Idx);
 
   // If we already have emitted an equivalent DeclContext, just point
   // at it.
   if (isODRAttribute(AttrSpec.Attr)) {