[llvm] r247386 - [dsymutil] Discard useless location attributes.
Frederic Riss via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 10 21:17:30 PDT 2015
Author: friss
Date: Thu Sep 10 23:17:30 2015
New Revision: 247386
URL: http://llvm.org/viewvc/llvm-project?rev=247386&view=rev
Log:
[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.
Added:
llvm/trunk/test/tools/dsymutil/Inputs/dead-stripped/
llvm/trunk/test/tools/dsymutil/Inputs/dead-stripped/1.o
llvm/trunk/test/tools/dsymutil/X86/dead-stripped.cpp
Modified:
llvm/trunk/tools/dsymutil/DwarfLinker.cpp
Added: llvm/trunk/test/tools/dsymutil/Inputs/dead-stripped/1.o
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/Inputs/dead-stripped/1.o?rev=247386&view=auto
==============================================================================
Binary files llvm/trunk/test/tools/dsymutil/Inputs/dead-stripped/1.o (added) and llvm/trunk/test/tools/dsymutil/Inputs/dead-stripped/1.o Thu Sep 10 23:17:30 2015 differ
Added: llvm/trunk/test/tools/dsymutil/X86/dead-stripped.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/dead-stripped.cpp?rev=247386&view=auto
==============================================================================
--- llvm/trunk/test/tools/dsymutil/X86/dead-stripped.cpp (added)
+++ llvm/trunk/test/tools/dsymutil/X86/dead-stripped.cpp Thu Sep 10 23:17:30 2015
@@ -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() {}
Modified: llvm/trunk/tools/dsymutil/DwarfLinker.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=247386&r1=247385&r2=247386&view=diff
==============================================================================
--- llvm/trunk/tools/dsymutil/DwarfLinker.cpp (original)
+++ llvm/trunk/tools/dsymutil/DwarfLinker.cpp Thu Sep 10 23:17:30 2015
@@ -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::cloneScalarAttribu
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 DWARFDe
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 DWARFDe
// 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::patchFrameInfoForObjec
}
}
+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 &M
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)
More information about the llvm-commits
mailing list