[llvm] [BOLT][DWARF][NFC] Fix DebugStrOffsetsWriter (PR #100672)

Sayhaan Siddiqui via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 25 20:54:28 PDT 2024


https://github.com/sayhaan updated https://github.com/llvm/llvm-project/pull/100672

>From f7d4d240ee6c2870169dd8287529df1328a231ce Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Thu, 25 Jul 2024 17:01:38 -0700
Subject: [PATCH 1/4] [BOLT][DWARF][NFC] Fix DebugStrOffsetsWriter so
 updateAddressMap can't be called after it is finalized

---
 bolt/lib/Core/DIEBuilder.cpp                  | 1 +
 bolt/lib/Core/DebugData.cpp                   | 1 +
 llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | 6 +++++-
 llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp        | 6 ++++--
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index b0f550fd77318..23e06e2010f97 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -77,6 +77,7 @@ 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/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();
 }
 

>From 9027af9312a6a97725405d89fef5c55e6f440e4e Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Thu, 25 Jul 2024 17:03:13 -0700
Subject: [PATCH 2/4] Formatting changes

---
 bolt/lib/Core/DIEBuilder.cpp       | 3 ++-
 bolt/lib/Rewrite/DWARFRewriter.cpp | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index 23e06e2010f97..eb5b6ed31313f 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -77,7 +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.");
+    assert(!Unit.isDebugStrOffsetFinalized() &&
+           "debug_str_offsets was already finalized.");
     StrOffstsWriter.updateAddressMap(DIEAttrInfo.getDIEInteger().getValue(),
                                      NewOffset);
     return;
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 674b5f17adb3f..9563c5e45546a 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -757,8 +757,10 @@ 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);
+                      AddressWriter, DWOName, DwarfOutputPath);
     }
     for (DWARFUnit *CU : DIEBlder.getProcessedCUs())
       processMainBinaryCU(*CU, DIEBlder);

>From 26c92a9330029d1c12018b21b80081d800e5e09f Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Thu, 25 Jul 2024 17:05:39 -0700
Subject: [PATCH 3/4] Formatting changes

---
 bolt/lib/Rewrite/DWARFRewriter.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 9563c5e45546a..adf4c7a32e579 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -760,7 +760,7 @@ void DWARFRewriter::updateDebugInfo() {
       if (CU->getVersion() >= 5)
         StrOffstsWriter->finalizeSection(*CU, DIEBlder);
       processSplitCU(*CU, **SplitCU, DIEBlder, *TempRangesSectionWriter,
-                      AddressWriter, DWOName, DwarfOutputPath);
+                     AddressWriter, DWOName, DwarfOutputPath);
     }
     for (DWARFUnit *CU : DIEBlder.getProcessedCUs())
       processMainBinaryCU(*CU, DIEBlder);

>From 9bb80a70c3690e7763e01a5f67c8fb346c8d2dd8 Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Thu, 25 Jul 2024 20:54:05 -0700
Subject: [PATCH 4/4] Updates

---
 bolt/include/bolt/Core/DebugData.h                  | 3 +++
 bolt/lib/Core/DIEBuilder.cpp                        | 2 +-
 bolt/lib/Core/DebugData.cpp                         | 2 +-
 bolt/test/X86/dwarf5-df-input-lowpc-ranges-cus.test | 2 +-
 llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h       | 6 +-----
 llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp              | 6 ++----
 6 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/bolt/include/bolt/Core/DebugData.h b/bolt/include/bolt/Core/DebugData.h
index 5935ffaa46af7..c66d9b1cd8610 100644
--- a/bolt/include/bolt/Core/DebugData.h
+++ b/bolt/include/bolt/Core/DebugData.h
@@ -503,10 +503,13 @@ class DebugStrOffsetsWriter {
     return StrOffsetSectionWasModified;
   }
 
+  std::unordered_map<uint64_t, bool> &getDebugStrOffsetFinalized() { return DebugStrOffsetFinalized; }
+
 private:
   std::unique_ptr<DebugStrOffsetsBufferVector> StrOffsetsBuffer;
   std::unique_ptr<raw_svector_ostream> StrOffsetsStream;
   std::map<uint32_t, uint32_t> IndexToAddressMap;
+  std::unordered_map<uint64_t, bool> DebugStrOffsetFinalized;
   SmallVector<uint32_t, 5> StrOffsets;
   std::unordered_map<uint64_t, uint64_t> ProcessedBaseOffsets;
   bool StrOffsetSectionWasModified = false;
diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index eb5b6ed31313f..b058a5b510619 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -77,7 +77,7 @@ static void addStringHelper(DebugStrOffsetsWriter &StrOffstsWriter,
                             DIEValue &DIEAttrInfo, StringRef Str) {
   uint32_t NewOffset = StrWriter.addString(Str);
   if (Unit.getVersion() >= 5) {
-    assert(!Unit.isDebugStrOffsetFinalized() &&
+    assert(StrOffstsWriter.getDebugStrOffsetFinalized().count(Unit.getOffset()) == 0 &&
            "debug_str_offsets was already finalized.");
     StrOffstsWriter.updateAddressMap(DIEAttrInfo.getDIEInteger().getValue(),
                                      NewOffset);
diff --git a/bolt/lib/Core/DebugData.cpp b/bolt/lib/Core/DebugData.cpp
index e96656687fdfe..c1662271abe8b 100644
--- a/bolt/lib/Core/DebugData.cpp
+++ b/bolt/lib/Core/DebugData.cpp
@@ -906,7 +906,7 @@ void DebugStrOffsetsWriter::finalizeSection(DWARFUnit &Unit,
   }
 
   StrOffsetSectionWasModified = false;
-  Unit.setDebugStrOffsetFinalized();
+  DebugStrOffsetFinalized[Unit.getOffset()] = true;
   clear();
 }
 
diff --git a/bolt/test/X86/dwarf5-df-input-lowpc-ranges-cus.test b/bolt/test/X86/dwarf5-df-input-lowpc-ranges-cus.test
index a325395fd5320..45734bdb229fd 100644
--- a/bolt/test/X86/dwarf5-df-input-lowpc-ranges-cus.test
+++ b/bolt/test/X86/dwarf5-df-input-lowpc-ranges-cus.test
@@ -6,7 +6,7 @@
 ; RUN: llvm-mc -dwarf-version=5 -filetype=obj -triple x86_64-unknown-linux %p/Inputs/dwarf5-df-input-lowpc-ranges-other.s \
 ; RUN: -split-dwarf-file=mainOther.dwo -o other.o
 ; RUN: %clang %cflags main.o other.o -o main.exe
-; RUN: llvm-bolt main.exe -o main.exe.bolt --update-debug-sections
+; RUN: llvm-bolt main.exe -o main.exe.bolt --update-debug-sections --cu-processing-batch-size=2
 ; RUN: llvm-dwarfdump --show-form --verbose --debug-rnglists main.exe.bolt &> %t/foo.txt
 ; RUN: llvm-dwarfdump --show-form --verbose --debug-addr main.exe.bolt >> %t/foo.txt
 ; RUN: llvm-dwarfdump --show-form --verbose --debug-info main.exe.bolt >> %t/foo.txt
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 2642d13b16e4c..80c27aea89312 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -230,7 +230,6 @@ 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
@@ -311,15 +310,12 @@ 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,
-            bool IsDebugStrOffsetFinalized = false);
+            const DWARFUnitVector &UnitVector);
 
   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 d2143772946c3..bdd04b00f557b 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -196,13 +196,11 @@ 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 IsDebugStrOffsetFinalized)
+                     bool IsDWO, const DWARFUnitVector &UnitVector)
     : Context(DC), InfoSection(Section), Header(Header), Abbrev(DA),
       RangeSection(RS), LineSection(LS), StringSection(SS),
       StringOffsetSection(SOS), AddrOffsetSection(AOS), IsLittleEndian(LE),
-      IsDWO(IsDWO), IsDebugStrOffsetFinalized(IsDebugStrOffsetFinalized),
-      UnitVector(UnitVector) {
+      IsDWO(IsDWO), UnitVector(UnitVector) {
   clear();
 }
 



More information about the llvm-commits mailing list