[BOLT] Make .debug_loc update deterministic

Summary:
Change the single DebugLocWriter to one for each compilation unit. Then, each thread can write to its own DebugLocWriter and we can combine the data in a deterministic order once the threads are done.

The only catch is that each thread would need the offset of the location lists it adds, so we make a list of pending location list patches and compute the final offsets at the end.

(cherry picked from FBD18153069)

bolt/src/DWARFRewriter.cpp

@@ -77,23 +77,38 @@ void DWARFRewriter::updateDebugInfo() {
 
   ARangesSectionWriter = llvm::make_unique<DebugARangesSectionWriter>();
   RangesSectionWriter = llvm::make_unique<DebugRangesSectionWriter>(&BC);
-  LocationListWriter = llvm::make_unique<DebugLocWriter>(&BC);
 
-  auto processUnitDIE = [&](const DWARFDie DIE) {
+  size_t NumCUs = BC.DwCtx->getNumCompileUnits();
+  if (opts::NoThreads || opts::DeterministicDebugInfo) {
+    // Use single entry for efficiency when running single-threaded
+    NumCUs = 1;
+  }
+
+  LocListWritersByCU.resize(NumCUs);
+
+  for (size_t CUIndex = 0; CUIndex < NumCUs; ++CUIndex) {
+    LocListWritersByCU[CUIndex] = llvm::make_unique<DebugLocWriter>(&BC);
+  }
+
+  auto processUnitDIE = [&](size_t CUIndex, const DWARFDie DIE) {
     const BinaryFunction *CachedFunction = nullptr;
     std::map<DebugAddressRangesVector, uint64_t> CachedRanges{};
-    updateUnitDebugInfo(DIE, std::vector<const BinaryFunction *>{},
+    updateUnitDebugInfo(CUIndex, DIE, std::vector<const BinaryFunction *>{},
                         CachedFunction, CachedRanges);
   };
 
   if (opts::NoThreads || opts::DeterministicDebugInfo) {
-    for (auto &CU : BC.DwCtx->compile_units())
-      processUnitDIE(CU->getUnitDIE(false));
+    for (auto &CU : BC.DwCtx->compile_units()) {
+      processUnitDIE(0, CU->getUnitDIE(false));
+    }
   } else {
     // Update unit debug info in parallel
     auto &ThreadPool = ParallelUtilities::getThreadPool();
-    for (auto &CU : BC.DwCtx->compile_units())
-      ThreadPool.async(processUnitDIE, CU->getUnitDIE(false));
+    size_t CUIndex = 0;
+    for (auto &CU : BC.DwCtx->compile_units()) {
+      ThreadPool.async(processUnitDIE, CUIndex, CU->getUnitDIE(false));
+      CUIndex++;
+    }
 
     ThreadPool.wait();
   }
@@ -106,6 +121,7 @@ void DWARFRewriter::updateDebugInfo() {
 }
 
 void DWARFRewriter::updateUnitDebugInfo(
+    size_t CUIndex,
     const DWARFDie DIE, std::vector<const BinaryFunction *> FunctionStack,
     const BinaryFunction *&CachedFunction,
     std::map<DebugAddressRangesVector, uint64_t> &CachedRanges) {
@@ -219,7 +235,7 @@ void DWARFRewriter::updateUnitDebugInfo(
         Value = *V;
         if (Value.isFormClass(DWARFFormValue::FC_Constant) ||
             Value.isFormClass(DWARFFormValue::FC_SectionOffset)) {
-          auto LocListSectionOffset = LocationListWriter->getEmptyListOffset();
+          auto LocListOffset = DebugLocWriter::EmptyListTag;
           if (Function) {
             // Limit parsing to a single list to save memory.
             DWARFDebugLoc::LocationList LL;
@@ -247,12 +263,19 @@ void DWARFRewriter::updateUnitDebugInfo(
                        << Twine::utohexstr(DIE.getDwarfUnit()->getOffset())
                        << '\n';
               });
-              LocListSectionOffset = LocationListWriter->addList(OutputLL);
+              LocListOffset = LocListWritersByCU[CUIndex]->addList(OutputLL);
             }
           }
 
-          std::lock_guard<std::mutex> Lock(DebugInfoPatcherMutex);
-          DebugInfoPatcher->addLE32Patch(AttrOffset, LocListSectionOffset);
+          if (LocListOffset != DebugLocWriter::EmptyListTag) {
+            std::lock_guard<std::mutex> Lock(LocListDebugInfoPatchesMutex);
+            LocListDebugInfoPatches.push_back(
+                {AttrOffset, CUIndex, LocListOffset});
+          } 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)) &&
@@ -284,7 +307,8 @@ void DWARFRewriter::updateUnitDebugInfo(
 
   // Recursively update each child.
   for (auto Child = DIE.getFirstChild(); Child; Child = Child.getSibling()) {
-    updateUnitDebugInfo(Child, FunctionStack, CachedFunction, CachedRanges);
+    updateUnitDebugInfo(CUIndex, Child, FunctionStack, CachedFunction,
+                        CachedRanges);
   }
 
   if (IsFunctionDef)
@@ -494,7 +518,7 @@ void DWARFRewriter::finalizeDebugSections() {
                                   copyByteArray(*RangesSectionContents),
                                   RangesSectionContents->size());
 
-  auto LocationListSectionContents = LocationListWriter->finalize();
+  auto LocationListSectionContents = makeFinalLocListsSection();
   BC.registerOrUpdateNoteSection(".debug_loc",
                                   copyByteArray(*LocationListSectionContents),
                                   LocationListSectionContents->size());
@@ -663,6 +687,39 @@ void DWARFRewriter::convertPending(const DWARFAbbreviationDeclaration *Abbrev) {
   ConvertedRangesAbbrevs.emplace(Abbrev);
 }
 
+std::unique_ptr<LocBufferVector> DWARFRewriter::makeFinalLocListsSection() {
+  auto LocBuffer = llvm::make_unique<LocBufferVector>();
+  auto LocStream = llvm::make_unique<raw_svector_ostream>(*LocBuffer);
+  auto Writer =
+    std::unique_ptr<MCObjectWriter>(BC.createObjectWriter(*LocStream));
+
+  uint32_t SectionOffset = 0;
+
+  // Add an empty list as the first entry;
+  Writer->writeLE64(0);
+  Writer->writeLE64(0);
+  SectionOffset += 2 * 8;
+
+  std::vector<uint32_t> SectionOffsetByCU(LocListWritersByCU.size());
+
+  for (size_t CUIndex = 0; CUIndex < LocListWritersByCU.size(); ++CUIndex) {
+    SectionOffsetByCU[CUIndex] = SectionOffset;
+    auto CurrCULocationLists = LocListWritersByCU[CUIndex]->finalize();
+    Writer->writeBytes(*CurrCULocationLists);
+    SectionOffset += CurrCULocationLists->size();
+  }
+
+  for (auto &Patch : LocListDebugInfoPatches) {
+    DebugInfoPatcher->addLE32Patch(
+      Patch.DebugInfoOffset,
+      SectionOffsetByCU[Patch.CUIndex]
+      + Patch.CUWriterOffset
+    );
+  }
+
+  return std::move(LocBuffer);
+}
+
 void DWARFRewriter::flushPendingRanges() {
   for (auto &I : PendingRanges) {
     for (auto &RangePair : I.second) {

bolt/src/DWARFRewriter.h

@@ -16,6 +16,7 @@
 #include "RewriteInstance.h"
 #include <map>
 #include <mutex>
+#include <vector>
 
 namespace llvm {
 
@@ -48,12 +49,26 @@ class DWARFRewriter {
   /// .debug_aranges DWARF section.
   std::unique_ptr<DebugARangesSectionWriter> ARangesSectionWriter;
 
-  std::unique_ptr<DebugLocWriter> LocationListWriter;
+  /// Use a separate location list writer for each compilation unit
+  std::vector<std::unique_ptr<DebugLocWriter>> LocListWritersByCU;
+
+  struct LocListDebugInfoPatchType {
+    uint32_t DebugInfoOffset;
+    size_t CUIndex;
+    uint64_t CUWriterOffset;
+  };
+
+  /// The list of debug info patches to be made once individual
+  /// location list writers have been filled
+  std::vector<LocListDebugInfoPatchType> LocListDebugInfoPatches;
+
+  std::mutex LocListDebugInfoPatchesMutex;
 
   /// Recursively update debug info for all DIEs in \p Unit.
   /// If \p Function is not empty, it points to a function corresponding
   /// to a parent DW_TAG_subprogram node of the current \p DIE.
   void updateUnitDebugInfo(
+      size_t CUIndex,
       const DWARFDie DIE, std::vector<const BinaryFunction *> FunctionStack,
       const BinaryFunction *&CachedFunction,
       std::map<DebugAddressRangesVector, uint64_t> &CachedRanges);
@@ -68,6 +83,8 @@ class DWARFRewriter {
   void updateDWARFObjectAddressRanges(const DWARFDie DIE,
                                       uint64_t DebugRangesOffset);
 
+  std::unique_ptr<LocBufferVector> makeFinalLocListsSection();
+
   /// Generate new contents for .debug_ranges and .debug_aranges section.
   void finalizeDebugSections();
 

bolt/src/DebugData.cpp

@@ -153,21 +153,15 @@ DebugLocWriter::DebugLocWriter(BinaryContext *BC) {
   LocStream = llvm::make_unique<raw_svector_ostream>(*LocBuffer);
   Writer =
     std::unique_ptr<MCObjectWriter>(BC->createObjectWriter(*LocStream));
-
-  // Add an empty list as the first entry;
-  Writer->writeLE64(0);
-  Writer->writeLE64(0);
-  SectionOffset += 2 * 8;
 }
 
 // DWARF 4: 2.6.2
 uint64_t DebugLocWriter::addList(const DWARFDebugLoc::LocationList &LocList) {
   if (LocList.Entries.empty())
-    return getEmptyListOffset();
+    return EmptyListTag;
 
-  // Reading the SectionOffset and updating it should be atomic to guarantee
-  // unique and correct offsets in patches.
-  std::lock_guard<std::mutex> Lock(WriterMutex);
+  // Since there is a separate DebugLocWriter for each thread,
+  // we don't need a lock to read the SectionOffset and update it.
   const auto EntryOffset = SectionOffset;
 
   for (const auto &Entry : LocList.Entries) {

bolt/src/DebugData.h

@@ -168,19 +168,25 @@ private:
 
 using LocBufferVector = SmallVector<char, 16>;
 
-/// Serializes the .debug_loc DWARF section with LocationLists.
+/// Serializes part of a .debug_loc DWARF section with LocationLists.
 class DebugLocWriter {
 public:
   DebugLocWriter(BinaryContext *BC);
 
   uint64_t addList(const DWARFDebugLoc::LocationList &LocList);
 
-  uint64_t getEmptyListOffset() const { return EmptyListOffset; }
-
   std::unique_ptr<LocBufferVector> finalize() {
     return std::move(LocBuffer);
   }
 
+  /// 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;
+
 private:
   std::unique_ptr<LocBufferVector> LocBuffer;
 
@@ -188,13 +194,10 @@ private:
 
   std::unique_ptr<MCObjectWriter> Writer;
 
-  std::mutex WriterMutex;
-
-  /// Offset of an empty location list.
-  static uint64_t const EmptyListOffset = 0;
-
   /// Current offset in the section (updated as new entries are written).
-  /// Starts with 16 since the first 16 bytes are reserved for an empty range.
+  /// Starts with 0 here since this only writes part of a full location lists
+  /// section. In the final section, the first 16 bytes are reserved for an
+  /// empty list.
   uint32_t SectionOffset{0};
 };