[BOLT][DWARF] Refactor of Loc and LocLists writers

Summary: Refactored Loc and LocList writers to write out entries during finalization phase, and hid some of the details in a class. This simplifies things from impelementation details, and also will be needed for DWARF5 where we need to know how many locLists entries there are there. (cherry picked from FBD31563795)
2026-01-15 12:25:46 +08:00 · 2021-10-11 17:51:05 -07:00
parent 53ec21e3a1
commit fdd9184db5
4 changed files with 130 additions and 145 deletions
--- a/bolt/src/DWARFRewriter.cpp
+++ b/bolt/src/DWARFRewriter.cpp
@@ -244,8 +244,6 @@ void DWARFRewriter::updateDebugInfo() {
      DebugAbbrevWriter *DWOAbbrevWriter = getBinaryDWOAbbrevWriter(*DWOId);
      updateUnitDebugInfo(*DWOId, *(*SplitCU), *DwoDebugInfoPatcher,
                          *DWOAbbrevWriter);
-      static_cast<DebugLoclistWriter *>(LocListWritersByCU[*DWOId].get())
-          ->finalizePatches();
      if (!DwoDebugInfoPatcher->getWasRangBasedUsed())
        RangesBase = None;
    }
@@ -477,7 +475,6 @@ void DWARFRewriter::updateUnitDebugInfo(uint64_t CUIndex, DWARFUnit &Unit,
                return true;
              });

-          uint64_t OutputLocListOffset = DebugLocWriter::EmptyListTag;
          if (E || InputLL.empty()) {
            errs() << "BOLT-WARNING: empty location list detected at 0x"
                   << Twine::utohexstr(Offset) << " for DIE at 0x"
@@ -487,35 +484,17 @@ void DWARFRewriter::updateUnitDebugInfo(uint64_t CUIndex, DWARFUnit &Unit,
            const uint64_t Address = InputLL.front().LowPC;
            if (const BinaryFunction *Function =
                    BC.getBinaryFunctionContainingAddress(Address)) {
-              const DebugLocationsVector OutputLL = Function
-                  ->translateInputToOutputLocationList(InputLL);
+              DebugLocationsVector OutputLL =
+                  Function->translateInputToOutputLocationList(InputLL);
              LLVM_DEBUG(if (OutputLL.empty()) {
                dbgs() << "BOLT-DEBUG: location list translated to an empty "
                          "one at 0x"
                       << Twine::utohexstr(DIE.getOffset()) << " in CU at 0x"
                       << Twine::utohexstr(Unit.getOffset()) << '\n';
              });
-              OutputLocListOffset = DebugLocWriter.addList(OutputLL);
+              DebugLocWriter.addList(AttrOffset, std::move(OutputLL));
            }
          }
-
-          if (OutputLocListOffset != DebugLocWriter::EmptyListTag) {
-            std::lock_guard<std::mutex> Lock(LocListDebugInfoPatchesMutex);
-            if (Unit.isDWOUnit()) {
-              // Not sure if better approach is to hide all of this away in a
-              // class. Also re-using LocListDebugInfoPatchType. Wasting some
-              // space for DWOID/CUIndex.
-              DwoLocListDebugInfoPatches[CUIndex].push_back(
-                  {AttrOffset, CUIndex, OutputLocListOffset});
-            } else {
-              LocListDebugInfoPatches.push_back(
-                  {AttrOffset, CUIndex, OutputLocListOffset});
-            }
-          } else {
-            std::lock_guard<std::mutex> Lock(DebugInfoPatcherMutex);
-            DebugInfoPatcher.addLE32Patch(AttrOffset,
-                                          DebugLocWriter::EmptyListOffset);
-          }
        } else {
          assert((Value.isFormClass(DWARFFormValue::FC_Exprloc) ||
                  Value.isFormClass(DWARFFormValue::FC_Block)) &&
@@ -768,6 +747,20 @@ void DWARFRewriter::finalizeDebugSections(
                                   DebugStrSectionContents->size());
  }

+  std::unique_ptr<DebugBufferVector> RangesSectionContents =
+      RangesSectionWriter->finalize();
+  BC.registerOrUpdateNoteSection(".debug_ranges",
+                                 copyByteArray(*RangesSectionContents),
+                                 RangesSectionContents->size());
+
+  std::unique_ptr<DebugBufferVector> LocationListSectionContents =
+      makeFinalLocListsSection(DebugInfoPatcher);
+  BC.registerOrUpdateNoteSection(".debug_loc",
+                                 copyByteArray(*LocationListSectionContents),
+                                 LocationListSectionContents->size());
+
+  // AddrWriter should be finalized after debug_loc since more addresses can be
+  // added there.
  if (AddrWriter->isInitialized()) {
    AddressSectionBuffer AddressSectionContents = AddrWriter->finalize();
    BC.registerOrUpdateNoteSection(".debug_addr",
@@ -784,18 +777,6 @@ void DWARFRewriter::finalizeDebugSections(
    }
  }

-  std::unique_ptr<DebugBufferVector> RangesSectionContents =
-      RangesSectionWriter->finalize();
-  BC.registerOrUpdateNoteSection(".debug_ranges",
-                                 copyByteArray(*RangesSectionContents),
-                                 RangesSectionContents->size());
-
-  std::unique_ptr<DebugBufferVector> LocationListSectionContents =
-      makeFinalLocListsSection(DebugInfoPatcher);
-  BC.registerOrUpdateNoteSection(".debug_loc",
-                                 copyByteArray(*LocationListSectionContents),
-                                 LocationListSectionContents->size());
-
  std::unique_ptr<DebugBufferVector> AbbrevSectionContents =
      AbbrevWriter->finalize();
  BC.registerOrUpdateNoteSection(".debug_abbrev",
@@ -943,7 +924,7 @@ updateDebugData(std::string &Storage, const SectionRef &Section,
  }
  case DWARFSectionKind::DW_SECT_EXT_LOC: {
    DebugLocWriter *LocWriter = Writer.getDebugLocWriter(DWOId);
-    OutputBuffer = LocWriter->finalize();
+    OutputBuffer = LocWriter->getBuffer();
    // Creating explicit StringRef here, otherwise
    // with impicit conversion it will take null byte as end of
    // string.
@@ -1339,39 +1320,22 @@ DWARFRewriter::makeFinalLocListsSection(SimpleBinaryPatcher &DebugInfoPatcher) {
  *LocStream << StringRef(Zeroes, 16);
  SectionOffset += 2 * 8;

-  std::unordered_map<uint64_t, uint64_t> SectionOffsetByCU(
-      LocListWritersByCU.size());
-
  for (std::pair<const uint64_t, std::unique_ptr<DebugLocWriter>> &Loc :
       LocListWritersByCU) {
-    uint64_t CUIndex = Loc.first;
    DebugLocWriter *LocWriter = Loc.second.get();
-    if (llvm::isa<DebugLoclistWriter>(*LocWriter))
+    if (auto *LocListWriter = llvm::dyn_cast<DebugLoclistWriter>(LocWriter)) {
+      SimpleBinaryPatcher *Patcher =
+          getBinaryDWODebugInfoPatcher(LocListWriter->getDWOID());
+      LocListWriter->finalize(0, *Patcher);
      continue;
-    SectionOffsetByCU[CUIndex] = SectionOffset;
+    }
+    LocWriter->finalize(SectionOffset, DebugInfoPatcher);
    std::unique_ptr<DebugBufferVector> CurrCULocationLists =
-        LocWriter->finalize();
+        LocWriter->getBuffer();
    *LocStream << *CurrCULocationLists;
    SectionOffset += CurrCULocationLists->size();
  }

-  for (std::pair<const uint64_t, VectorLocListDebugInfoPatchType> &Iter :
-       DwoLocListDebugInfoPatches) {
-    uint64_t DWOId = Iter.first;
-    SimpleBinaryPatcher *Patcher = getBinaryDWODebugInfoPatcher(DWOId);
-    for (LocListDebugInfoPatchType &Patch : Iter.second) {
-      Patcher->addLE32Patch(Patch.DebugInfoOffset,
-                            SectionOffsetByCU[Patch.CUIndex] +
-                                Patch.CUWriterOffset);
-    }
-  }
-
-  for (LocListDebugInfoPatchType &Patch : LocListDebugInfoPatches) {
-    DebugInfoPatcher.addLE32Patch(Patch.DebugInfoOffset,
-                                  SectionOffsetByCU[Patch.CUIndex] +
-                                      Patch.CUWriterOffset);
-  }
-
  return LocBuffer;
 }

--- a/bolt/src/DWARFRewriter.h
+++ b/bolt/src/DWARFRewriter.h
@@ -68,20 +68,6 @@ class DWARFRewriter {
  /// Binary patchers for DWO debug_info sections.
  DebugInfoDWOPatchers BinaryDWODebugInfoPatchers;

-  struct LocListDebugInfoPatchType {
-    uint64_t DebugInfoOffset;
-    size_t CUIndex;
-    uint64_t CUWriterOffset;
-  };
-  using VectorLocListDebugInfoPatchType =
-      std::vector<LocListDebugInfoPatchType>;
-  /// The list of debug info patches to be made once individual
-  /// location list writers have been filled
-  VectorLocListDebugInfoPatchType LocListDebugInfoPatches;
-
-  std::unordered_map<uint64_t, VectorLocListDebugInfoPatchType>
-      DwoLocListDebugInfoPatches;
-
  std::mutex LocListDebugInfoPatchesMutex;

  /// Update debug info for all DIEs in \p Unit.
--- a/bolt/src/DebugData.cpp
+++ b/bolt/src/DebugData.cpp
@@ -11,6 +11,7 @@
 #include "DebugData.h"
 #include "BinaryBasicBlock.h"
 #include "BinaryFunction.h"
+#include "Utils.h"

 #include "llvm/MC/MCObjectStreamer.h"
 #include "llvm/MC/MCSymbol.h"
@@ -262,12 +263,12 @@ DebugLocWriter::DebugLocWriter(BinaryContext *BC) {
  LocStream = std::make_unique<raw_svector_ostream>(*LocBuffer);
 }

-// DWARF 4: 2.6.2
-uint64_t
-DebugLocWriter::addList(const DebugLocationsVector &LocList) {
-  if (LocList.empty())
-    return EmptyListTag;
-
+void DebugLocWriter::addList(uint64_t AttrOffset,
+                             DebugLocationsVector &&LocList) {
+  if (LocList.empty()) {
+    EmptyAttrLists.push_back(AttrOffset);
+    return;
+  }
  // Since there is a separate DebugLocWriter for each thread,
  // we don't need a lock to read the SectionOffset and update it.
  const uint32_t EntryOffset = SectionOffset;
@@ -285,58 +286,68 @@ DebugLocWriter::addList(const DebugLocationsVector &LocList) {
  }
  LocStream->write_zeros(16);
  SectionOffset += 16;
+  LocListDebugInfoPatches.push_back({AttrOffset, EntryOffset});
+}

-  return EntryOffset;
+void DebugLoclistWriter::addList(uint64_t AttrOffset,
+                                 DebugLocationsVector &&LocList) {
+  Patches.push_back({AttrOffset, std::move(LocList)});
+}
+
+std::unique_ptr<DebugBufferVector> DebugLocWriter::getBuffer() {
+  return std::move(LocBuffer);
+}
+
+// DWARF 4: 2.6.2
+void DebugLocWriter::finalize(uint64_t SectionOffset,
+                              SimpleBinaryPatcher &DebugInfoPatcher) {
+  for (const auto LocListDebugInfoPatchType : LocListDebugInfoPatches) {
+    uint64_t Offset = SectionOffset + LocListDebugInfoPatchType.LocListOffset;
+    DebugInfoPatcher.addLE32Patch(LocListDebugInfoPatchType.DebugInfoAttrOffset,
+                                  Offset);
+  }
+
+  for (uint64_t DebugInfoAttrOffset : EmptyAttrLists)
+    DebugInfoPatcher.addLE32Patch(DebugInfoAttrOffset,
+                                  DebugLocWriter::EmptyListOffset);
+}
+
+void DebugLoclistWriter::finalize(uint64_t SectionOffset,
+                                  SimpleBinaryPatcher &DebugInfoPatcher) {
+  for (LocPatch &Patch : Patches) {
+    if (Patch.LocList.empty()) {
+      DebugInfoPatcher.addLE32Patch(Patch.AttrOffset,
+                                    DebugLocWriter::EmptyListOffset);
+      continue;
+    }
+    const uint32_t EntryOffset = LocBuffer->size();
+    for (const DebugLocationEntry &Entry : Patch.LocList) {
+      support::endian::write(*LocStream,
+                             static_cast<uint8_t>(dwarf::DW_LLE_startx_length),
+                             support::little);
+      uint32_t Index = AddrWriter->getIndexFromAddress(Entry.LowPC, DWOId);
+      encodeULEB128(Index, *LocStream);
+
+      // TODO: Support DWARF5
+      support::endian::write(*LocStream,
+                             static_cast<uint32_t>(Entry.HighPC - Entry.LowPC),
+                             support::little);
+      support::endian::write(*LocStream,
+                             static_cast<uint16_t>(Entry.Expr.size()),
+                             support::little);
+      *LocStream << StringRef(reinterpret_cast<const char *>(Entry.Expr.data()),
+                              Entry.Expr.size());
+    }
+    support::endian::write(*LocStream,
+                           static_cast<uint8_t>(dwarf::DW_LLE_end_of_list),
+                           support::little);
+    DebugInfoPatcher.addLE32Patch(Patch.AttrOffset, EntryOffset);
+    clearList(Patch.LocList);
+  }
+  clearList(Patches);
 }

 DebugAddrWriter *DebugLoclistWriter::AddrWriter = nullptr;
-void DebugLoclistWriter::finalizePatches() {
-  auto numOfBytes = [](uint32_t Val) -> uint32_t {
-    int LogVal = (int)std::log2(Val) + 1;
-    uint32_t CeilVal = (LogVal + 8 - 1) / 8;
-    return !Val ? 1 : CeilVal;
-  };
-  (void)numOfBytes;
-
-  for (const auto &Patch : IndexPatches) {
-    uint32_t Index = AddrWriter->getIndexFromAddress(Patch.Address, DWOId);
-    assert(numOfBytes(Index) <= DebugLoclistWriter::NumBytesForIndex &&
-           "Index size in DebugLocation too large.");
-    std::string Buff;
-    raw_string_ostream OS(Buff);
-    encodeULEB128(Index, OS, DebugLoclistWriter::NumBytesForIndex);
-    for (uint32_t I = 0; I < DebugLoclistWriter::NumBytesForIndex; ++I) {
-      (*LocBuffer)[Patch.Offset + I] = Buff[I];
-    }
-  }
-}
-
-uint64_t DebugLoclistWriter::addList(const DebugLocationsVector &LocList) {
-  if (LocList.empty())
-    return EmptyListTag;
-  uint64_t EntryOffset = LocBuffer->size();
-
-  for (const DebugLocationEntry &Entry : LocList) {
-    support::endian::write(*LocStream,
-                           static_cast<uint8_t>(dwarf::DW_LLE_startx_length),
-                           support::little);
-    IndexPatches.emplace_back(static_cast<uint32_t>(LocBuffer->size()),
-                              Entry.LowPC);
-    LocStream->write_zeros(DebugLoclistWriter::NumBytesForIndex);
-    // TODO: Support DWARF5
-    support::endian::write(*LocStream,
-                           static_cast<uint32_t>(Entry.HighPC - Entry.LowPC),
-                           support::little);
-    support::endian::write(*LocStream, static_cast<uint16_t>(Entry.Expr.size()),
-                           support::little);
-    *LocStream << StringRef(reinterpret_cast<const char *>(Entry.Expr.data()),
-                            Entry.Expr.size());
-  }
-  support::endian::write(*LocStream,
-                         static_cast<uint8_t>(dwarf::DW_LLE_end_of_list),
-                         support::little);
-  return EntryOffset;
-}

 void SimpleBinaryPatcher::addBinaryPatch(uint32_t Offset,
                                         const std::string &NewValue) {
--- a/bolt/src/DebugData.h
+++ b/bolt/src/DebugData.h
@@ -287,26 +287,26 @@ private:
 enum class LocWriterKind { DebugLocWriter, DebugLoclistWriter };

 /// Serializes part of a .debug_loc DWARF section with LocationLists.
+class SimpleBinaryPatcher;
 class DebugLocWriter {
 public:
  DebugLocWriter() = delete;
  DebugLocWriter(BinaryContext *BC);
  virtual ~DebugLocWriter(){};

-  virtual uint64_t addList(const DebugLocationsVector &LocList);
+  /// Writes out location lists and stores internal patches.
+  virtual void addList(uint64_t AttrOffset, DebugLocationsVector &&LocList);

-  virtual std::unique_ptr<DebugBufferVector> finalize() {
-    return std::move(LocBuffer);
-  }
+  /// Writes out locations in to a local buffer, and adds Debug Info patches.
+  virtual void finalize(uint64_t SectionOffset,
+                        SimpleBinaryPatcher &DebugInfoPatcher);
+
+  /// Return internal buffer.
+  virtual std::unique_ptr<DebugBufferVector> getBuffer();

  /// Offset of an empty location list.
  static constexpr uint32_t EmptyListOffset = 0;

-  /// Value returned by addList if list is empty
-  /// Use 64 bits here so that a max 32 bit value can still
-  /// be stored while we use max 64 bit value as empty tag
-  static constexpr uint64_t EmptyListTag = -1;
-
  LocWriterKind getKind() const { return Kind; }

  static bool classof(const DebugLocWriter *Writer) {
@@ -323,6 +323,21 @@ protected:
  /// empty list.
  uint32_t SectionOffset{0};
  LocWriterKind Kind{LocWriterKind::DebugLocWriter};
+
+private:
+  struct LocListDebugInfoPatchType {
+    uint64_t DebugInfoAttrOffset;
+    uint64_t LocListOffset;
+  };
+  using VectorLocListDebugInfoPatchType =
+      std::vector<LocListDebugInfoPatchType>;
+  /// The list of debug info patches to be made once individual
+  /// location list writers have been filled
+  VectorLocListDebugInfoPatchType LocListDebugInfoPatches;
+
+  using VectorEmptyLocListAttributes = std::vector<uint64_t>;
+  /// Contains all the attributes pointing to empty location list.
+  VectorEmptyLocListAttributes EmptyAttrLists;
 };

 class DebugLoclistWriter : public DebugLocWriter {
@@ -339,17 +354,29 @@ public:

  static void setAddressWriter(DebugAddrWriter *AddrW) { AddrWriter = AddrW; }

-  /// Writes out locationList, with index in to .debug_addr to be patched later.
-  uint64_t addList(const DebugLocationsVector &LocList) override;
+  /// Stores location lists internally to be written out during finalize phase.
+  virtual void addList(uint64_t AttrOffset,
+                       DebugLocationsVector &&LocList) override;

-  /// Finalizes all the location by patching correct index in to .debug_addr.
-  void finalizePatches();
+  /// Writes out locations in to a local buffer and applies debug info patches.
+  void finalize(uint64_t SectionOffset,
+                SimpleBinaryPatcher &DebugInfoPatcher) override;
+
+  /// Returns DWO ID.
+  uint64_t getDWOID() const { return DWOId; }

  static bool classof(const DebugLocWriter *Writer) {
    return Writer->getKind() == LocWriterKind::DebugLoclistWriter;
  }

 private:
+  struct LocPatch {
+    uint64_t AttrOffset{0};
+    DebugLocationsVector LocList;
+  };
+  using LocPatchVec = SmallVector<LocPatch, 4>;
+  LocPatchVec Patches;
+
  class Patch {
  public:
    Patch() = delete;
@@ -359,9 +386,6 @@ private:
  };
  static DebugAddrWriter *AddrWriter;
  uint64_t DWOId{0};
-  std::unordered_map<uint64_t, uint32_t> AddressMap;
-  std::vector<Patch> IndexPatches;
-  constexpr static uint32_t NumBytesForIndex{3};
 };

 /// Abstract interface for classes that apply modifications to a binary string.