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

[JDevlieghere]

Summary

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

[friss]

LGTM

[dblaikie]

Have you got any size data on it? Might be nice to avoid the fragmentation/extra flag if the cost isn't especially high.

llvm/docs/CommandGuide/dsymutil.rst

 .. option:: --gen-reproducer
 
  Generate a reproducer consisting of the input object files.
 
 .. option:: --help, -h
 
  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
  the .debug_inlines, .debug_pubnames, and .debug_pubtypes sections since
  dsymutil currently has better equivalents: .apple_names and .apple_types. When
  used in conjunction with ``--update`` option, this option will cause redundant
  accelerator tables to be removed.
 

llvm/include/llvm/DWARFLinker/DWARFLinker.h

public:
   void setNoOutput(bool NoOut) { Options.NoOutput = NoOut; }
 
   /// Do not unique types according to ODR.
   void setNoODR(bool NoODR) { Options.NoODR = NoODR; }
 
   /// 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; }
 
   /// Set kind of accelerator tables to be generated.
   void setAccelTableKind(AccelTableKind Kind) {
     Options.TheAccelTableKind = Kind;
   }
 
struct DWARFLinkerOptions {
     bool NoOutput = false;
 
     /// Do not unique types according to ODR
     bool NoODR = false;
 
     /// 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;
 
     /// The accelerator table kind
     AccelTableKind TheAccelTableKind = AccelTableKind::Default;
 
     /// Prepend path for the clang modules.
     std::string PrependPath;

llvm/lib/DWARFLinker/DWARFLinker.cpp

unsigned DWARFLinker::shouldKeepVariableDIE(AddressesMap &RelocMgr,
 
   // Global variables with constant value can always be kept.
   if (!(Flags & TF_InFunctionScope) &&
       Abbrev->findAttributeIndex(dwarf::DW_AT_const_value)) {
     MyInfo.InDebugMap = true;
     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) {
     outs() << "Keeping variable DIE:";
     DIDumpOptions DumpOpts;
     DumpOpts.ChildRecurseDepth = 0;
     DumpOpts.Verbose = Options.Verbose;
     DIE.dump(outs(), 8 /* Indent */, DumpOpts);

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

+$ 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/cmdline.test

 RUN: dsymutil -help 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=HELP %s
 RUN: cat %S/../../../docs/CommandGuide/dsymutil.rst  | FileCheck %s
 
 HELP: OVERVIEW: manipulate archived DWARF debug symbol files.
 HELP: USAGE: {{.*}}dsymutil{{[^ ]*}} [options] <input files>
 HELP-NOT: -reverse-iterate
 HELP: Dsymutil Options:
 CHECK: -accelerator
 CHECK: -arch <arch>
 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
 CHECK: -num-threads  <threads>
 CHECK: -object-prefix-map <prefix=remapped>
 CHECK: -oso-prepend-path <path>
 CHECK: -out <filename>
 CHECK: {{-o <filename>}}

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

bool DwarfLinkerForBinary::link(const DebugMap &Map) {
   GeneralLinker.setVerbosity(Options.Verbose);
   GeneralLinker.setStatistics(Options.Statistics);
   GeneralLinker.setNoOutput(Options.NoOutput);
   GeneralLinker.setNoODR(Options.NoODR);
   GeneralLinker.setUpdate(Options.Update);
   GeneralLinker.setNumThreads(Options.Threads);
   GeneralLinker.setAccelTableKind(Options.TheAccelTableKind);
   GeneralLinker.setPrependPath(Options.PrependPath);
+  GeneralLinker.setKeepFunctionForStatic(Options.KeepFunctionForStatic);
   if (Options.Translator)
     GeneralLinker.setStringsTranslator(TranslationLambda);
   GeneralLinker.setWarningHandler(
       [&](const Twine &Warning, StringRef Context, const DWARFDie *DIE) {
         reportWarning(Warning, Context, DIE);
       });
   GeneralLinker.setErrorHandler(
       [&](const Twine &Error, StringRef Context, const DWARFDie *) {

llvm/tools/dsymutil/LinkUtils.h

struct LinkOptions {
   bool NoODR = false;
 
   /// Update
   bool Update = false;
 
   /// 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;
 
   // Output file type.
   OutputFileType FileType = OutputFileType::Object;
 
   /// The accelerator table kind
   AccelTableKind TheAccelTableKind;

llvm/tools/dsymutil/Options.td

def: Flag<["-"], "v">,
   Alias<version>,
   HelpText<"Alias for --version">,
   Group<grp_general>;
 
 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 "
            "object file name, the size of the debug info in the object file "
            "(in bytes) and the size contributed (in bytes) to the linked dSYM. "
            "The table is sorted by the output size listing the object files "
            "with the largest contribution first.">,
   Group<grp_general>;

llvm/tools/dsymutil/dsymutil.cpp

 
 struct DsymutilOptions {
   bool DumpDebugMap = false;
   bool DumpStab = false;
   bool Flat = false;
   bool InputIsYAMLDebugMap = false;
   bool PaperTrailWarnings = false;
   bool Verify = false;
+  bool ForceKeepFunctionForStatic = false;
   std::string SymbolMap;
   std::string OutputFile;
   std::string Toolchain;
   std::string ReproducerPath;
   std::vector<std::string> Archs;
   std::vector<std::string> InputFiles;
   unsigned NumThreads;
   ReproducerMode ReproMode = ReproducerMode::Off;
static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
   Options.Verify = Args.hasArg(OPT_verify);
 
   Options.LinkOpts.NoODR = Args.hasArg(OPT_no_odr);
   Options.LinkOpts.NoOutput = Args.hasArg(OPT_no_output);
   Options.LinkOpts.NoTimestamp = Args.hasArg(OPT_no_swiftmodule_timestamp);
   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;
     Options.ReproducerPath = ReproducerPath->getValue();
   }
 
   if (Args.hasArg(OPT_gen_reproducer))
     Options.ReproMode = ReproducerMode::Generate;