Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BOLT][DWARF] Update DW_AT_comp_dir/DW_AT_dwo_name for DWO TUs #91486

Merged
merged 4 commits into from
May 14, 2024

Conversation

ayermolo
Copy link
Contributor

@ayermolo ayermolo commented May 8, 2024

Type unit DIE generated by clang contains DW_AT_comp_dir/DW_AT_dwo_name. This was added to clang to help LLDB to figure out where type unit come from when accessing an entry in a .debug_names accelerator table and type units in .dwp file.

When BOLT writes out .dwo files it changes the name of them. User can also specify directory of where they can be written out. Added support to BOLT to update those attributes.

Type unit DIE generated by clang contains DW_AT_comp_dir/DW_AT_dwo_name. This
was added to clang to help LLDB to figure out where type unit come from when
accessing an entry in a .debug_names accelerator table and type units in .dwp file.

When BOLT writes out .dwo files it changes the name of them. User can also
specify directory of where they can be written out. Added support to BOLT to
update those attributes.
@llvmbot
Copy link
Collaborator

llvmbot commented May 8, 2024

@llvm/pr-subscribers-bolt

Author: Alexander Yermolovich (ayermolo)

Changes

Type unit DIE generated by clang contains DW_AT_comp_dir/DW_AT_dwo_name. This was added to clang to help LLDB to figure out where type unit come from when accessing an entry in a .debug_names accelerator table and type units in .dwp file.

When BOLT writes out .dwo files it changes the name of them. User can also specify directory of where they can be written out. Added support to BOLT to update those attributes.


Patch is 40.85 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91486.diff

9 Files Affected:

  • (modified) bolt/include/bolt/Core/DIEBuilder.h (+14)
  • (modified) bolt/include/bolt/Core/DebugData.h (+12-3)
  • (modified) bolt/include/bolt/Rewrite/DWARFRewriter.h (+7-2)
  • (modified) bolt/lib/Core/DIEBuilder.cpp (+87)
  • (modified) bolt/lib/Core/DebugData.cpp (+15-4)
  • (modified) bolt/lib/Rewrite/DWARFRewriter.cpp (+59-100)
  • (modified) bolt/test/X86/Inputs/dwarf5-df-types-debug-names-main.s (+11-11)
  • (added) bolt/test/X86/dwarf5-df-types-modify-dwo-name-mixed.test (+198)
  • (added) bolt/test/X86/dwarf5-df-types-modify-dwo-name.test (+175)
diff --git a/bolt/include/bolt/Core/DIEBuilder.h b/bolt/include/bolt/Core/DIEBuilder.h
index 06084819ec0b3..0686ec03f4afb 100644
--- a/bolt/include/bolt/Core/DIEBuilder.h
+++ b/bolt/include/bolt/Core/DIEBuilder.h
@@ -129,6 +129,9 @@ class DIEBuilder {
   uint64_t UnitSize{0};
   llvm::DenseSet<uint64_t> AllProcessed;
   DWARF5AcceleratorTable &DebugNamesTable;
+  // Unordered maps to handle name collision if output DWO directory is
+  // specified.
+  std::unordered_map<std::string, uint32_t> NameToIndexMap;
 
   /// Returns current state of the DIEBuilder
   State &getState() { return *BuilderState.get(); }
@@ -384,6 +387,17 @@ class DIEBuilder {
   bool deleteValue(DIEValueList *Die, dwarf::Attribute Attribute) {
     return Die->deleteValue(Attribute);
   }
+  /// Updates DWO Name and Compilation direcotry for Skeleton CU \p Unit.
+  std::string updateDWONameCompDir(DebugStrOffsetsWriter &StrOffstsWriter,
+                                   DebugStrWriter &StrWriter,
+                                   DWARFUnit &SkeletonCU,
+                                   std::optional<StringRef> DwarfOutputPath,
+                                   std::optional<StringRef> DWONameToUse);
+  /// Updates DWO Name and Compilation direcotry for Type Units.
+  void updateDWONameCompDirForTypes(DebugStrOffsetsWriter &StrOffstsWriter,
+                                    DebugStrWriter &StrWriter, DWARFUnit &Unit,
+                                    std::optional<StringRef> DwarfOutputPath,
+                                    const StringRef DWOName);
 };
 } // namespace bolt
 } // namespace llvm
diff --git a/bolt/include/bolt/Core/DebugData.h b/bolt/include/bolt/Core/DebugData.h
index 166bb3617e57f..b9e8c7583d9ad 100644
--- a/bolt/include/bolt/Core/DebugData.h
+++ b/bolt/include/bolt/Core/DebugData.h
@@ -430,7 +430,7 @@ class DebugAddrWriterDwarf5 : public DebugAddrWriter {
 using DebugStrOffsetsBufferVector = SmallVector<char, 16>;
 class DebugStrOffsetsWriter {
 public:
-  DebugStrOffsetsWriter() {
+  DebugStrOffsetsWriter(BinaryContext &BC) : BC(BC) {
     StrOffsetsBuffer = std::make_unique<DebugStrOffsetsBufferVector>();
     StrOffsetsStream = std::make_unique<raw_svector_ostream>(*StrOffsetsBuffer);
   }
@@ -460,6 +460,11 @@ class DebugStrOffsetsWriter {
     StrOffsets.clear();
   }
 
+  /// Returns True if StrOffsets Section was modified.
+  bool isStrOffsetsSectionModified() const {
+    return StrOffsetSectionWasModified;
+  }
+
 private:
   std::unique_ptr<DebugStrOffsetsBufferVector> StrOffsetsBuffer;
   std::unique_ptr<raw_svector_ostream> StrOffsetsStream;
@@ -467,13 +472,16 @@ class DebugStrOffsetsWriter {
   SmallVector<uint32_t, 5> StrOffsets;
   std::unordered_map<uint64_t, uint64_t> ProcessedBaseOffsets;
   bool StrOffsetSectionWasModified = false;
+  BinaryContext &BC;
 };
 
 using DebugStrBufferVector = SmallVector<char, 16>;
 class DebugStrWriter {
 public:
   DebugStrWriter() = delete;
-  DebugStrWriter(BinaryContext &BC) : BC(BC) { create(); }
+  DebugStrWriter(DWARFContext &DwCtx, bool IsDWO) : DwCtx(DwCtx), IsDWO(IsDWO) {
+    create();
+  }
   std::unique_ptr<DebugStrBufferVector> releaseBuffer() {
     return std::move(StrBuffer);
   }
@@ -495,7 +503,8 @@ class DebugStrWriter {
   void create();
   std::unique_ptr<DebugStrBufferVector> StrBuffer;
   std::unique_ptr<raw_svector_ostream> StrStream;
-  BinaryContext &BC;
+  DWARFContext &DwCtx;
+  bool IsDWO;
 };
 
 enum class LocWriterKind { DebugLocWriter, DebugLoclistWriter };
diff --git a/bolt/include/bolt/Rewrite/DWARFRewriter.h b/bolt/include/bolt/Rewrite/DWARFRewriter.h
index 12e0813d089d1..6b26ed9db5b28 100644
--- a/bolt/include/bolt/Rewrite/DWARFRewriter.h
+++ b/bolt/include/bolt/Rewrite/DWARFRewriter.h
@@ -203,13 +203,17 @@ class DWARFRewriter {
   using OverriddenSectionsMap = std::unordered_map<DWARFSectionKind, StringRef>;
   /// Output .dwo files.
   void writeDWOFiles(DWARFUnit &, const OverriddenSectionsMap &,
-                     const std::string &, DebugLocWriter &);
+                     const std::string &, DebugLocWriter &,
+                     DebugStrOffsetsWriter &, DebugStrWriter &);
   using KnownSectionsEntry = std::pair<MCSection *, DWARFSectionKind>;
   struct DWPState {
     std::unique_ptr<ToolOutputFile> Out;
     std::unique_ptr<BinaryContext> TmpBC;
     std::unique_ptr<MCStreamer> Streamer;
     std::unique_ptr<DWPStringPool> Strings;
+    /// This is used to store String sections for .dwo files if they are being
+    /// modified.
+    std::vector<std::unique_ptr<DebugBufferVector>> StrSections;
     const MCObjectFileInfo *MCOFI = nullptr;
     const DWARFUnitIndex *CUIndex = nullptr;
     std::deque<SmallString<32>> UncompressedSections;
@@ -230,7 +234,8 @@ class DWARFRewriter {
 
   /// add content of dwo to .dwp file.
   void updateDWP(DWARFUnit &, const OverriddenSectionsMap &, const UnitMeta &,
-                 UnitMetaVectorType &, DWPState &, DebugLocWriter &);
+                 UnitMetaVectorType &, DWPState &, DebugLocWriter &,
+                 DebugStrOffsetsWriter &, DebugStrWriter &);
 };
 
 } // namespace bolt
diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index c4b0b251c1201..f2b42c8147cdf 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -22,6 +22,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/LEB128.h"
 
 #include <algorithm>
@@ -41,6 +42,92 @@ extern cl::opt<unsigned> Verbosity;
 namespace llvm {
 namespace bolt {
 
+/// Returns DWO Name to be used. Handles case where user specifies output DWO
+/// directory, and there are duplicate names. Assumes DWO ID is unique.
+static std::string
+getDWOName(llvm::DWARFUnit &CU,
+           std::unordered_map<std::string, uint32_t> &NameToIndexMap,
+           std::optional<StringRef> &DwarfOutputPath) {
+  std::optional<uint64_t> DWOId = CU.getDWOId();
+  assert(DWOId && "DWO ID not found.");
+  (void)DWOId;
+
+  std::string DWOName = dwarf::toString(
+      CU.getUnitDIE().find({dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}),
+      "");
+  assert(!DWOName.empty() &&
+         "DW_AT_dwo_name/DW_AT_GNU_dwo_name does not exists.");
+  if (DwarfOutputPath) {
+    DWOName = std::string(sys::path::filename(DWOName));
+    auto Iter = NameToIndexMap.find(DWOName);
+    if (Iter == NameToIndexMap.end())
+      Iter = NameToIndexMap.insert({DWOName, 0}).first;
+    DWOName.append(std::to_string(Iter->second));
+    ++Iter->second;
+  }
+  DWOName.append(".dwo");
+  return DWOName;
+}
+
+/// Adds a \p Str to .debug_str section.
+/// Uses \p AttrInfoVal to either update entry in a DIE for legacy DWARF using
+/// \p DebugInfoPatcher, or for DWARF5 update an index in .debug_str_offsets
+/// for this contribution of \p Unit.
+static void addStringHelper(DebugStrOffsetsWriter &StrOffstsWriter,
+                            DebugStrWriter &StrWriter, DIEBuilder &DIEBldr,
+                            DIE &Die, const DWARFUnit &Unit,
+                            DIEValue &DIEAttrInfo, StringRef Str) {
+  uint32_t NewOffset = StrWriter.addString(Str);
+  if (Unit.getVersion() >= 5) {
+    StrOffstsWriter.updateAddressMap(DIEAttrInfo.getDIEInteger().getValue(),
+                                     NewOffset);
+    return;
+  }
+  DIEBldr.replaceValue(&Die, DIEAttrInfo.getAttribute(), DIEAttrInfo.getForm(),
+                       DIEInteger(NewOffset));
+}
+
+std::string DIEBuilder::updateDWONameCompDir(
+    DebugStrOffsetsWriter &StrOffstsWriter, DebugStrWriter &StrWriter,
+    DWARFUnit &SkeletonCU, std::optional<StringRef> DwarfOutputPath,
+    std::optional<StringRef> DWONameToUse) {
+  DIE &UnitDIE = *getUnitDIEbyUnit(SkeletonCU);
+  DIEValue DWONameAttrInfo = UnitDIE.findAttribute(dwarf::DW_AT_dwo_name);
+  if (!DWONameAttrInfo)
+    DWONameAttrInfo = UnitDIE.findAttribute(dwarf::DW_AT_GNU_dwo_name);
+  if (!DWONameAttrInfo)
+    return "";
+  std::string ObjectName;
+  if (DWONameToUse)
+    ObjectName = *DWONameToUse;
+  else
+    ObjectName = getDWOName(SkeletonCU, NameToIndexMap, DwarfOutputPath);
+  addStringHelper(StrOffstsWriter, StrWriter, *this, UnitDIE, SkeletonCU,
+                  DWONameAttrInfo, ObjectName);
+
+  DIEValue CompDirAttrInfo = UnitDIE.findAttribute(dwarf::DW_AT_comp_dir);
+  assert(CompDirAttrInfo && "DW_AT_comp_dir is not in Skeleton CU.");
+
+  if (DwarfOutputPath) {
+    if (!sys::fs::exists(*DwarfOutputPath))
+      sys::fs::create_directory(*DwarfOutputPath);
+    addStringHelper(StrOffstsWriter, StrWriter, *this, UnitDIE, SkeletonCU,
+                    CompDirAttrInfo, *DwarfOutputPath);
+  }
+  return ObjectName;
+}
+
+void DIEBuilder::updateDWONameCompDirForTypes(
+    DebugStrOffsetsWriter &StrOffstsWriter, DebugStrWriter &StrWriter,
+    DWARFUnit &Unit, std::optional<StringRef> DwarfOutputPath,
+    const StringRef DWOName) {
+  for (DWARFUnit *DU : getState().DWARF5TUVector)
+    updateDWONameCompDir(StrOffstsWriter, StrWriter, *DU, DwarfOutputPath,
+                         DWOName);
+  if (StrOffstsWriter.isStrOffsetsSectionModified())
+    StrOffstsWriter.finalizeSection(Unit, *this);
+}
+
 void DIEBuilder::updateReferences() {
   for (auto &[SrcDIEInfo, ReferenceInfo] : getState().AddrReferences) {
     DIEInfo *DstDIEInfo = ReferenceInfo.Dst;
diff --git a/bolt/lib/Core/DebugData.cpp b/bolt/lib/Core/DebugData.cpp
index a987a103a08b9..7bfd9e883c325 100644
--- a/bolt/lib/Core/DebugData.cpp
+++ b/bolt/lib/Core/DebugData.cpp
@@ -31,6 +31,7 @@
 #include <cstdint>
 #include <functional>
 #include <memory>
+#include <optional>
 #include <unordered_map>
 #include <vector>
 
@@ -867,10 +868,16 @@ void DebugStrOffsetsWriter::finalizeSection(DWARFUnit &Unit,
                                             DIEBuilder &DIEBldr) {
   std::optional<AttrInfo> AttrVal =
       findAttributeInfo(Unit.getUnitDIE(), dwarf::DW_AT_str_offsets_base);
-  if (!AttrVal)
+  if (!AttrVal && !Unit.isDWOUnit())
     return;
-  std::optional<uint64_t> Val = AttrVal->V.getAsSectionOffset();
-  assert(Val && "DW_AT_str_offsets_base Value not present.");
+  std::optional<uint64_t> Val = std::nullopt;
+  if (AttrVal) {
+    Val = AttrVal->V.getAsSectionOffset();
+  } else {
+    if (!Unit.isDWOUnit())
+      BC.errs() << "DW_AT_str_offsets_base Value not present.\n";
+    Val = 0;
+  }
   DIE &Die = *DIEBldr.getUnitDIEbyUnit(Unit);
   DIEValue StrListBaseAttrInfo =
       Die.findAttribute(dwarf::DW_AT_str_offsets_base);
@@ -915,7 +922,11 @@ void DebugStrWriter::create() {
 }
 
 void DebugStrWriter::initialize() {
-  auto StrSection = BC.DwCtx->getDWARFObj().getStrSection();
+  StringRef StrSection;
+  if (IsDWO)
+    StrSection = DwCtx.getDWARFObj().getStrDWOSection();
+  else
+    StrSection = DwCtx.getDWARFObj().getStrSection();
   (*StrStream) << StrSection;
 }
 
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 26e4889faadac..4ed3a507439eb 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -458,32 +458,6 @@ static std::optional<uint64_t> getAsAddress(const DWARFUnit &DU,
   return std::nullopt;
 }
 
-/// Returns DWO Name to be used. Handles case where user specifies output DWO
-/// directory, and there are duplicate names. Assumes DWO ID is unique.
-static std::string
-getDWOName(llvm::DWARFUnit &CU,
-           std::unordered_map<std::string, uint32_t> &NameToIndexMap) {
-  std::optional<uint64_t> DWOId = CU.getDWOId();
-  assert(DWOId && "DWO ID not found.");
-  (void)DWOId;
-
-  std::string DWOName = dwarf::toString(
-      CU.getUnitDIE().find({dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}),
-      "");
-  assert(!DWOName.empty() &&
-         "DW_AT_dwo_name/DW_AT_GNU_dwo_name does not exists.");
-  if (!opts::DwarfOutputPath.empty()) {
-    DWOName = std::string(sys::path::filename(DWOName));
-    auto Iter = NameToIndexMap.find(DWOName);
-    if (Iter == NameToIndexMap.end())
-      Iter = NameToIndexMap.insert({DWOName, 0}).first;
-    DWOName.append(std::to_string(Iter->second));
-    ++Iter->second;
-  }
-  DWOName.append(".dwo");
-  return DWOName;
-}
-
 static std::unique_ptr<DIEStreamer>
 createDIEStreamer(const Triple &TheTriple, raw_pwrite_stream &OutFile,
                   StringRef Swift5ReflectionSegmentName, DIEBuilder &DIEBldr,
@@ -515,7 +489,9 @@ static void emitDWOBuilder(const std::string &DWOName,
                            DIEBuilder &DWODIEBuilder, DWARFRewriter &Rewriter,
                            DWARFUnit &SplitCU, DWARFUnit &CU,
                            DWARFRewriter::DWPState &State,
-                           DebugLocWriter &LocWriter) {
+                           DebugLocWriter &LocWriter,
+                           DebugStrOffsetsWriter &StrOffstsWriter,
+                           DebugStrWriter &StrWriter) {
   // Populate debug_info and debug_abbrev for current dwo into StringRef.
   DWODIEBuilder.generateAbbrevs();
   DWODIEBuilder.finish();
@@ -577,54 +553,10 @@ static void emitDWOBuilder(const std::string &DWOName,
   }
   if (opts::WriteDWP)
     Rewriter.updateDWP(CU, OverriddenSections, CUMI, TUMetaVector, State,
-                       LocWriter);
+                       LocWriter, StrOffstsWriter, StrWriter);
   else
-    Rewriter.writeDWOFiles(CU, OverriddenSections, DWOName, LocWriter);
-}
-
-/// Adds a \p Str to .debug_str section.
-/// Uses \p AttrInfoVal to either update entry in a DIE for legacy DWARF using
-/// \p DebugInfoPatcher, or for DWARF5 update an index in .debug_str_offsets
-/// for this contribution of \p Unit.
-static void addStringHelper(DebugStrOffsetsWriter &StrOffstsWriter,
-                            DebugStrWriter &StrWriter, DIEBuilder &DIEBldr,
-                            DIE &Die, const DWARFUnit &Unit,
-                            DIEValue &DIEAttrInfo, StringRef Str) {
-  uint32_t NewOffset = StrWriter.addString(Str);
-  if (Unit.getVersion() >= 5) {
-    StrOffstsWriter.updateAddressMap(DIEAttrInfo.getDIEInteger().getValue(),
-                                     NewOffset);
-    return;
-  }
-  DIEBldr.replaceValue(&Die, DIEAttrInfo.getAttribute(), DIEAttrInfo.getForm(),
-                       DIEInteger(NewOffset));
-}
-
-static std::string
-updateDWONameCompDir(DebugStrOffsetsWriter &StrOffstsWriter,
-                     DebugStrWriter &StrWriter,
-                     std::unordered_map<std::string, uint32_t> &NameToIndexMap,
-                     DWARFUnit &Unit, DIEBuilder &DIEBldr, DIE &UnitDIE) {
-  DIEValue DWONameAttrInfo = UnitDIE.findAttribute(dwarf::DW_AT_dwo_name);
-  if (!DWONameAttrInfo)
-    DWONameAttrInfo = UnitDIE.findAttribute(dwarf::DW_AT_GNU_dwo_name);
-  assert(DWONameAttrInfo && "DW_AT_dwo_name is not in Skeleton CU.");
-  std::string ObjectName;
-
-  ObjectName = getDWOName(Unit, NameToIndexMap);
-  addStringHelper(StrOffstsWriter, StrWriter, DIEBldr, UnitDIE, Unit,
-                  DWONameAttrInfo, ObjectName.c_str());
-
-  DIEValue CompDirAttrInfo = UnitDIE.findAttribute(dwarf::DW_AT_comp_dir);
-  assert(CompDirAttrInfo && "DW_AT_comp_dir is not in Skeleton CU.");
-
-  if (!opts::DwarfOutputPath.empty()) {
-    if (!sys::fs::exists(opts::DwarfOutputPath))
-      sys::fs::create_directory(opts::DwarfOutputPath);
-    addStringHelper(StrOffstsWriter, StrWriter, DIEBldr, UnitDIE, Unit,
-                    CompDirAttrInfo, opts::DwarfOutputPath.c_str());
-  }
-  return ObjectName;
+    Rewriter.writeDWOFiles(CU, OverriddenSections, DWOName, LocWriter,
+                           StrOffstsWriter, StrWriter);
 }
 
 using DWARFUnitVec = std::vector<DWARFUnit *>;
@@ -673,9 +605,8 @@ void DWARFRewriter::updateDebugInfo() {
     return;
 
   ARangesSectionWriter = std::make_unique<DebugARangesSectionWriter>();
-  StrWriter = std::make_unique<DebugStrWriter>(BC);
-
-  StrOffstsWriter = std::make_unique<DebugStrOffsetsWriter>();
+  StrWriter = std::make_unique<DebugStrWriter>(*BC.DwCtx, false);
+  StrOffstsWriter = std::make_unique<DebugStrOffsetsWriter>(BC);
 
   if (!opts::DeterministicDebugInfo) {
     opts::DeterministicDebugInfo = true;
@@ -720,10 +651,6 @@ void DWARFRewriter::updateDebugInfo() {
     return LocListWritersByCU[CUIndex++].get();
   };
 
-  // Unordered maps to handle name collision if output DWO directory is
-  // specified.
-  std::unordered_map<std::string, uint32_t> NameToIndexMap;
-
   DWARF5AcceleratorTable DebugNamesTable(opts::CreateDebugNames, BC,
                                          *StrWriter);
   DWPState State;
@@ -747,13 +674,20 @@ void DWARFRewriter::updateDebugInfo() {
                                Unit);
       DWODIEBuilder.buildDWOUnit(**SplitCU);
       std::string DWOName = "";
+      std::optional<std::string> DwarfOutputPath =
+          opts::DwarfOutputPath.empty()
+              ? std::nullopt
+              : std::optional<std::string>(opts::DwarfOutputPath.c_str());
       {
         std::lock_guard<std::mutex> Lock(AccessMutex);
-        DWOName = updateDWONameCompDir(*StrOffstsWriter, *StrWriter,
-                                       NameToIndexMap, *Unit, *DIEBlder,
-                                       *DIEBlder->getUnitDIEbyUnit(*Unit));
+        DWOName = DIEBlder->updateDWONameCompDir(
+            *StrOffstsWriter, *StrWriter, *Unit, DwarfOutputPath, std::nullopt);
       }
-
+      DebugStrOffsetsWriter DWOStrOffstsWriter(BC);
+      DebugStrWriter DWOStrWriter((*SplitCU)->getContext(), true);
+      DWODIEBuilder.updateDWONameCompDirForTypes(DWOStrOffstsWriter,
+                                                 DWOStrWriter, **SplitCU,
+                                                 DwarfOutputPath, DWOName);
       DebugLoclistWriter DebugLocDWoWriter(*Unit, Unit->getVersion(), true);
       DebugRangesSectionWriter *TempRangesSectionWriter = RangesSectionWriter;
       if (Unit->getVersion() >= 5) {
@@ -771,7 +705,7 @@ void DWARFRewriter::updateDebugInfo() {
         TempRangesSectionWriter->finalizeSection();
 
       emitDWOBuilder(DWOName, DWODIEBuilder, *this, **SplitCU, *Unit, State,
-                     DebugLocDWoWriter);
+                     DebugLocDWoWriter, DWOStrOffstsWriter, DWOStrWriter);
     }
 
     if (Unit->getVersion() >= 5) {
@@ -1736,6 +1670,7 @@ std::optional<StringRef> updateDebugData(
     const DWARFUnitIndex::Entry *CUDWOEntry, uint64_t DWOId,
     std::unique_ptr<DebugBufferVector> &OutputBuffer,
     DebugRangeListsSectionWriter *RangeListsWriter, DebugLocWriter &LocWriter,
+    DebugStrOffsetsWriter &StrOffstsWriter, DebugStrWriter &StrWriter,
     const llvm::bolt::DWARFRewriter::OverriddenSectionsMap &OverridenSections) {
 
   using DWOSectionContribution =
@@ -1774,6 +1709,11 @@ std::optional<StringRef> updateDebugData(
     if (!SectionName.equals("debug_str.dwo"))
       errs() << "BOLT-WARNING: unsupported debug section: " << SectionName
              << "\n";
+    if (StrWriter.isInitialized()) {
+      OutputBuffer = StrWriter.releaseBuffer();
+      return StringRef(reinterpret_cast<const char *>(OutputBuffer->data()),
+                       OutputBuffer->size());
+    }
     return SectionContents;
   }
   case DWARFSectionKind::DW_SECT_INFO: {
@@ -1783,6 +1723,11 @@ std::optional<StringRef> updateDebugData(
     return getOverridenSection(DWARFSectionKind::DW_SECT_EXT_TYPES);
   }
   case DWARFSectionKind::DW_SECT_STR_OFFSETS: {
+    if (StrOffstsWriter.isFinalized()) {
+      OutputBuffer = StrOffstsWriter.releaseBuffer();
+      return StringRef(reinterpret_cast<const char *>(OutputBuffer->data()),
+                       OutputBuffer->size());
+    }
     return getSliceData(CUDWOEntry, SectionContents,
                         DWARFSectionKind::DW_SECT_STR_OFFSETS, DWPOffset);
   }
@@ -1884,7 +1829,9 @@ void DWARFRewriter::updateDWP(DWARFUnit &CU,
                               const OverriddenSectionsMap &OverridenSections,
                               const DWARFRewriter::UnitMeta &CUMI,
                               DWARFRewriter::UnitMetaVectorType &TUMetaVector,
-                              DWPState &State, DebugLocWriter &LocWriter) {
+                              DWPState &State, DebugLocWriter &LocWriter,
+                              DebugStrOffsetsWriter &StrOffstsWriter,
+                              DebugStrWriter &StrWriter) {
   const uint64_t DWOId = *CU.getDWOId();
   MCSection *const StrOffsetSection = S...
[truncated]

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of comments.

bolt/include/bolt/Core/DIEBuilder.h Outdated Show resolved Hide resolved
bolt/include/bolt/Core/DIEBuilder.h Show resolved Hide resolved
bolt/include/bolt/Core/DebugData.h Outdated Show resolved Hide resolved
bolt/include/bolt/Rewrite/DWARFRewriter.h Outdated Show resolved Hide resolved
@@ -41,6 +42,92 @@ extern cl::opt<unsigned> Verbosity;
namespace llvm {
namespace bolt {

/// Returns DWO Name to be used. Handles case where user specifies output DWO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be used for what? Maybe you can reword this comment a bit.

bolt/include/bolt/Core/DIEBuilder.h Outdated Show resolved Hide resolved
bolt/lib/Core/DIEBuilder.cpp Outdated Show resolved Hide resolved
bolt/lib/Core/DIEBuilder.cpp Outdated Show resolved Hide resolved
@maksfb maksfb changed the title [BOLT][DWARF] Updated DW_AT_comp_dir/DW_AT_dwo_name for DWO TUs. [BOLT][DWARF] Update DW_AT_comp_dir/DW_AT_dwo_name for DWO TUs May 14, 2024
bolt/test/X86/dwarf5-df-types-modify-dwo-name-mixed.test Outdated Show resolved Hide resolved
bolt/lib/Core/DebugData.cpp Outdated Show resolved Hide resolved
bolt/test/X86/dwarf5-df-types-modify-dwo-name-mixed.test Outdated Show resolved Hide resolved
; BOLT-PATH: DW_TAG_compile_unit
; BOLT-PATH: .debug_str_offsets.dwo contents:
; BOLT-PATH-NEXT: 0x00000000: Contribution size = 68, Format = DWARF32, Version = 5
; BOLT-PATH-NEXT: "main"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to check all strings or file names should be enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think testing all strings makes a more complete test. In case we corrupt others somehow.

@ayermolo ayermolo merged commit 99fad7e into llvm:main May 14, 2024
4 checks passed
; BOLT-NEXT: "c2"
; BOLT-NEXT: "c3"
; BOLT-NEXT: "Foo2a"
; BOLT-NEXT: "clang version 18.0.0git (git@github.com:ayermolo/llvm-project.git db35fa8fc524127079662802c4735dbf397f86d0)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcoded version should fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because input is assembly.

; BOLT-NEXT: "char"
; BOLT-NEXT: "c3"
; BOLT-NEXT: "Foo2a"
; BOLT-NEXT: "clang version 18.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too

; BOLT-DWP-NEXT: "c2"
; BOLT-DWP-NEXT: "c3"
; BOLT-DWP-NEXT: "Foo2a"
; BOLT-DWP-NEXT: "clang version 18.0.0git (git@github.com:ayermolo/llvm-project.git db35fa8fc524127079662802c4735dbf397f86d0)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

; BOLT-DWP-NEXT: "char"
; BOLT-DWP-NEXT: "c3"
; BOLT-DWP-NEXT: "Foo2a"
; BOLT-DWP-NEXT: "clang version 18.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the same checks here and in other files.

; BOLT-DWP: DW_TAG_compile_unit
; BOLT-DWP: DW_AT_dwo_name ("main.dwo.dwo")
; BOLT-DWP: DW_TAG_type_unit
; BOLT-DW-NOT: DW_AT_dwo_name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

; BOLT-DWP-NOT: DW_AT_dwo_name typo in filecheck annotation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants