[dsymutil] Discard useless location attributes.

When cloning the debug info for a function that hasn't been linked,
strip the DIEs from all location attributes that wouldn't contain any
meaningful information anyway.

This kind of situation can happen when a function got discarded by the
linker, but its debug information is still wanted in the final link
because it was marked as required as some other DIE dependency. The easiest
way to get into that situation is to have using directives. They get
linked unconditionally, but their targets might not always be present.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@247386 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/test/tools/dsymutil/Inputs/dead-stripped/1.o b/test/tools/dsymutil/Inputs/dead-stripped/1.o
new file mode 100644 (file)
index 0000000..fbdffbc
Binary files /dev/null and b/test/tools/dsymutil/Inputs/dead-stripped/1.o differ

diff --git a/test/tools/dsymutil/X86/dead-stripped.cpp b/test/tools/dsymutil/X86/dead-stripped.cpp
new file mode 100644 (file)
index 0000000..ecab758
--- /dev/null
+++ b/test/tools/dsymutil/X86/dead-stripped.cpp
@@ -0,0 +1,48 @@
+// RUN: llvm-dsymutil -f -y %p/dummy-debug-map.map -oso-prepend-path %p/../Inputs/dead-stripped -o - | llvm-dwarfdump - -debug-dump=info | FileCheck %s
+
+// The test was compiled with:
+// clang++ -O2 -g -c dead-strip.cpp -o 1.o
+
+// The goal of the test is to exercise dsymutil's behavior in presence of
+// functions/variables that have been dead-stripped by the linker but
+// that are still present in the linked debug info (in this case because
+// they have been DW_TAG_import'd in another namespace).
+
+// Everything in the N namespace bellow doesn't have a debug map entry, and
+// thus is considered dead (::foo() has a debug map entry, otherwse dsymutil
+// would just drop the CU altogether).
+
+namespace N {
+int blah = 42;
+// This is actually a dsymutil-classic bug that we reproduced
+// CHECK: DW_TAG_variable
+// CHECK-NOT: DW_TAG
+// CHECK: DW_AT_location
+
+__attribute__((always_inline)) int foo() { return blah; }
+// CHECK: DW_TAG_subprogram
+// CHECK-NOT: DW_AT_low_pc
+// CHECK-NOT: DW_AT_high_pc
+// CHECK: DW_AT_frame_base
+
+// CHECK: DW_TAG_subprogram
+
+int bar(unsigned i) {
+       int val = foo();
+       if (i)
+               return val + bar(i-1);
+       return foo();
+}
+// CHECK: DW_TAG_subprogram
+// CHECK-NOT: DW_AT_low_pc
+// CHECK-NOT: DW_AT_high_pc
+// CHECK: DW_AT_frame_base
+// CHECK-NOT: DW_AT_location
+// CHECK-NOT: DW_AT_low_pc
+// CHECK-NOT: DW_AT_high_pc
+
+}
+// CHECK: TAG_imported_module
+using namespace N;
+
+void foo() {}

diff --git a/tools/dsymutil/DwarfLinker.cpp b/tools/dsymutil/DwarfLinker.cpp
index 71d6ea6dd3a6bd00143380c235c456740245ffcb..619bbf923eaf6622f93dd37368cccf7748aca249 100644 (file)
--- a/tools/dsymutil/DwarfLinker.cpp
+++ b/tools/dsymutil/DwarfLinker.cpp
@@ -1159,6 +1159,7 @@ private:
     TF_DependencyWalk = 1 << 2,  ///< Walking the dependencies of a kept DIE.
     TF_ParentWalk = 1 << 3,      ///< Walking up the parents of a kept DIE.
     TF_ODR = 1 << 4,             ///< Use the ODR whhile keeping dependants.
+    TF_SkipPC = 1 << 5,          ///< Skip all location attributes.
   };
 
   /// \brief Mark the passed DIE as well as all the ones it depends on
@@ -1200,7 +1201,7 @@ private:
   ///
   /// \returns the root of the cloned tree.
   DIE *cloneDIE(const DWARFDebugInfoEntryMinimal &InputDIE, CompileUnit &U,
-                int64_t PCOffset, uint32_t OutOffset);
+                int64_t PCOffset, uint32_t OutOffset, unsigned Flags);
 
   typedef DWARFAbbreviationDeclaration::AttributeSpec AttributeSpec;
 
@@ -1263,6 +1264,9 @@ private:
   /// \brief Assign an abbreviation number to \p Abbrev
   void AssignAbbrev(DIEAbbrev &Abbrev);
 
+  /// Create a copy of abbreviation Abbrev.
+  void copyAbbrev(const DWARFAbbreviationDeclaration &Abbrev, bool hasODR);
+
   /// \brief FoldingSet that uniques the abbreviations.
   FoldingSet<DIEAbbrev> AbbreviationsSet;
   /// \brief Storage for the unique Abbreviations.
@@ -2338,6 +2342,7 @@ unsigned DwarfLinker::cloneScalarAttribute(
                    dwarf::Form(AttrSpec.Form), DIEInteger(Value));
   if (AttrSpec.Attr == dwarf::DW_AT_ranges)
     Unit.noteRangeAttribute(Die, Patch);
+
   // A more generic way to check for location attributes would be
   // nice, but it's very unlikely that any other attribute needs a
   // location list.
@@ -2476,6 +2481,30 @@ static bool isTypeTag(uint16_t Tag) {
   return false;
 }
 
+static bool
+shouldSkipAttribute(DWARFAbbreviationDeclaration::AttributeSpec AttrSpec,
+                    uint16_t Tag, bool InDebugMap, bool SkipPC,
+                    bool InFunctionScope) {
+  switch (AttrSpec.Attr) {
+  default:
+    return false;
+  case dwarf::DW_AT_low_pc:
+  case dwarf::DW_AT_high_pc:
+  case dwarf::DW_AT_ranges:
+    return SkipPC;
+  case dwarf::DW_AT_location:
+  case dwarf::DW_AT_frame_base:
+    // FIXME: for some reason dsymutil-classic keeps the location
+    // attributes when they are of block type (ie. not location
+    // lists). This is totally wrong for globals where we will keep a
+    // wrong address. It is mostly harmless for locals, but there is
+    // no point in keeping these anyway when the function wasn't linked.
+    return (SkipPC || (!InFunctionScope && Tag == dwarf::DW_TAG_variable &&
+                       !InDebugMap)) &&
+           !DWARFFormValue(AttrSpec.Form).isFormClass(DWARFFormValue::FC_Block);
+  }
+}
+
 /// \brief Recursively clone \p InputDIE's subtrees that have been
 /// selected to appear in the linked output.
 ///
@@ -2485,7 +2514,7 @@ static bool isTypeTag(uint16_t Tag) {
 /// \returns the cloned DIE object or null if nothing was selected.
 DIE *DwarfLinker::cloneDIE(const DWARFDebugInfoEntryMinimal &InputDIE,
                            CompileUnit &Unit, int64_t PCOffset,
-                           uint32_t OutOffset) {
+                           uint32_t OutOffset, unsigned Flags) {
   DWARFUnit &U = Unit.getOrigUnit();
   unsigned Idx = U.getDIEIndex(&InputDIE);
   CompileUnit::DIEInfo &Info = Unit.getInfo(Idx);
@@ -2551,7 +2580,27 @@ DIE *DwarfLinker::cloneDIE(const DWARFDebugInfoEntryMinimal &InputDIE,
     PCOffset = Info.AddrAdjust;
   AttrInfo.PCOffset = PCOffset;
 
+  if (Abbrev->getTag() == dwarf::DW_TAG_subprogram) {
+    Flags |= TF_InFunctionScope;
+    if (!Info.InDebugMap)
+      Flags |= TF_SkipPC;
+  }
+
+  bool Copied = false;
   for (const auto &AttrSpec : Abbrev->attributes()) {
+    if (shouldSkipAttribute(AttrSpec, Die->getTag(), Info.InDebugMap,
+                            Flags & TF_SkipPC, Flags & TF_InFunctionScope)) {
+      DWARFFormValue::skipValue(AttrSpec.Form, Data, &Offset, &U);
+      // FIXME: dsymutil-classic keeps the old abbreviation around
+      // even if it's not used. We can remove this (and the copyAbbrev
+      // helper) as soon as bit-for-bit compatibility is not a goal anymore.
+      if (!Copied) {
+        copyAbbrev(*InputDIE.getAbbreviationDeclarationPtr(), Unit.hasODR());
+        Copied = true;
+      }
+      continue;
+    }
+
     DWARFFormValue Val(AttrSpec.Form);
     uint32_t AttrSize = Offset;
     Val.extractValue(Data, &Offset, &U);
@@ -2603,7 +2652,7 @@ DIE *DwarfLinker::cloneDIE(const DWARFDebugInfoEntryMinimal &InputDIE,
   // Recursively clone children.
   for (auto *Child = InputDIE.getFirstChild(); Child && !Child->isNULL();
        Child = Child->getSibling()) {
-    if (DIE *Clone = cloneDIE(*Child, Unit, PCOffset, OutOffset)) {
+    if (DIE *Clone = cloneDIE(*Child, Unit, PCOffset, OutOffset, Flags)) {
       Die->addChild(Clone);
       OutOffset = Clone->getOffset() + Clone->getSize();
     }
@@ -2941,6 +2990,21 @@ void DwarfLinker::patchFrameInfoForObject(const DebugMapObject &DMO,
   }
 }
 
+void DwarfLinker::copyAbbrev(const DWARFAbbreviationDeclaration &Abbrev,
+                             bool hasODR) {
+  DIEAbbrev Copy(dwarf::Tag(Abbrev.getTag()),
+                 dwarf::Form(Abbrev.hasChildren()));
+
+  for (const auto &Attr : Abbrev.attributes()) {
+    uint16_t Form = Attr.Form;
+    if (hasODR && isODRAttribute(Attr.Attr))
+      Form = dwarf::DW_FORM_ref_addr;
+    Copy.AddAttribute(dwarf::Attribute(Attr.Attr), dwarf::Form(Form));
+  }
+
+  AssignAbbrev(Copy);
+}
+
 ErrorOr<const object::ObjectFile &>
 DwarfLinker::loadObject(BinaryHolder &BinaryHolder, DebugMapObject &Obj,
                         const DebugMap &Map) {
@@ -3020,7 +3084,7 @@ bool DwarfLinker::link(const DebugMap &Map) {
         const auto *InputDIE = CurrentUnit.getOrigUnit().getUnitDIE();
         CurrentUnit.setStartOffset(OutputDebugInfoSize);
         DIE *OutputDIE = cloneDIE(*InputDIE, CurrentUnit, 0 /* PCOffset */,
-                                  11 /* Unit Header size */);
+                                  11 /* Unit Header size */, /* Flags =*/0);
         CurrentUnit.setOutputUnitDIE(OutputDIE);
         OutputDebugInfoSize = CurrentUnit.computeNextUnitOffset();
         if (Options.NoOutput)