[DWARFLinker][NFC] Set the target DWARF version explicitly.

[avl]

Summary

Currently, DWARFLinker determines the target DWARF version internally.
It examines incoming object files, detects maximal
DWARF version and uses that version for the output file.
This patch allows explicitly setting output DWARF version by the consumer
of DWARFLinker. So that DWARFLinker uses a specified version instead
of autodetected one. It allows consumers to use different logic for
setting the target DWARF version. f.e. instead of the maximally used version
someone could set a higher version to convert from DWARFv4 to DWARFv5
(This possibility is not supported yet, but it would be good if
the interface will support it). Or another variant is to set the target
version through the command line. This patch moves the autodetection into the
consumers(DwarfLinkerForBinary.cpp, DebugInfoLinker.cpp).

[JDevlieghere]

However, as they're built with the same compiler and flags, it is safe to assume that they will follow the decision made here.

When using static libraries, that's far from a guarantee.

[avl]

In D132755#3752563, @JDevlieghere wrote:

However, as they're built with the same compiler and flags, it is safe to assume that they will follow the decision made here.

When using static libraries, that's far from a guarantee.

That assumption is currently used in DWARFLinker. If we would like to remove it and use another - I will prepare separate review which will also examine modules.

[avl]

rebased. Returned check for modules version back.

[avl]

@JDevlieghere ping.

[dblaikie]

What's the motivation for this, exactly? It seems weird to use this tool as a way to upgrade DWARF (& sort of weird to upgrade DWARF after-the-fact anyway, why not generate the desired DWARF in the first place?)

[avl]

In D132755#3778058, @dblaikie wrote:

What's the motivation for this, exactly?

The main motivation is to have the possibility to set conversion options explicitly. f.e. currently, Target dwarf version is detected by DWARFLinker internally, type of accelerator table is also detected internally. The caller of DWARFLinker does not know the state of these options. So it is hard to implement handling depending on these implicit options. f.e. If the caller would like to set "Apple" accelerator table for DWARFv4 and "DebugNames" for DWARFv5 - it could not be done as the caller does not know the target version. It is also not good to put that handling(setting type of accelerator table based on DWARF version) inside DWARFLinker as different callers of DWARFLinker may have different requirements. So the main motivation for that change is to set options explicitly, to have a possibility to implement different behaviors in different callers.

It seems weird to use this tool as a way to upgrade DWARF (& sort of weird to upgrade DWARF after-the-fact anyway, why not generate the desired DWARF in the first place?)

It looks to me, that feature of converting to different DWARF versions might be good. We can have a backup of binaries compiled with DWARF4. It might be useful to convert them into the DWARF5 without recompiling. That feature is not supported by the current DWARFLinker, but it might be useful. This interface change makes it possible to implement that behavior in the future. Though it is not the main motivation.

[dblaikie]

Mixed feelings, but the indexing situation's somewhat motivational - though I think it'd be OK for it to be an error if you asked for DWARFv5 accelerator tables but the contents was producing DWARFv4 (or to have the request for DWARFv5 accelerator tables to up the version to DWARFv5 - though that wouldn't need a user-visible option to explicitly ask for DWARFv5)

But don't mind me - was curious, but not a blocker.

[avl]

In D132755#3779145, @dblaikie wrote:

Mixed feelings, but the indexing situation's somewhat motivational - though I think it'd be OK for it to be an error if you asked for DWARFv5 accelerator tables but the contents was producing DWARFv4 (or to have the request for DWARFv5 accelerator tables to up the version to DWARFv5 - though that wouldn't need a user-visible option to explicitly ask for DWARFv5)

The good thing about interface changes from this patch is that it allows implementing any strategy(since option is set explicitly). It may be done "OK for it to be an error if you asked for DWARFv5 accelerator tables but the contents was producing DWARFv4", or it could be done "request for DWARFv5 accelerator tables to up the version to DWARFv5", or "set Apple for DWARFv4, set DebugNames for DWARF5 ", or anything else.

[avl]

ping.

[JDevlieghere]

In D132755#3779145, @dblaikie wrote:

Mixed feelings, but the indexing situation's somewhat motivational - though I think it'd be OK for it to be an error if you asked for DWARFv5 accelerator tables but the contents was producing DWARFv4 (or to have the request for DWARFv5 accelerator tables to up the version to DWARFv5 - though that wouldn't need a user-visible option to explicitly ask for DWARFv5)

But don't mind me - was curious, but not a blocker.

This support exists because dsymutil has the ability to upgrade the accelerator tables. IIRC Greg added this when he designed the Apple accelerator tables. I extended that support when we added support for .debug_names and used that to test LLDB. So this is definitely intentional.

Things are easy when the user explicitly asked for a given accelerator type. I think the same thing makes sense for the DWARF version which is what this patch is doing.

[JDevlieghere]

LGTM

[avl]

Thank you for the review!

llvm/include/llvm/DWARFLinker/DWARFLinker.h

void setSwiftInterfacesMap(swiftInterfacesMap *Map) {
     Options.ParseableSwiftInterfaces = Map;
   }
 
   /// Set prefix map for objects.
   void setObjectPrefixMap(objectPrefixMap *Map) {
     Options.ObjectPrefixMap = Map;
   }
 
+  /// Set target DWARF version.
+  Error setTargetDWARFVersion(uint16_t TargetDWARFVersion) {
+    if (TargetDWARFVersion < 1 || TargetDWARFVersion > 5)
+      return createStringError(std::errc::invalid_argument,
+                               "unsupported DWARF version: %d",
+                               TargetDWARFVersion);
+
+    Options.TargetDWARFVersion = TargetDWARFVersion;
+    return Error::success();
+  }
+
 private:
   /// Flags passed to DwarfLinker::lookForDIEsToKeep
   enum TraversalFlags {
     TF_Keep = 1 << 0,            ///< Mark the traversed DIEs as kept.
     TF_InFunctionScope = 1 << 1, ///< Current scope is a function scope.
     TF_DependencyWalk = 1 << 2,  ///< Walking the dependencies of a kept DIE.
     TF_ParentWalk = 1 << 3,      ///< Walking up the parents of a kept DIE.
     TF_ODR = 1 << 4,             ///< Use the ODR while keeping dependents.
private:
   }
 
   void reportError(const Twine &Warning, const DWARFFile &File,
                    const DWARFDie *DIE = nullptr) const {
     if (Options.ErrorHandler != nullptr)
       Options.ErrorHandler(Warning, File.FileName, DIE);
   }
 
-  /// Remembers the oldest and newest DWARF version we've seen in a unit.
-  void updateDwarfVersion(unsigned Version) {
-    MaxDwarfVersion = std::max(MaxDwarfVersion, Version);
-    MinDwarfVersion = std::min(MinDwarfVersion, Version);
-  }
-
   /// 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);
 
   void copyInvariantDebugSection(DWARFContext &Dwarf);
private:
 
   /// Allocator used for all the DIEValue objects.
   BumpPtrAllocator DIEAlloc;
   /// @}
 
   DwarfEmitter *TheDwarfEmitter;
   std::vector<LinkContext> ObjectContexts;
 
-  unsigned MaxDwarfVersion = 0;
-  unsigned MinDwarfVersion = std::numeric_limits<unsigned>::max();
-
   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 different object files.
   StringMap<uint32_t> EmittedCIEs;
 
private:
 
   std::function<StringRef(StringRef)> StringsTranslator = nullptr;
 
   /// A unique ID that identifies each compile unit.
   unsigned UniqueUnitID = 0;
 
   /// linking options
   struct DWARFLinkerOptions {
+    /// DWARF version for the output.
+    uint16_t TargetDWARFVersion = 0;
+
     /// Generate processing log to the standard output.
     bool Verbose = false;
 
     /// Print statistics.
     bool Statistics = false;
 
     /// Verify the input DWARF.
     bool VerifyInputDWARF = false;

llvm/lib/DWARFLinker/DWARFLinker.cpp

Error DWARFLinker::loadClangModule(objFileLoader Loader, const DWARFDie &CUDie,
 
   auto ErrOrObj = Loader(Context.File.FileName, Path);
   if (!ErrOrObj)
     return Error::success();
 
   std::unique_ptr<CompileUnit> Unit;
   for (const auto &CU : ErrOrObj->Dwarf->compile_units()) {
     OnCUDieLoaded(*CU);
-    updateDwarfVersion(CU->getVersion());
     // Recursively get all modules imported by this one.
     auto ChildCUDie = CU->getUnitDIE();
     if (!ChildCUDie)
       continue;
     if (!registerModuleReference(ChildCUDie, Context, Loader, OnCUDieLoaded,
                                  Indent)) {
       if (Unit) {
         std::string Err =
for (const std::unique_ptr<DWARFUnit> &CU :
         registerModuleReference(CUDie, ObjectContexts.back(), Loader,
                                 OnCUDieLoaded);
     }
   }
 }
 
 Error DWARFLinker::link() {
   assert(Options.NoOutput || TheDwarfEmitter);
+  assert((Options.TargetDWARFVersion != 0) &&
+         "TargetDWARFVersion should be set");
 
   // First populate the data structure we need for each iteration of the
   // parallel loop.
   unsigned NumObjects = ObjectContexts.size();
 
   // This Dwarf string pool which is used for emission. It must be used
   // serially as the order of calling getStringOffset matters for
   // reproducibility.
if (OptContext.File.Dwarf->getDWARFObj().getMacinfoSection().size() > 0) {
       }
     }
 
     // In a first phase, just read in the debug info and load all clang modules.
     OptContext.CompileUnits.reserve(
         OptContext.File.Dwarf->getNumCompileUnits());
 
     for (const auto &CU : OptContext.File.Dwarf->compile_units()) {
-      updateDwarfVersion(CU->getVersion());
       auto CUDie = CU->getUnitDIE(false);
       if (Options.Verbose) {
         outs() << "Input compilation unit:";
         DIDumpOptions DumpOpts;
         DumpOpts.ChildRecurseDepth = 0;
         DumpOpts.Verbose = Options.Verbose;
         CUDie.dump(outs(), 0, DumpOpts);
       }
     }
 
     for (auto &CU : OptContext.ModuleUnits) {
       if (Error Err =
               cloneModuleUnit(OptContext, CU, ODRContexts, OffsetsStringPool))
         reportWarning(toString(std::move(Err)), CU.File);
     }
   }
 
-  // If we haven't seen any CUs, pick an arbitrary valid Dwarf version anyway.
-  if (MaxDwarfVersion == 0)
-    MaxDwarfVersion = 3;
-
   // At this point we know how much data we have emitted. We use this value to
   // compare canonical DIE offsets in analyzeContextInfo to see if a definition
   // is already emitted, without being affected by canonical die offsets set
   // later. This prevents undeterminism when analyze and clone execute
   // concurrently, as clone set the canonical DIE offset and analyze reads it.
   const uint64_t ModulesEndOffset =
       Options.NoOutput ? 0 : TheDwarfEmitter->getDebugInfoSectionSize();
 
   // These variables manage the list of processed object files.
   // The mutex and condition variable are to ensure that this is thread safe.
   std::mutex ProcessedFilesMutex;
   std::condition_variable ProcessedFilesConditionVariable;
   BitVector ProcessedFiles(NumObjects, false);
 
   //  Analyzing the context info is particularly expensive so it is executed in
   //  parallel with emitting the previous compile unit.
   auto AnalyzeLambda = [&](size_t I) {
     auto &Context = ObjectContexts[I];
 
     if (Context.Skip || !Context.File.Dwarf)
       return;
 
     for (const auto &CU : Context.File.Dwarf->compile_units()) {
-      updateDwarfVersion(CU->getVersion());
       // The !isClangModuleRef condition effectively skips over fully resolved
       // skeleton units.
       auto CUDie = CU->getUnitDIE();
       std::string PCMFile = getPCMFile(CUDie, Options.ObjectPrefixMap);
 
       if (!CUDie || LLVM_UNLIKELY(Options.Update) ||
           !isClangModuleRef(CUDie, PCMFile, Context, 0, true).first) {
         Context.CompileUnits.push_back(std::make_unique<CompileUnit>(
auto CloneLambda = [&](size_t I) {
 
     // Clean-up before starting working on the next object.
     cleanupAuxiliarryData(OptContext);
   };
 
   auto EmitLambda = [&]() {
     // Emit everything that's global.
     if (!Options.NoOutput) {
-      TheDwarfEmitter->emitAbbrevs(Abbreviations, MaxDwarfVersion);
+      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);

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

bool DwarfLinkerForBinary::link(const DebugMap &Map) {
   if (!Options.NoOutput && !ReflectionSectionsPresentInBinary) {
     auto SectionToOffsetInDwarf =
         calculateStartOfStrippableReflectionSections(Map);
     for (const auto &Obj : Map.objects()) 
       copySwiftReflectionMetadata(Obj.get(), Streamer.get(),
                                   SectionToOffsetInDwarf, RelocationsToApply);
   }
 
+  uint16_t MaxDWARFVersion = 0;
+  std::function<void(const DWARFUnit &Unit)> OnCUDieLoaded =
+      [&MaxDWARFVersion](const DWARFUnit &Unit) {
+        MaxDWARFVersion = std::max(Unit.getVersion(), MaxDWARFVersion);
+      };
+
   for (const auto &Obj : Map.objects()) {
     // N_AST objects (swiftmodule files) should get dumped directly into the
     // appropriate DWARF section.
     if (Obj->getType() == MachO::N_AST) {
       if (Options.Verbose)
         outs() << "DEBUG MAP OBJECT: " << Obj->getObjectFilename() << "\n";
 
       StringRef File = Obj->getObjectFilename();
if (Obj->getType() == MachO::N_AST) {
       // Copy the module into the .swift_ast section.
       if (!Options.NoOutput)
         Streamer->emitSwiftAST((*ErrorOrMem)->getBuffer());
 
       continue;
     }
 
     if (auto ErrorOrObj = loadObject(*Obj, Map, RL))
-      GeneralLinker.addObjectFile(*ErrorOrObj, Loader);
+      GeneralLinker.addObjectFile(*ErrorOrObj, Loader, OnCUDieLoaded);
     else {
       ObjectsForLinking.push_back(std::make_unique<DWARFFile>(
           Obj->getObjectFilename(), nullptr, nullptr,
           Obj->empty() ? Obj->getWarnings() : EmptyWarnings));
       GeneralLinker.addObjectFile(*ObjectsForLinking.back());
     }
   }
 
+  // If we haven't seen any CUs, pick an arbitrary valid Dwarf version anyway.
+  if (MaxDWARFVersion == 0)
+    MaxDWARFVersion = 3;
+
+  if (Error E = GeneralLinker.setTargetDWARFVersion(MaxDWARFVersion))
+    return error(toString(std::move(E)));
+
   // link debug info for loaded object files.
   if (Error E = GeneralLinker.link())
     return error(toString(std::move(E)));
 
   StringRef ArchName = Map.getTriple().getArchName();
   if (Error E = emitRemarks(Options, Map.getBinaryPath(), ArchName, RL))
     return error(toString(std::move(E)));
 

llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp

Error linkDebugInfo(object::ObjectFile &File, const Options &Options,
   DwarfStreamer OutStreamer(OutputFileType::Object, OutStream, nullptr,
                             ReportWarn, ReportWarn);
   Triple TargetTriple = File.makeTriple();
   if (!OutStreamer.init(TargetTriple, formatv("cannot create a stream for {0}",
                                               TargetTriple.getTriple())
                                           .str()))
     return createStringError(std::errc::invalid_argument, "");
 
+  std::unique_ptr<DWARFContext> Context = DWARFContext::create(File);
+
+  uint16_t MaxDWARFVersion = 0;
+  std::function<void(const DWARFUnit &Unit)> OnCUDieLoaded =
+      [&MaxDWARFVersion](const DWARFUnit &Unit) {
+        MaxDWARFVersion = std::max(Unit.getVersion(), MaxDWARFVersion);
+      };
+
   // Create DWARF linker.
   DWARFLinker DebugInfoLinker(&OutStreamer, DwarfLinkerClient::LLD);
 
   DebugInfoLinker.setEstimatedObjfilesAmount(1);
   DebugInfoLinker.setAccelTableKind(DwarfLinkerAccelTableKind::None);
   DebugInfoLinker.setErrorHandler(ReportErr);
   DebugInfoLinker.setWarningHandler(ReportWarn);
   DebugInfoLinker.setNumThreads(Options.NumThreads);
   DebugInfoLinker.setNoODR(!Options.DoODRDeduplication);
   DebugInfoLinker.setVerbosity(Options.Verbose);
   DebugInfoLinker.setUpdate(!Options.DoGarbageCollection);
 
   std::vector<std::unique_ptr<DWARFFile>> ObjectsForLinking(1);
   std::vector<std::unique_ptr<AddressesMap>> AddresssMapForLinking(1);
   std::vector<std::string> EmptyWarnings;
 
-  std::unique_ptr<DWARFContext> Context = DWARFContext::create(File);
-
   // Unknown debug sections would be removed. Display warning
   // for such sections.
   for (SectionName Sec : Context->getDWARFObj().getSectionNames()) {
     if (isDebugSection(Sec.Name) && !knownByDWARFUtil(Sec.Name))
       warning(
           formatv("'{0}' is not currently supported: section will be skipped",
                   Sec.Name),
           Options.InputFileName);
   }
 
   // Add object files to the DWARFLinker.
   AddresssMapForLinking[0] =
       std::make_unique<ObjFileAddressMap>(*Context, Options, File);
 
   ObjectsForLinking[0] = std::make_unique<DWARFFile>(
       File.getFileName(), &*Context, AddresssMapForLinking[0].get(),
       EmptyWarnings);
 
   for (size_t I = 0; I < ObjectsForLinking.size(); I++)
-    DebugInfoLinker.addObjectFile(*ObjectsForLinking[I]);
+    DebugInfoLinker.addObjectFile(*ObjectsForLinking[I], nullptr,
+                                  OnCUDieLoaded);
+
+  // If we haven't seen any CUs, pick an arbitrary valid Dwarf version anyway.
+  if (MaxDWARFVersion == 0)
+    MaxDWARFVersion = 3;
+
+  if (Error Err = DebugInfoLinker.setTargetDWARFVersion(MaxDWARFVersion))
+    return Err;
 
   // Link debug info.
   if (Error Err = DebugInfoLinker.link())
     return Err;
 
   OutStreamer.finish();
   return Error::success();
 }
 
 } // end of namespace dwarfutil
 } // end of namespace llvm