Merge pull request #2890 from apple/🍒/ganymede/625bd94c6d645c4cae40649800de8047bacd3f53

[dsymutil] Add flag to force a static variable to keep its enclosing …

Author: JDevlieghere

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

llvm/include/llvm/DWARFLinker/DWARFLinker.h

@@ -279,6 +279,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; }
 
@@ -779,6 +784,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;
 

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) {

llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.o

@@ -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")

llvm/test/tools/dsymutil/Inputs/private/tmp/keep_func/main.out

@@ -11,6 +11,7 @@ CHECK: -dump-debug-map
 CHECK: -flat
 CHECK: -gen-reproducer
 CHECK: -help
+CHECK: -keep-function-for-static
 CHECK: -minimize
 CHECK: -no-odr
 CHECK: -no-output

llvm/test/tools/dsymutil/X86/keep-func.test

@@ -322,6 +322,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(

llvm/test/tools/dsymutil/cmdline.test

@@ -45,6 +45,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;
 

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

@@ -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 "

llvm/tools/dsymutil/LinkUtils.h

@@ -92,6 +92,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;