From 8a6c6bbbd4a79f9df7d7d2804aa2f5af376721d6 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 9 Oct 2019 16:19:13 +0000 Subject: [PATCH] Re-land "[dsymutil] Fix handling of common symbols in multiple object files." The original patch got reverted because it hit a long-standing legacy issue on Windows that prevents files from being named `com`. Thanks Kristina & Jeremy for pointing this out. llvm-svn: 374178 --- .../Inputs/private/tmp/common/common.x86_64 | Bin 0 -> 4688 bytes .../Inputs/private/tmp/common/common1.o | Bin 0 -> 2108 bytes .../Inputs/private/tmp/common/common2.o | Bin 0 -> 2096 bytes test/tools/dsymutil/X86/common-sym-multi.test | 39 ++++++++++++++++++ tools/dsymutil/MachODebugMapParser.cpp | 35 +++++++++++++++- 5 files changed, 72 insertions(+), 2 deletions(-) create mode 100755 test/tools/dsymutil/Inputs/private/tmp/common/common.x86_64 create mode 100644 test/tools/dsymutil/Inputs/private/tmp/common/common1.o create mode 100644 test/tools/dsymutil/Inputs/private/tmp/common/common2.o create mode 100644 test/tools/dsymutil/X86/common-sym-multi.test diff --git a/test/tools/dsymutil/Inputs/private/tmp/common/common.x86_64 b/test/tools/dsymutil/Inputs/private/tmp/common/common.x86_64 new file mode 100755 index 0000000000000000000000000000000000000000..8260af4a1b79072761618d3ef6d10f01e59248d1 GIT binary patch literal 4688 zcmeHL&ubGw6rOETEwME5;4c(iq=*&FYETpeNud@?JP;;MLL31~fRyFJ_2PQ*&TVR=Vnpdg#yFnKG1)X zIe3}CEMWDz9c#;}S&f~o1~}y?z&buVUms<1SFY|pedngvRzBpQGXXzmI)xuV%0Kfh zgqWE+gB7L!lGC_9lX0L3_KP^{S&RA`W1Qn=Ghv9-p6461^B!3& zteNKyFdyR7%>#!33i&dYFmF()WITT#`fxq>!I^({@H}o|5V&Wfw-`_~`b)+Mt|5hA# zW!LM~WDWml;0dkmj&rx_*_P*STfzScmI(Iq+4@AYwj!(_GvK+udgrq6R+an&aGtX; literal 0 HcmV?d00001 diff --git a/test/tools/dsymutil/Inputs/private/tmp/common/common1.o b/test/tools/dsymutil/Inputs/private/tmp/common/common1.o new file mode 100644 index 0000000000000000000000000000000000000000..f238fc8fd03a87824e1a7120b6ddc8a360f6cc5d GIT binary patch literal 2108 zcma)7&2JM|5TCdHh>4SpsgaV1s&XVMK_zSlLgJ(aVu7nf;((G+5+d}vUhgIw+3S_P z2^1kU5-KX9t$IXAl~9lU1KQ@?LvQUrz<~>h3wK0Y(wVpO5@Uk!So7xN&HQ%Wybu5V z>c#6m01*QmkmJbPG~|$xMv+rwzk|+FBGMy{(KccN@c>OCNf|mf!$^gJkkhb`rj^5WI5>vH>wr|pj)!Tc+YXY+|mQ_8kVgfyYaqfJQi>= z`M&zBAgDXiw;E3Sfck!5pk;>1_lVc5ZrGI4UEedtJH~i-d&E=q1(w~7*U#P2c)#?B z7k(8SxZXVDxm@30@5Qs54Xi7aoBj>YuSt37BBv_xfbnwdEa`ns@p*ph{5k8=ZQ*W2 z2~xg!KYnMtS;kw|cx&0TpXa*gE5z>py=1%!Cp|zsqSQO;qdsRSV5px)b$Lcm zW-y+hiYV!>V;yiAxszz?HFx@Z(XY(idpdVtU5-C1|5C>N+4%K8?7HQz<+hww+iUu{ z@nRl-x#R0$7_?^!g*7i+->l|whYI!jRy3RpTFni|4s)j`r(O4yHC1z_imp>DO`f(V z&bY<#qFtPrE{#uGRrgHIJykjZnBQpn<9QnjLCf2+LZ=WmfNKj&?yWG2bfzd&(YUt$BFgp z9_?;)WpvfvUDTdme_7mG0C6)T zG6Q|7A2Es`K14qLAFXM3SN~5z+05KEE)42tr;IUi7{gHr@sBx3fR>(*JwMf{`|3F)?1*sf~LrBt+fY4*T-c2^L*DHII zrW{bDDiIMq_9sm&s9FR5SO&YSuNN12!G(U#MC=lr>`$TiZ1mb>LM3OSFH-bY*MNkk-N)vl~9%T~}-CMj>+F3o)tEklI!j4jVOzkLUQSeuUdg?MJA z(zMqp-@|w>*!e!oTZ#5)r5}msdQO8@JMjk9rU0UnykNZ7xL+>mk$82}Qjnc^Ta3p7 z&L>aG&-DG8Ej_btw~okfn}HS?=0cZvjmoM;A)WdC!FWTA_pnPmm7i}~op>>>PK5V! zmw3Ss{*n8=z<3Vl_v?vxR-=x21#->%-u0?d-n_w~O6)RTmW>5I*SAqWf6#ySigcQI z8exExhwsO8#w#-3>_ML>iw>~DOG-rjTys5z*jc~7881qD7x9SR9g`o`S>36DKIKQ@ zr;)W(xRD{0bk{Ktm_=?UI{Jv)?cM0#mUsRvKUSCHaZ$VX%Bq>(a(3Nrw%mr79WLbX z%YMER1b*vEKELb+D{GY;9!P$E>PqzkcMPa^hHFYk75fo~Ysfs~(LnGBM47;%QE!I% z6gD5}cUwQbF5;YOL3JQ}+AVcD64iuEw9F>d_DS7^&|}|5O7v&(U=GAaTBQ4XQoD#l z5T79z-qW7;_BbA(WTdxa*H1;yO%Z)GIESMU;$Pzw2}%Q5wDl!g0D5V=iZY45zF!jY vWpXl<##x@Dp!H;qCE*Xyz(l#=RC_wW21$)AU7Gz5e4pY( literal 0 HcmV?d00001 diff --git a/test/tools/dsymutil/X86/common-sym-multi.test b/test/tools/dsymutil/X86/common-sym-multi.test new file mode 100644 index 00000000000..1f33b37283b --- /dev/null +++ b/test/tools/dsymutil/X86/common-sym-multi.test @@ -0,0 +1,39 @@ +RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/common/common.x86_64 -f -o - | llvm-dwarfdump -debug-info - | FileCheck %s +RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/common/common.x86_64 -dump-debug-map | FileCheck %s --check-prefix DEBUGMAP + +The test was compiled from two source files: +$ cd /private/tmp/common +$ cat common1.c +int i[1000]; +int main() { + return i[1]; +} +$ cat common2.c +extern int i[1000]; +int bar() { + return i[0]; +} +$ clang -fcommon -g -c common1.c -o common1.o +$ clang -fcommon -g -c common2.c -o common2.o +$ clang -fcommon -g common1.o common2.o -o common.x86_64 + +CHECK: DW_TAG_compile_unit +CHECK: DW_TAG_variable +CHECK-NOT: {{NULL|DW_TAG}} +CHECK: DW_AT_name{{.*}}"i" +CHECK-NOT: {{NULL|DW_TAG}} +CHECK: DW_AT_location{{.*}}DW_OP_addr 0x100001000) + +CHECK: DW_TAG_compile_unit +CHECK: DW_TAG_variable +CHECK-NOT: {{NULL|DW_TAG}} +CHECK: DW_AT_name{{.*}}"i" +CHECK-NOT: {{NULL|DW_TAG}} +CHECK: DW_AT_location{{.*}}DW_OP_addr 0x100001000) + +DEBUGMAP: filename:{{.*}}common1.o +DEBUGMAP: symbols: +DEBUGMAP: sym: _i, binAddr: 0x0000000100001000, size: 0x00000000 +DEBUGMAP: filename:{{.*}}common2.o +DEBUGMAP: symbols: +DEBUGMAP: sym: _i, binAddr: 0x0000000100001000, size: 0x00000000 diff --git a/tools/dsymutil/MachODebugMapParser.cpp b/tools/dsymutil/MachODebugMapParser.cpp index 27379c232de..487fbfff50c 100644 --- a/tools/dsymutil/MachODebugMapParser.cpp +++ b/tools/dsymutil/MachODebugMapParser.cpp @@ -14,6 +14,7 @@ #include "llvm/Support/Path.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" +#include namespace { using namespace llvm; @@ -51,6 +52,8 @@ private: StringRef MainBinaryStrings; /// The constructed DebugMap. std::unique_ptr Result; + /// List of common symbols that need to be added to the debug map. + std::vector CommonSymbols; /// Map of the currently processed object file symbol addresses. StringMap> CurrentObjectAddresses; @@ -81,6 +84,8 @@ private: STE.n_value); } + void addCommonSymbols(); + /// Dump the symbol table output header. void dumpSymTabHeader(raw_ostream &OS, StringRef Arch); @@ -122,11 +127,32 @@ void MachODebugMapParser::resetParserState() { CurrentDebugMapObject = nullptr; } +/// Commons symbols won't show up in the symbol map but might need to be +/// relocated. We can add them to the symbol table ourselves by combining the +/// information in the object file (the symbol name) and the main binary (the +/// address). +void MachODebugMapParser::addCommonSymbols() { + for (auto &CommonSymbol : CommonSymbols) { + uint64_t CommonAddr = getMainBinarySymbolAddress(CommonSymbol); + if (CommonAddr == 0) { + // The main binary doesn't have an address for the given symbol. + continue; + } + if (!CurrentDebugMapObject->addSymbol(CommonSymbol, None /*ObjectAddress*/, + CommonAddr, 0 /*size*/)) { + // The symbol is already present. + continue; + } + } + CommonSymbols.clear(); +} + /// Create a new DebugMapObject. This function resets the state of the /// parser that was referring to the last object file and sets /// everything up to add symbols to the new one. void MachODebugMapParser::switchToNewDebugMapObject( StringRef Filename, sys::TimePoint Timestamp) { + addCommonSymbols(); resetParserState(); SmallString<80> Path(PathPrefix); @@ -466,10 +492,15 @@ void MachODebugMapParser::loadCurrentObjectFileSymbols( // relocations will use the symbol itself, and won't need an // object file address. The object file address field is optional // in the DebugMap, leave it unassigned for these symbols. - if (Sym.getFlags() & (SymbolRef::SF_Absolute | SymbolRef::SF_Common)) + uint32_t Flags = Sym.getFlags(); + if (Flags & SymbolRef::SF_Absolute) { CurrentObjectAddresses[*Name] = None; - else + } else if (Flags & SymbolRef::SF_Common) { + CurrentObjectAddresses[*Name] = None; + CommonSymbols.push_back(*Name); + } else { CurrentObjectAddresses[*Name] = Addr; + } } }