From ffe6466b17128a7391bd460ea0fbbca386a0fd22 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Mon, 26 Apr 2021 18:20:49 -0700 Subject: [PATCH] [dsymutil] Add flag to force a static variable to keep its enclosing function Add a flag to change dsymutil's behavior and force a static variable to keep its enclosing function. The test shows a situation where that could be useful. I'm not convinced this behavior makes sense as a default, which is why it's behind a flag. rdar://74918374 Differential revision: https://reviews.llvm.org/D101337 --- docs/CommandGuide/dsymutil.rst | 5 +++ include/llvm/DWARFLinker/DWARFLinker.h | 9 +++++ lib/DWARFLinker/DWARFLinker.cpp | 16 ++++---- .../Inputs/private/tmp/keep_func/main.o | Bin 0 -> 2552 bytes .../Inputs/private/tmp/keep_func/main.out | Bin 0 -> 66080 bytes test/tools/dsymutil/X86/keep-func.test | 36 ++++++++++++++++++ test/tools/dsymutil/cmdline.test | 1 + tools/dsymutil/DwarfLinkerForBinary.cpp | 1 + tools/dsymutil/LinkUtils.h | 4 ++ tools/dsymutil/Options.td | 4 ++ tools/dsymutil/dsymutil.cpp | 3 ++ 11 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o create mode 100755 test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out create mode 100644 test/tools/dsymutil/X86/keep-func.test diff --git a/docs/CommandGuide/dsymutil.rst b/docs/CommandGuide/dsymutil.rst index ca489cdabf6..74a72f63634 100644 --- a/docs/CommandGuide/dsymutil.rst +++ b/docs/CommandGuide/dsymutil.rst @@ -50,6 +50,11 @@ OPTIONS Print this help output. +.. option:: --keep-function-for-static + + Make a static variable keep the enclosing function even if it would have been + omitted otherwise. + .. option:: --minimize, -z When used when creating a dSYM file, this option will suppress the emission of diff --git a/include/llvm/DWARFLinker/DWARFLinker.h b/include/llvm/DWARFLinker/DWARFLinker.h index 1e35b9b717b..7b89c9f66f8 100644 --- a/include/llvm/DWARFLinker/DWARFLinker.h +++ b/include/llvm/DWARFLinker/DWARFLinker.h @@ -281,6 +281,11 @@ public: /// update existing DWARF info(for the linked binary). void setUpdate(bool Update) { Options.Update = Update; } + /// Set whether to keep the enclosing function for a static variable. + void setKeepFunctionForStatic(bool KeepFunctionForStatic) { + Options.KeepFunctionForStatic = KeepFunctionForStatic; + } + /// Use specified number of threads for parallel files linking. void setNumThreads(unsigned NumThreads) { Options.Threads = NumThreads; } @@ -782,6 +787,10 @@ private: /// Update bool Update = false; + /// Whether we want a static variable to force us to keep its enclosing + /// function. + bool KeepFunctionForStatic = false; + /// Number of threads. unsigned Threads = 1; diff --git a/lib/DWARFLinker/DWARFLinker.cpp b/lib/DWARFLinker/DWARFLinker.cpp index 3ad1f48aff2..58dd5918d4b 100644 --- a/lib/DWARFLinker/DWARFLinker.cpp +++ b/lib/DWARFLinker/DWARFLinker.cpp @@ -434,13 +434,15 @@ unsigned DWARFLinker::shouldKeepVariableDIE(AddressesMap &RelocMgr, return Flags | TF_Keep; } - // See if there is a relocation to a valid debug map entry inside - // this variable's location. The order is important here. We want to - // always check if the variable has a valid relocation, so that the - // DIEInfo is filled. However, we don't want a static variable in a - // function to force us to keep the enclosing function. - if (!RelocMgr.hasLiveMemoryLocation(DIE, MyInfo) || - (Flags & TF_InFunctionScope)) + // See if there is a relocation to a valid debug map entry inside this + // variable's location. The order is important here. We want to always check + // if the variable has a valid relocation, so that the DIEInfo is filled. + // However, we don't want a static variable in a function to force us to keep + // the enclosing function, unless requested explicitly. + const bool HasLiveMemoryLocation = + RelocMgr.hasLiveMemoryLocation(DIE, MyInfo); + if (!HasLiveMemoryLocation || ((Flags & TF_InFunctionScope) && + !LLVM_UNLIKELY(Options.KeepFunctionForStatic))) return Flags; if (Options.Verbose) { diff --git a/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o b/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o new file mode 100644 index 0000000000000000000000000000000000000000..715415e79be41f121f0ddfcf79f02aedd5765db9 GIT binary patch literal 2552 zcmb7GO>7%Q6n^Xf#PKGnOG!i#KBQ0%@lRZbP(+ET6D1;1O5*%gLQ{LaPMq57#dbmx zRZ7aCKL?~-kPsI*0C$j(dJLjRq;TNSLnVq32nkg^aH#+hzPCHuq^nevC%t*|e&(Au z^WLnz_vih8f<%g!Fo1pFJ3Pd|pkD9->rbGf^gWc)(Sb?K5xGu?mcHIP01+ZTvXDQ} zof+@P#CQyOsg20P1|My+a44V6iQMeuXIlFiw%8^NQm5gc6#(tL_*F!m+~bS z9?XwHI(QYMT<0ZjydNaa*HV5b@l4zw-y`TqJRI6`y76vuGk_l2g02SSeA*E+BWleB%YO@Si4SeUr`@ysa?-Lr~b6n+pQeKv+%^R(tC!y>Ui{Q__gp2 zrPG5W_)gGx)T^?E$RAY(!{M;41R{@Ohu{nLK|KdAQ7BG^YOeu&43@SywmudZ?%C|M z?c>omdf_8#f1XlN_AP`L!}AEGFTr*hLe`b|h)ZRFzW_5iUk9G^(LLVV7{|d%o3G95 zd^COhVcuhF$nOs+{=i+#`Bf_c3jFNMgV-zhBQ(D+^ee;z4)7`{m`%KaGWQ3M5nkNB zi9xmrsch~D}4W9RGjYBMpDd#jn4END|R3#n8*9dDK{lS@ssYG&fH zuA8E~YG^dMkz1|RHs%Tq8l9RFbJ=N;U%W6v>nW-jEm|KSad9Gzc3LA0RyH&?lO5wK z@r5vVfSK5)5ZX%4xxx7%?CyQ@Zsqsu;gzkAugu>&b)O%@C9nzJ2Hytr+BBHSss`jI zmPTaN0rP88=ZbN><9G658PS&=D(b-nnoF-+xOX=zehISCekXD=j&71G< z%k@NAwdIp0o+jVZkVKBV%8`l9T$kyP?f QV+wu)BuxCrX_blo1=4Xj*Z=?k literal 0 HcmV?d00001 diff --git a/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out b/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out new file mode 100755 index 0000000000000000000000000000000000000000..5ec92339e21269ee0b98c9683ac80be527dd5446 GIT binary patch literal 66080 zcmeI)&ud&&6u|L2NmECgrm+jLT4B_tXbYLdAK*qZqm!ColK#k~)TrFfkC$d}emV1| zc0eQ>kroO8H|nZuH*Q>HRRjJ3;?94-QW3$mNaH#8zL(5QlXXD82kyJ?ynEky_kC`X z-ODfk{PpMwV-my0m?3E=rKOG<^QB42nK7rN%}KNNQue*s)!8c-^=f}4`qOYr8WWm~DQHC*1mA6rp9-E$4i2W$8de^lphq*+3Tl!@p16{I3?&EhrbjF)7`rrgwo(Pt-R^jVktLrQY} z`I(Yl=V=)oUzhxZ_KAeql3ss5E=kkSb8R~Jc+^X`Z(Ni6_wyf<*(qtG($bxFD_yM= z)8%fpEV;(>(sUh<9{l;;$eo#=e*Exj`fB4%gb|Gc>~(aadLaQB5zrOPhwOcdn$=DHlr9}jkRb}lp;sZya@4csTD zdednglj!aBTTuO0^bP5?>G$dydamD?j>>nN<9pw3@)4)v5C|ZE00IagfB*srAbo&L*lsaKx*Movif3a4m)Xl_@%o#R+f1z_F;*&S!d@N~9 zazjeUc~0{u<*@DZkL0v0Esm<3^yVGO*M<}3Ug@jrI!9|v%Aq;q&%c)ZOi!-kXQZ8y zrlI+g9OC?2$+Cd + +static void Foo(void) +{ + typedef struct { + int x1; + int x2; + } FOO_VAR_TYPE; + static FOO_VAR_TYPE MyDummyVar __attribute__((aligned(4), used, section("TAD_VIRTUAL, TAD_DUMMY_DATA"), nocommon)); + printf("Foo called"); +} + +int main() +{ + Foo(); + return 1; +} + +$ clang++ -O2 -g main.cpp -c -o main.o +$ clang++ main.o -o main.out + +RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/keep_func/main.out -o %t.omit.dSYM +RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/keep_func/main.out -o %t.keep.dSYM -keep-function-for-static +RUN: llvm-dwarfdump %t.omit.dSYM | FileCheck %s --check-prefix OMIT +RUN: llvm-dwarfdump %t.keep.dSYM | FileCheck %s --check-prefix KEEP + +KEEP: DW_AT_name ("MyDummyVar") +KEEP: DW_AT_name ("FOO_VAR_TYPE") +KEEP: DW_AT_name ("x1") +KEEP: DW_AT_name ("x2") + +OMIT-NOT: DW_AT_name ("MyDummyVar") +OMIT-NOT: DW_AT_name ("FOO_VAR_TYPE") +OMIT-NOT: DW_AT_name ("x1") +OMIT-NOT: DW_AT_name ("x2") diff --git a/test/tools/dsymutil/cmdline.test b/test/tools/dsymutil/cmdline.test index ec8c52ed596..14707110825 100644 --- a/test/tools/dsymutil/cmdline.test +++ b/test/tools/dsymutil/cmdline.test @@ -11,6 +11,7 @@ CHECK: -dump-debug-map CHECK: -flat CHECK: -gen-reproducer CHECK: -help +CHECK: -keep-function-for-static CHECK: -no-odr CHECK: -no-output CHECK: -no-swiftmodule-timestamp diff --git a/tools/dsymutil/DwarfLinkerForBinary.cpp b/tools/dsymutil/DwarfLinkerForBinary.cpp index c597b696394..ca145484fa5 100644 --- a/tools/dsymutil/DwarfLinkerForBinary.cpp +++ b/tools/dsymutil/DwarfLinkerForBinary.cpp @@ -325,6 +325,7 @@ bool DwarfLinkerForBinary::link(const DebugMap &Map) { GeneralLinker.setNumThreads(Options.Threads); GeneralLinker.setAccelTableKind(Options.TheAccelTableKind); GeneralLinker.setPrependPath(Options.PrependPath); + GeneralLinker.setKeepFunctionForStatic(Options.KeepFunctionForStatic); if (Options.Translator) GeneralLinker.setStringsTranslator(TranslationLambda); GeneralLinker.setWarningHandler( diff --git a/tools/dsymutil/LinkUtils.h b/tools/dsymutil/LinkUtils.h index 58623af19cd..872a65deb4d 100644 --- a/tools/dsymutil/LinkUtils.h +++ b/tools/dsymutil/LinkUtils.h @@ -42,6 +42,10 @@ struct LinkOptions { /// Do not check swiftmodule timestamp bool NoTimestamp = false; + /// Whether we want a static variable to force us to keep its enclosing + /// function. + bool KeepFunctionForStatic = false; + /// Number of threads. unsigned Threads = 1; diff --git a/tools/dsymutil/Options.td b/tools/dsymutil/Options.td index b3d07531b81..67eac56c9ab 100644 --- a/tools/dsymutil/Options.td +++ b/tools/dsymutil/Options.td @@ -24,6 +24,10 @@ def verbose: F<"verbose">, HelpText<"Enable verbose mode.">, Group; +def keep_func_for_static: F<"keep-function-for-static">, + HelpText<"Make a static variable keep the enclosing function even if it would have been omitted otherwise.">, + Group; + def statistics: F<"statistics">, HelpText<"Print statistics about the contribution of each object file to " "the linked debug info. This prints a table after linking with the " diff --git a/tools/dsymutil/dsymutil.cpp b/tools/dsymutil/dsymutil.cpp index 830855e74e9..87369f2bc3c 100644 --- a/tools/dsymutil/dsymutil.cpp +++ b/tools/dsymutil/dsymutil.cpp @@ -91,6 +91,7 @@ struct DsymutilOptions { bool InputIsYAMLDebugMap = false; bool PaperTrailWarnings = false; bool Verify = false; + bool ForceKeepFunctionForStatic = false; std::string SymbolMap; std::string OutputFile; std::string Toolchain; @@ -230,6 +231,8 @@ static Expected getOptions(opt::InputArgList &Args) { Options.LinkOpts.Update = Args.hasArg(OPT_update); Options.LinkOpts.Verbose = Args.hasArg(OPT_verbose); Options.LinkOpts.Statistics = Args.hasArg(OPT_statistics); + Options.LinkOpts.KeepFunctionForStatic = + Args.hasArg(OPT_keep_func_for_static); if (opt::Arg *ReproducerPath = Args.getLastArg(OPT_use_reproducer)) { Options.ReproMode = ReproducerMode::Use;