[llvm] 82a06a6 - [DWARFLinker][NFC] Change interface of DWARFLinker to specify accel table kinds explicitly.

Alexey Lapshin via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 4 02:17:31 PST 2022


Author: Alexey Lapshin
Date: 2022-12-04T10:40:56+01:00
New Revision: 82a06a6bd180d7062095131e26a2a04ffa790b2f

URL: https://github.com/llvm/llvm-project/commit/82a06a6bd180d7062095131e26a2a04ffa790b2f
DIFF: https://github.com/llvm/llvm-project/commit/82a06a6bd180d7062095131e26a2a04ffa790b2f.diff

LOG: [DWARFLinker][NFC] Change interface of DWARFLinker to specify accel table kinds explicitly.

Currently, DWARFLinker receives kind of accel tables as predefined sets:

```
  Apple,   ///< .apple_names, .apple_namespaces, .apple_types, .apple_objc.
  Dwarf,   ///< DWARF v5 .debug_names.
  Default, ///< Dwarf for DWARF5 or later, Apple otherwise.
  Pub,     ///< .debug_pubnames, .debug_pubtypes
```

This patch removes implicit sets of tables(Default, Dwarf) and allows to ask for several sets:

```
  Apple,     ///< .apple_names, .apple_namespaces, .apple_types, .apple_objc.
  Pub,       ///< .debug_pubnames, .debug_pubtypes
  DebugNames ///< .debug_names.
```

It allows seamlessness adding more accel tables in the future: .gdb_index, .debug_cu_index...
Doing things that way, DWARFLinker will be independent of consumers' requirements.
f.e. dsymutil and llvm-dwarfutil may have different variants for Default set
(so, instead of implementing these differencies inside DWARFLinker it could be
implemented in the corresponding module).

Differential Revision: https://reviews.llvm.org/D132371

Added: 
    

Modified: 
    llvm/include/llvm/DWARFLinker/DWARFLinker.h
    llvm/lib/DWARFLinker/DWARFLinker.cpp
    llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
    llvm/tools/dsymutil/LinkUtils.h
    llvm/tools/dsymutil/dsymutil.cpp
    llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
index 6c202c6b8e5a5..be8c8c2860e1a 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -31,11 +31,9 @@ enum class DwarfLinkerClient { Dsymutil, LLD, General };
 
 /// The kind of accelerator tables we should emit.
 enum class DwarfLinkerAccelTableKind : uint8_t {
-  None,
-  Apple,   ///< .apple_names, .apple_namespaces, .apple_types, .apple_objc.
-  Dwarf,   ///< DWARF v5 .debug_names.
-  Default, ///< Dwarf for DWARF5 or later, Apple otherwise.
-  Pub,     ///< .debug_pubnames, .debug_pubtypes
+  Apple,     ///< .apple_names, .apple_namespaces, .apple_types, .apple_objc.
+  Pub,       ///< .debug_pubnames, .debug_pubtypes
+  DebugNames ///< .debug_names.
 };
 
 /// AddressesMap represents information about valid addresses used
@@ -284,9 +282,11 @@ class DWARFLinker {
   /// 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(DwarfLinkerAccelTableKind Kind) {
-    Options.TheAccelTableKind = Kind;
+  /// Add kind of accelerator tables to be generated.
+  void addAccelTableKind(DwarfLinkerAccelTableKind Kind) {
+    assert(std::find(Options.AccelTables.begin(), Options.AccelTables.end(),
+                     Kind) == Options.AccelTables.end());
+    Options.AccelTables.emplace_back(Kind);
   }
 
   /// Set prepend path for clang modules.
@@ -409,9 +409,6 @@ class DWARFLinker {
       Options.ErrorHandler(Warning, File.FileName, DIE);
   }
 
-  /// Remembers the kinds of accelerator tables we've seen in a unit.
-  void updateAccelKind(DWARFContext &Dwarf);
-
   /// Emit warnings as Dwarf compile units to leave a trail after linking.
   bool emitPaperTrailWarnings(const DWARFFile &File,
                               OffsetsStringPool &StringPool);
@@ -735,9 +732,6 @@ class DWARFLinker {
 
   /// Emit the accelerator entries for \p Unit.
   void emitAcceleratorEntriesForUnit(CompileUnit &Unit);
-  void emitDwarfAcceleratorEntriesForUnit(CompileUnit &Unit);
-  void emitAppleAcceleratorEntriesForUnit(CompileUnit &Unit);
-  void emitPubAcceleratorEntriesForUnit(CompileUnit &Unit);
 
   /// Patch the frame info for an object file and emit it.
   void patchFrameInfoForObject(const DWARFFile &, RangesTy &Ranges,
@@ -764,9 +758,6 @@ class DWARFLinker {
   DwarfEmitter *TheDwarfEmitter;
   std::vector<LinkContext> ObjectContexts;
 
-  bool AtLeastOneAppleAccelTable = false;
-  bool AtLeastOneDwarfAccelTable = false;
-
   /// The CIEs that have been emitted in the output section. The actual CIE
   /// data serves a the key to this StringMap, this takes care of comparing the
   /// semantics of CIEs defined in 
diff erent object files.
@@ -823,9 +814,8 @@ class DWARFLinker {
     /// Number of threads.
     unsigned Threads = 1;
 
-    /// The accelerator table kind
-    DwarfLinkerAccelTableKind TheAccelTableKind =
-        DwarfLinkerAccelTableKind::Default;
+    /// The accelerator table kinds
+    SmallVector<DwarfLinkerAccelTableKind, 1> AccelTables;
 
     /// Prepend path for the clang modules.
     std::string PrependPath;

diff  --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index 9588528ca37fe..e067828ed0795 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -1814,67 +1814,49 @@ void DWARFLinker::patchLineTableForUnit(CompileUnit &Unit,
 }
 
 void DWARFLinker::emitAcceleratorEntriesForUnit(CompileUnit &Unit) {
-  switch (Options.TheAccelTableKind) {
-  case DwarfLinkerAccelTableKind::None:
-    // Nothing to do.
-    break;
-  case DwarfLinkerAccelTableKind::Apple:
-    emitAppleAcceleratorEntriesForUnit(Unit);
-    break;
-  case DwarfLinkerAccelTableKind::Dwarf:
-    emitDwarfAcceleratorEntriesForUnit(Unit);
-    break;
-  case DwarfLinkerAccelTableKind::Pub:
-    emitPubAcceleratorEntriesForUnit(Unit);
-    break;
-  case DwarfLinkerAccelTableKind::Default:
-    llvm_unreachable("The default must be updated to a concrete value.");
-    break;
+  for (DwarfLinkerAccelTableKind AccelTableKind : Options.AccelTables) {
+    switch (AccelTableKind) {
+    case DwarfLinkerAccelTableKind::Apple: {
+      // Add namespaces.
+      for (const auto &Namespace : Unit.getNamespaces())
+        AppleNamespaces.addName(Namespace.Name, Namespace.Die->getOffset() +
+                                                    Unit.getStartOffset());
+      // Add names.
+      for (const auto &Pubname : Unit.getPubnames())
+        AppleNames.addName(Pubname.Name,
+                           Pubname.Die->getOffset() + Unit.getStartOffset());
+      // Add types.
+      for (const auto &Pubtype : Unit.getPubtypes())
+        AppleTypes.addName(
+            Pubtype.Name, Pubtype.Die->getOffset() + Unit.getStartOffset(),
+            Pubtype.Die->getTag(),
+            Pubtype.ObjcClassImplementation ? dwarf::DW_FLAG_type_implementation
+                                            : 0,
+            Pubtype.QualifiedNameHash);
+      // Add ObjC names.
+      for (const auto &ObjC : Unit.getObjC())
+        AppleObjc.addName(ObjC.Name,
+                          ObjC.Die->getOffset() + Unit.getStartOffset());
+    } break;
+    case DwarfLinkerAccelTableKind::Pub: {
+      TheDwarfEmitter->emitPubNamesForUnit(Unit);
+      TheDwarfEmitter->emitPubTypesForUnit(Unit);
+    } break;
+    case DwarfLinkerAccelTableKind::DebugNames: {
+      for (const auto &Namespace : Unit.getNamespaces())
+        DebugNames.addName(Namespace.Name, Namespace.Die->getOffset(),
+                           Namespace.Die->getTag(), Unit.getUniqueID());
+      for (const auto &Pubname : Unit.getPubnames())
+        DebugNames.addName(Pubname.Name, Pubname.Die->getOffset(),
+                           Pubname.Die->getTag(), Unit.getUniqueID());
+      for (const auto &Pubtype : Unit.getPubtypes())
+        DebugNames.addName(Pubtype.Name, Pubtype.Die->getOffset(),
+                           Pubtype.Die->getTag(), Unit.getUniqueID());
+    } break;
+    }
   }
 }
 
-void DWARFLinker::emitAppleAcceleratorEntriesForUnit(CompileUnit &Unit) {
-  // Add namespaces.
-  for (const auto &Namespace : Unit.getNamespaces())
-    AppleNamespaces.addName(Namespace.Name,
-                            Namespace.Die->getOffset() + Unit.getStartOffset());
-
-  /// Add names.
-  for (const auto &Pubname : Unit.getPubnames())
-    AppleNames.addName(Pubname.Name,
-                       Pubname.Die->getOffset() + Unit.getStartOffset());
-
-  /// Add types.
-  for (const auto &Pubtype : Unit.getPubtypes())
-    AppleTypes.addName(
-        Pubtype.Name, Pubtype.Die->getOffset() + Unit.getStartOffset(),
-        Pubtype.Die->getTag(),
-        Pubtype.ObjcClassImplementation ? dwarf::DW_FLAG_type_implementation
-                                        : 0,
-        Pubtype.QualifiedNameHash);
-
-  /// Add ObjC names.
-  for (const auto &ObjC : Unit.getObjC())
-    AppleObjc.addName(ObjC.Name, ObjC.Die->getOffset() + Unit.getStartOffset());
-}
-
-void DWARFLinker::emitDwarfAcceleratorEntriesForUnit(CompileUnit &Unit) {
-  for (const auto &Namespace : Unit.getNamespaces())
-    DebugNames.addName(Namespace.Name, Namespace.Die->getOffset(),
-                       Namespace.Die->getTag(), Unit.getUniqueID());
-  for (const auto &Pubname : Unit.getPubnames())
-    DebugNames.addName(Pubname.Name, Pubname.Die->getOffset(),
-                       Pubname.Die->getTag(), Unit.getUniqueID());
-  for (const auto &Pubtype : Unit.getPubtypes())
-    DebugNames.addName(Pubtype.Name, Pubtype.Die->getOffset(),
-                       Pubtype.Die->getTag(), Unit.getUniqueID());
-}
-
-void DWARFLinker::emitPubAcceleratorEntriesForUnit(CompileUnit &Unit) {
-  TheDwarfEmitter->emitPubNamesForUnit(Unit);
-  TheDwarfEmitter->emitPubTypesForUnit(Unit);
-}
-
 /// Read the frame info stored in the object, and emit the
 /// patched frame descriptions for the resulting file.
 ///
@@ -2256,25 +2238,6 @@ uint64_t DWARFLinker::DIECloner::cloneAllCompileUnits(
   return OutputDebugInfoSize - StartOutputDebugInfoSize;
 }
 
-void DWARFLinker::updateAccelKind(DWARFContext &Dwarf) {
-  if (Options.TheAccelTableKind != DwarfLinkerAccelTableKind::Default)
-    return;
-
-  auto &DwarfObj = Dwarf.getDWARFObj();
-
-  if (!AtLeastOneDwarfAccelTable &&
-      (!DwarfObj.getAppleNamesSection().Data.empty() ||
-       !DwarfObj.getAppleTypesSection().Data.empty() ||
-       !DwarfObj.getAppleNamespacesSection().Data.empty() ||
-       !DwarfObj.getAppleObjCSection().Data.empty())) {
-    AtLeastOneAppleAccelTable = true;
-  }
-
-  if (!AtLeastOneDwarfAccelTable && !DwarfObj.getNamesSection().Data.empty()) {
-    AtLeastOneDwarfAccelTable = true;
-  }
-}
-
 bool DWARFLinker::emitPaperTrailWarnings(const DWARFFile &File,
                                          OffsetsStringPool &StringPool) {
 
@@ -2357,8 +2320,6 @@ void DWARFLinker::addObjectFile(DWARFFile &File, objFileLoader Loader,
   ObjectContexts.emplace_back(LinkContext(File));
 
   if (ObjectContexts.back().File.Dwarf) {
-    updateAccelKind(*ObjectContexts.back().File.Dwarf);
-
     for (const std::unique_ptr<DWARFUnit> &CU :
          ObjectContexts.back().File.Dwarf->compile_units()) {
       DWARFDie CUDie = CU->getUnitDIE();
@@ -2392,19 +2353,6 @@ Error DWARFLinker::link() {
   // ODR Contexts for the optimize.
   DeclContextTree ODRContexts;
 
-  // If we haven't decided on an accelerator table kind yet, we base ourselves
-  // on the DWARF we have seen so far. At this point we haven't pulled in debug
-  // information from modules yet, so it is technically possible that they
-  // would affect the decision. However, as they're built with the same
-  // compiler and flags, it is safe to assume that they will follow the
-  // decision made here.
-  if (Options.TheAccelTableKind == DwarfLinkerAccelTableKind::Default) {
-    if (AtLeastOneDwarfAccelTable && !AtLeastOneAppleAccelTable)
-      Options.TheAccelTableKind = DwarfLinkerAccelTableKind::Dwarf;
-    else
-      Options.TheAccelTableKind = DwarfLinkerAccelTableKind::Apple;
-  }
-
   for (LinkContext &OptContext : ObjectContexts) {
     if (Options.Verbose) {
       if (DwarfLinkerClientID == DwarfLinkerClient::Dsymutil)
@@ -2624,25 +2572,22 @@ Error DWARFLinker::link() {
     if (!Options.NoOutput) {
       TheDwarfEmitter->emitAbbrevs(Abbreviations, Options.TargetDWARFVersion);
       TheDwarfEmitter->emitStrings(OffsetsStringPool);
-      switch (Options.TheAccelTableKind) {
-      case DwarfLinkerAccelTableKind::None:
-        // Nothing to do.
-        break;
-      case DwarfLinkerAccelTableKind::Apple:
-        TheDwarfEmitter->emitAppleNames(AppleNames);
-        TheDwarfEmitter->emitAppleNamespaces(AppleNamespaces);
-        TheDwarfEmitter->emitAppleTypes(AppleTypes);
-        TheDwarfEmitter->emitAppleObjc(AppleObjc);
-        break;
-      case DwarfLinkerAccelTableKind::Dwarf:
-        TheDwarfEmitter->emitDebugNames(DebugNames);
-        break;
-      case DwarfLinkerAccelTableKind::Pub:
-        // Already emitted by emitPubAcceleratorEntriesForUnit.
-        break;
-      case DwarfLinkerAccelTableKind::Default:
-        llvm_unreachable("Default should have already been resolved.");
-        break;
+      for (DwarfLinkerAccelTableKind TableKind : Options.AccelTables) {
+        switch (TableKind) {
+        case DwarfLinkerAccelTableKind::Apple:
+          TheDwarfEmitter->emitAppleNamespaces(AppleNamespaces);
+          TheDwarfEmitter->emitAppleNames(AppleNames);
+          TheDwarfEmitter->emitAppleTypes(AppleTypes);
+          TheDwarfEmitter->emitAppleObjc(AppleObjc);
+          break;
+        case DwarfLinkerAccelTableKind::Pub:
+          // Already emitted by emitAcceleratorEntriesForUnit.
+          // Already emitted by emitAcceleratorEntriesForUnit.
+          break;
+        case DwarfLinkerAccelTableKind::DebugNames:
+          TheDwarfEmitter->emitDebugNames(DebugNames);
+          break;
+        }
       }
     }
   };

diff  --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
index 208c2a7aa4973..ac140338020ae 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -579,7 +579,6 @@ bool DwarfLinkerForBinary::link(const DebugMap &Map) {
   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)
@@ -727,6 +726,27 @@ bool DwarfLinkerForBinary::link(const DebugMap &Map) {
   if (Error E = GeneralLinker.setTargetDWARFVersion(MaxDWARFVersion))
     return error(toString(std::move(E)));
 
+  switch (Options.TheAccelTableKind) {
+  case DsymutilAccelTableKind::Apple:
+    GeneralLinker.addAccelTableKind(DwarfLinkerAccelTableKind::Apple);
+    break;
+  case DsymutilAccelTableKind::Dwarf:
+    GeneralLinker.addAccelTableKind(DwarfLinkerAccelTableKind::DebugNames);
+    break;
+  case DsymutilAccelTableKind::Pub:
+    GeneralLinker.addAccelTableKind(DwarfLinkerAccelTableKind::Pub);
+    break;
+  case DsymutilAccelTableKind::Default:
+    if (MaxDWARFVersion >= 5)
+      GeneralLinker.addAccelTableKind(DwarfLinkerAccelTableKind::DebugNames);
+    else
+      GeneralLinker.addAccelTableKind(DwarfLinkerAccelTableKind::Apple);
+    break;
+  case DsymutilAccelTableKind::None:
+    // Nothing to do.
+    break;
+  }
+
   // link debug info for loaded object files.
   if (Error E = GeneralLinker.link())
     return error(toString(std::move(E)));

diff  --git a/llvm/tools/dsymutil/LinkUtils.h b/llvm/tools/dsymutil/LinkUtils.h
index 03b9a64b6bec5..da21d9bc94e71 100644
--- a/llvm/tools/dsymutil/LinkUtils.h
+++ b/llvm/tools/dsymutil/LinkUtils.h
@@ -23,6 +23,14 @@
 namespace llvm {
 namespace dsymutil {
 
+enum class DsymutilAccelTableKind : uint8_t {
+  None,
+  Apple,   ///< .apple_names, .apple_namespaces, .apple_types, .apple_objc.
+  Dwarf,   ///< DWARF v5 .debug_names.
+  Default, ///< Dwarf for DWARF5 or later, Apple otherwise.
+  Pub,     ///< .debug_pubnames, .debug_pubtypes
+};
+
 struct LinkOptions {
   /// Verbosity
   bool Verbose = false;
@@ -56,7 +64,7 @@ struct LinkOptions {
   OutputFileType FileType = OutputFileType::Object;
 
   /// The accelerator table kind
-  DwarfLinkerAccelTableKind TheAccelTableKind;
+  DsymutilAccelTableKind TheAccelTableKind;
 
   /// -oso-prepend-path
   std::string PrependPath;

diff  --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp
index 0743fc6d3b57d..5916df90cdf88 100644
--- a/llvm/tools/dsymutil/dsymutil.cpp
+++ b/llvm/tools/dsymutil/dsymutil.cpp
@@ -206,26 +206,26 @@ static Error verifyOptions(const DsymutilOptions &Options) {
   return Error::success();
 }
 
-static Expected<DwarfLinkerAccelTableKind>
+static Expected<DsymutilAccelTableKind>
 getAccelTableKind(opt::InputArgList &Args) {
   if (opt::Arg *Accelerator = Args.getLastArg(OPT_accelerator)) {
     StringRef S = Accelerator->getValue();
     if (S == "Apple")
-      return DwarfLinkerAccelTableKind::Apple;
+      return DsymutilAccelTableKind::Apple;
     if (S == "Dwarf")
-      return DwarfLinkerAccelTableKind::Dwarf;
+      return DsymutilAccelTableKind::Dwarf;
     if (S == "Pub")
-      return DwarfLinkerAccelTableKind::Pub;
+      return DsymutilAccelTableKind::Pub;
     if (S == "Default")
-      return DwarfLinkerAccelTableKind::Default;
+      return DsymutilAccelTableKind::Default;
     if (S == "None")
-      return DwarfLinkerAccelTableKind::None;
+      return DsymutilAccelTableKind::None;
     return make_error<StringError>("invalid accelerator type specified: '" + S +
                                        "'. Supported values are 'Apple', "
                                        "'Dwarf', 'Pub', 'Default' and 'None'.",
                                    inconvertibleErrorCode());
   }
-  return DwarfLinkerAccelTableKind::Default;
+  return DsymutilAccelTableKind::Default;
 }
 
 static Expected<ReproducerMode> getReproducerMode(opt::InputArgList &Args) {
@@ -310,7 +310,7 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
     }
   }
 
-  if (Expected<DwarfLinkerAccelTableKind> AccelKind = getAccelTableKind(Args)) {
+  if (Expected<DsymutilAccelTableKind> AccelKind = getAccelTableKind(Args)) {
     Options.LinkOpts.TheAccelTableKind = *AccelKind;
   } else {
     return AccelKind.takeError();

diff  --git a/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp b/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
index 3dafb62dc67cc..f06d8d038f650 100644
--- a/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
+++ b/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
@@ -297,7 +297,6 @@ Error linkDebugInfo(object::ObjectFile &File, const Options &Options,
   DWARFLinker DebugInfoLinker(&OutStreamer, DwarfLinkerClient::LLD);
 
   DebugInfoLinker.setEstimatedObjfilesAmount(1);
-  DebugInfoLinker.setAccelTableKind(DwarfLinkerAccelTableKind::None);
   DebugInfoLinker.setErrorHandler(ReportErr);
   DebugInfoLinker.setWarningHandler(ReportWarn);
   DebugInfoLinker.setNumThreads(Options.NumThreads);


        


More information about the llvm-commits mailing list