[BOLT][DWARF][NFC] Fix DebugStrOffsetsWriter

[sayhaan]

Fix DebugStrOffsetsWriter so updateAddressMap can't be called after it is finalized.

[llvmbot]

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-bolt

Author: Sayhaan Siddiqui (sayhaan)

Changes

Fix DebugStrOffsetsWriter so updateAddressMap can't be called after it is finalized.

---

Full diff: https://github.com/llvm/llvm-project/pull/100672.diff

5 Files Affected:

• (modified) bolt/lib/Core/DIEBuilder.cpp (+2)
• (modified) bolt/lib/Core/DebugData.cpp (+1)
• (modified) bolt/lib/Rewrite/DWARFRewriter.cpp (+2)
• (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h (+5-1)
• (modified) llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp (+4-2)

diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index b0f550fd77318..eb5b6ed31313f 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -77,6 +77,8 @@ static void addStringHelper(DebugStrOffsetsWriter &StrOffstsWriter,
                             DIEValue &DIEAttrInfo, StringRef Str) {
   uint32_t NewOffset = StrWriter.addString(Str);
   if (Unit.getVersion() >= 5) {
+    assert(!Unit.isDebugStrOffsetFinalized() &&
+           "debug_str_offsets was already finalized.");
     StrOffstsWriter.updateAddressMap(DIEAttrInfo.getDIEInteger().getValue(),
                                      NewOffset);
     return;
diff --git a/bolt/lib/Core/DebugData.cpp b/bolt/lib/Core/DebugData.cpp
index 002f58c474346..e96656687fdfe 100644
--- a/bolt/lib/Core/DebugData.cpp
+++ b/bolt/lib/Core/DebugData.cpp
@@ -906,6 +906,7 @@ void DebugStrOffsetsWriter::finalizeSection(DWARFUnit &Unit,
   }
 
   StrOffsetSectionWasModified = false;
+  Unit.setDebugStrOffsetFinalized();
   clear();
 }
 
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 674b5f17adb3f..adf4c7a32e579 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -757,6 +757,8 @@ void DWARFRewriter::updateDebugInfo() {
               : std::optional<std::string>(opts::DwarfOutputPath.c_str());
       std::string DWOName = DIEBlder.updateDWONameCompDir(
           *StrOffstsWriter, *StrWriter, *CU, DwarfOutputPath, std::nullopt);
+      if (CU->getVersion() >= 5)
+        StrOffstsWriter->finalizeSection(*CU, DIEBlder);
       processSplitCU(*CU, **SplitCU, DIEBlder, *TempRangesSectionWriter,
                      AddressWriter, DWOName, DwarfOutputPath);
     }
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 80c27aea89312..2642d13b16e4c 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -230,6 +230,7 @@ class DWARFUnit {
   std::optional<uint64_t> AddrOffsetSectionBase;
   bool IsLittleEndian;
   bool IsDWO;
+  bool IsDebugStrOffsetFinalized;
   const DWARFUnitVector &UnitVector;
 
   /// Start, length, and DWARF format of the unit's contribution to the string
@@ -310,12 +311,15 @@ class DWARFUnit {
             const DWARFSection *RS, const DWARFSection *LocSection,
             StringRef SS, const DWARFSection &SOS, const DWARFSection *AOS,
             const DWARFSection &LS, bool LE, bool IsDWO,
-            const DWARFUnitVector &UnitVector);
+            const DWARFUnitVector &UnitVector,
+            bool IsDebugStrOffsetFinalized = false);
 
   virtual ~DWARFUnit();
 
   bool isLittleEndian() const { return IsLittleEndian; }
   bool isDWOUnit() const { return IsDWO; }
+  bool isDebugStrOffsetFinalized() const { return IsDebugStrOffsetFinalized; }
+  void setDebugStrOffsetFinalized() { IsDebugStrOffsetFinalized = true; }
   DWARFContext& getContext() const { return Context; }
   const DWARFSection &getInfoSection() const { return InfoSection; }
   uint64_t getOffset() const { return Header.getOffset(); }
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index bdd04b00f557b..d2143772946c3 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -196,11 +196,13 @@ DWARFUnit::DWARFUnit(DWARFContext &DC, const DWARFSection &Section,
                      const DWARFSection *RS, const DWARFSection *LocSection,
                      StringRef SS, const DWARFSection &SOS,
                      const DWARFSection *AOS, const DWARFSection &LS, bool LE,
-                     bool IsDWO, const DWARFUnitVector &UnitVector)
+                     bool IsDWO, const DWARFUnitVector &UnitVector,
+                     bool IsDebugStrOffsetFinalized)
     : Context(DC), InfoSection(Section), Header(Header), Abbrev(DA),
       RangeSection(RS), LineSection(LS), StringSection(SS),
       StringOffsetSection(SOS), AddrOffsetSection(AOS), IsLittleEndian(LE),
-      IsDWO(IsDWO), UnitVector(UnitVector) {
+      IsDWO(IsDWO), IsDebugStrOffsetFinalized(IsDebugStrOffsetFinalized),
+      UnitVector(UnitVector) {
   clear();
 }
 

[ayermolo]

The check is BOLT specific, please do not modify DWARFUnit.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[ayermolo]

Add/modify a test.

[ayermolo]

For the test, all we care about are strings (since that's what bug is about). Please reduce the test, and name it more appropriately.

[ayermolo]

LGTM
Check internal diff passes before merging.