[llvm] 625bd94 - [dsymutil] Add flag to force a static variable to keep its enclosing function

Jonas Devlieghere via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 28 11:33:09 PDT 2021


Author: Jonas Devlieghere
Date: 2021-04-28T11:33:04-07:00
New Revision: 625bd94c6d645c4cae40649800de8047bacd3f53

URL: https://github.com/llvm/llvm-project/commit/625bd94c6d645c4cae40649800de8047bacd3f53
DIFF: https://github.com/llvm/llvm-project/commit/625bd94c6d645c4cae40649800de8047bacd3f53.diff

LOG: [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

Added: 
    llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o
    llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out
    llvm/test/tools/dsymutil/X86/keep-func.test

Modified: 
    llvm/docs/CommandGuide/dsymutil.rst
    llvm/include/llvm/DWARFLinker/DWARFLinker.h
    llvm/lib/DWARFLinker/DWARFLinker.cpp
    llvm/test/tools/dsymutil/cmdline.test
    llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
    llvm/tools/dsymutil/LinkUtils.h
    llvm/tools/dsymutil/Options.td
    llvm/tools/dsymutil/dsymutil.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/docs/CommandGuide/dsymutil.rst b/llvm/docs/CommandGuide/dsymutil.rst
index ca489cdabf693..74a72f6363445 100644
--- a/llvm/docs/CommandGuide/dsymutil.rst
+++ b/llvm/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/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
index 1e35b9b717b6f..7b89c9f66f86f 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -281,6 +281,11 @@ class DWARFLinker {
   /// 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 @@ class DWARFLinker {
     /// 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/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index 3ad1f48aff2e9..58dd5918d4b2f 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/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/llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o b/llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o
new file mode 100644
index 0000000000000..715415e79be41
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o 
diff er

diff  --git a/llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out b/llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out
new file mode 100755
index 0000000000000..5ec92339e2126
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out 
diff er

diff  --git a/llvm/test/tools/dsymutil/X86/keep-func.test b/llvm/test/tools/dsymutil/X86/keep-func.test
new file mode 100644
index 0000000000000..021c7212bd831
--- /dev/null
+++ b/llvm/test/tools/dsymutil/X86/keep-func.test
@@ -0,0 +1,36 @@
+$ cat main.cpp
+#include <stdio.h>
+
+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/llvm/test/tools/dsymutil/cmdline.test b/llvm/test/tools/dsymutil/cmdline.test
index ec8c52ed596f1..14707110825b4 100644
--- a/llvm/test/tools/dsymutil/cmdline.test
+++ b/llvm/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/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
index c597b696394d5..ca145484fa5cb 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ b/llvm/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/llvm/tools/dsymutil/LinkUtils.h b/llvm/tools/dsymutil/LinkUtils.h
index 58623af19cdba..872a65deb4dcd 100644
--- a/llvm/tools/dsymutil/LinkUtils.h
+++ b/llvm/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/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td
index b3d07531b81c7..67eac56c9abd2 100644
--- a/llvm/tools/dsymutil/Options.td
+++ b/llvm/tools/dsymutil/Options.td
@@ -24,6 +24,10 @@ def verbose: F<"verbose">,
   HelpText<"Enable verbose mode.">,
   Group<grp_general>;
 
+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<grp_general>;
+
 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/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp
index 830855e74e9ff..87369f2bc3cfa 100644
--- a/llvm/tools/dsymutil/dsymutil.cpp
+++ b/llvm/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<DsymutilOptions> 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;


        


More information about the llvm-commits mailing list