From 5f69f9fb8178057f7678d95ae89dd0a09b446369 Mon Sep 17 00:00:00 2001 From: Frederic Riss Date: Fri, 11 Sep 2015 04:17:30 +0000 Subject: [PATCH] [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 --- test/tools/dsymutil/Inputs/dead-stripped/1.o | Bin 0 -> 3200 bytes test/tools/dsymutil/X86/dead-stripped.cpp | 48 +++++++++++++ tools/dsymutil/DwarfLinker.cpp | 72 +++++++++++++++++-- 3 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 test/tools/dsymutil/Inputs/dead-stripped/1.o create mode 100644 test/tools/dsymutil/X86/dead-stripped.cpp diff --git a/test/tools/dsymutil/Inputs/dead-stripped/1.o b/test/tools/dsymutil/Inputs/dead-stripped/1.o new file mode 100644 index 0000000000000000000000000000000000000000..fbdffbc61a77479281e8ea3ca1455cfa196661d0 GIT binary patch literal 3200 zcma)7Uu=_A6hF7^_tCXoR~#q?aE%!e473|e`6JN|HZqrGK{nc64@CArjE_8EqAw;I!;3MP=!-$)3oj;OVxppniBF=j{?7f*?YeG+oqXpzf9|=z zd(QoP=fNNUts+uh!Vl~NM=kz=K~I8jvhyL-W4jFr?gKJ5Fo`+LPLShN(z6v;ckSWc z(cuN#Xa_`LJRbI9kjTS6ADv@i$Z;l;m87v4zo&b+d%^A#)ucWLJdLvuvPc77IyF7% zlq)6e2i~jQhfFp^0}}m_$akdPI*>*11}|3_Ul#9g>DVuFx6I?C@RF%iDLtd>U6kK` zUOdouuAmDm#LMP01@6*V?;5GbFSx_Jn4o#A2R^1{>n$bolj-tOybZj;>gaWUo#Cnt zc*$Zhmv-{WDTZ5;pTYZ6U{;D(NS)9N(pYauHcGqT4K#_eAReAyF*)9d*CBX&rGIgS zc$JgIW$VoeUPkafd?em@VG4Owoay|V+5Cj#4D>9NLUdQ~j)`+!fQXmqcPy?fRYSe$ z<4&f8)o?pF&f@dCOU}bp!J8F4F?r7%H#*DO`N-xqV&nXMEqJG;lV3KSFXY}+a4*WQ z9gYac1~D**8`ptDi04uVAiE$L<{-G9xLu~B-n}CIaNp&hujpLIZnv~0i0z?+S2l9z zx2lz3_0BK9R;yKY_rB-y-M$N7`j3v?&SWyNmbQx;pX0}F6Fm!SJcq**ok^F<*+M?L zBepBn5#3xVP3PZ;ZtvRJvHiI%(apKs%v8M2*SUq_m8oKUBAuMrhRZ8kET$)7{j0Jp5P0-AG^pKe!2Cf5SyOB+X*}>z8G;_BGB1NWW@+hH=N@D5=B2d2563HY{b)K zk;gV2i2PlP%%+Xh*~(or9-K}`!H7dsBfsdkE`V|nQQ4oS7Z~;MHQ-^5x#TS=2~s=`U78_y}vnmV&9R> znIDdRdggL@F8J?_`O~+K%>^rH=hHI}z5)IW{4MxLFwYl{`8=43^J4oR8z7z!wokGF z;yL8C;yd9Mn2GD)e2mZZr>aCCAxW*Xn_tz&L+6t-n1td>7 zlj+O(GMk0sTzNjdWMQvhmuj1+ 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 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) -- 2.34.1