Re-land "[PDB] Defer relocating .debug$S until commit time and parallelize it"

This reverts commit bacf9cf2c5cdec3567580e5030c4c82f42b3d745 and
reinstates commit 1a9bd5b81328adf0dd5a8b4f3ad5949463e66da3.

Reverting this commit did not appear to make the problem go away, so we
can go ahead and reland it.

include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h

@@ -34,6 +34,34 @@ struct MSFLayout;
 }
 namespace pdb {
 
+// Represents merged or unmerged symbols. Merged symbols can be written to the
+// output file as is, but unmerged symbols must be rewritten first. In either
+// case, the size must be known up front.
+struct SymbolListWrapper {
+  explicit SymbolListWrapper(ArrayRef<uint8_t> Syms)
+      : SymPtr(const_cast<uint8_t *>(Syms.data())), SymSize(Syms.size()),
+        NeedsToBeMerged(false) {}
+  explicit SymbolListWrapper(void *SymSrc, uint32_t Length)
+      : SymPtr(SymSrc), SymSize(Length), NeedsToBeMerged(true) {}
+
+  ArrayRef<uint8_t> asArray() const {
+    return ArrayRef<uint8_t>(static_cast<const uint8_t *>(SymPtr), SymSize);
+  }
+
+  uint32_t size() const { return SymSize; }
+
+  void *SymPtr = nullptr;
+  uint32_t SymSize = 0;
+  bool NeedsToBeMerged = false;
+};
+
+/// Represents a string table reference at some offset in the module symbol
+/// stream.
+struct StringTableFixup {
+  uint32_t StrTabOffset = 0;
+  uint32_t SymOffsetOfReference = 0;
+};
+
 class DbiModuleDescriptorBuilder {
   friend class DbiStreamBuilder;
 
@@ -48,10 +76,28 @@ public:
 
   void setPdbFilePathNI(uint32_t NI);
   void setObjFileName(StringRef Name);
+
+  // Callback to merge one source of unmerged symbols.
+  using MergeSymbolsCallback = Error (*)(void *Ctx, void *Symbols,
+                                         BinaryStreamWriter &Writer);
+
+  void setMergeSymbolsCallback(void *Ctx, MergeSymbolsCallback Callback) {
+    MergeSymsCtx = Ctx;
+    MergeSymsCallback = Callback;
+  }
+
+  void setStringTableFixups(std::vector<StringTableFixup> &&Fixups) {
+    StringTableFixups = std::move(Fixups);
+  }
+
   void setFirstSectionContrib(const SectionContrib &SC);
   void addSymbol(codeview::CVSymbol Symbol);
   void addSymbolsInBulk(ArrayRef<uint8_t> BulkSymbols);
 
+  // Add symbols of known size which will be merged (rewritten) when committing
+  // the PDB to disk.
+  void addUnmergedSymbols(void *SymSrc, uint32_t SymLength);
+
   void
   addDebugSubsection(std::shared_ptr<codeview::DebugSubsection> Subsection);
 
@@ -77,8 +123,14 @@ public:
   void finalize();
   Error finalizeMsfLayout();
 
-  Error commit(BinaryStreamWriter &ModiWriter, const msf::MSFLayout &MsfLayout,
+  /// Commit the DBI descriptor to the DBI stream.
-               WritableBinaryStreamRef MsfBuffer);
+  Error commit(BinaryStreamWriter &ModiWriter);
+
+  /// Commit the accumulated symbols to the module symbol stream. Safe to call
+  /// in parallel on different DbiModuleDescriptorBuilder objects. Only modifies
+  /// the pre-allocated stream in question.
+  Error commitSymbolStream(const msf::MSFLayout &MsfLayout,
+                           WritableBinaryStreamRef MsfBuffer);
 
 private:
   uint32_t calculateC13DebugInfoSize() const;
@@ -91,7 +143,12 @@ private:
   std::string ModuleName;
   std::string ObjFileName;
   std::vector<std::string> SourceFiles;
-  std::vector<ArrayRef<uint8_t>> Symbols;
+  std::vector<SymbolListWrapper> Symbols;
+
+  void *MergeSymsCtx = nullptr;
+  MergeSymbolsCallback MergeSymsCallback = nullptr;
+
+  std::vector<StringTableFixup> StringTableFixups;
 
   std::vector<codeview::DebugSubsectionRecordBuilder> C13Builders;
 

lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp

@@ -74,7 +74,7 @@ void DbiModuleDescriptorBuilder::addSymbolsInBulk(
   if (BulkSymbols.empty())
     return;
 
-  Symbols.push_back(BulkSymbols);
+  Symbols.push_back(SymbolListWrapper(BulkSymbols));
   // Symbols written to a PDB file are required to be 4 byte aligned. The same
   // is not true of object files.
   assert(BulkSymbols.size() % alignOf(CodeViewContainer::Pdb) == 0 &&
@@ -82,6 +82,18 @@ void DbiModuleDescriptorBuilder::addSymbolsInBulk(
   SymbolByteSize += BulkSymbols.size();
 }
 
+void DbiModuleDescriptorBuilder::addUnmergedSymbols(void *SymSrc,
+                                                    uint32_t SymLength) {
+  assert(SymLength > 0);
+  Symbols.push_back(SymbolListWrapper(SymSrc, SymLength));
+
+  // Symbols written to a PDB file are required to be 4 byte aligned. The same
+  // is not true of object files.
+  assert(SymLength % alignOf(CodeViewContainer::Pdb) == 0 &&
+         "Invalid Symbol alignment!");
+  SymbolByteSize += SymLength;
+}
+
 void DbiModuleDescriptorBuilder::addSourceFile(StringRef Path) {
   SourceFiles.push_back(std::string(Path));
 }
@@ -131,9 +143,7 @@ Error DbiModuleDescriptorBuilder::finalizeMsfLayout() {
   return Error::success();
 }
 
-Error DbiModuleDescriptorBuilder::commit(BinaryStreamWriter &ModiWriter,
+Error DbiModuleDescriptorBuilder::commit(BinaryStreamWriter &ModiWriter) {
-                                         const msf::MSFLayout &MsfLayout,
-                                         WritableBinaryStreamRef MsfBuffer) {
   // We write the Modi record to the `ModiWriter`, but we additionally write its
   // symbol stream to a brand new stream.
   if (auto EC = ModiWriter.writeObject(Layout))
@@ -144,34 +154,55 @@ Error DbiModuleDescriptorBuilder::commit(BinaryStreamWriter &ModiWriter,
     return EC;
   if (auto EC = ModiWriter.padToAlignment(sizeof(uint32_t)))
     return EC;
+  return Error::success();
+}
 
-  if (Layout.ModDiStream != kInvalidStreamIndex) {
+Error DbiModuleDescriptorBuilder::commitSymbolStream(
-    auto NS = WritableMappedBlockStream::createIndexedStream(
+    const msf::MSFLayout &MsfLayout, WritableBinaryStreamRef MsfBuffer) {
-        MsfLayout, MsfBuffer, Layout.ModDiStream, MSF.getAllocator());
+  if (Layout.ModDiStream == kInvalidStreamIndex)
-    WritableBinaryStreamRef Ref(*NS);
+    return Error::success();
-    BinaryStreamWriter SymbolWriter(Ref);
+
-    // Write the symbols.
+  auto NS = WritableMappedBlockStream::createIndexedStream(
-    if (auto EC =
+      MsfLayout, MsfBuffer, Layout.ModDiStream, MSF.getAllocator());
-            SymbolWriter.writeInteger<uint32_t>(COFF::DEBUG_SECTION_MAGIC))
+  WritableBinaryStreamRef Ref(*NS);
-      return EC;
+  BinaryStreamWriter SymbolWriter(Ref);
-    for (ArrayRef<uint8_t> Syms : Symbols) {
+  // Write the symbols.
-      if (auto EC = SymbolWriter.writeBytes(Syms))
+  if (auto EC = SymbolWriter.writeInteger<uint32_t>(COFF::DEBUG_SECTION_MAGIC))
+    return EC;
+  for (const SymbolListWrapper &Sym : Symbols) {
+    if (Sym.NeedsToBeMerged) {
+      assert(MergeSymsCallback);
+      if (auto EC = MergeSymsCallback(MergeSymsCtx, Sym.SymPtr, SymbolWriter))
+        return EC;
+    } else {
+      if (auto EC = SymbolWriter.writeBytes(Sym.asArray()))
         return EC;
     }
-    assert(SymbolWriter.getOffset() % alignOf(CodeViewContainer::Pdb) == 0 &&
-           "Invalid debug section alignment!");
-    // TODO: Write C11 Line data
-    for (const auto &Builder : C13Builders) {
-      if (auto EC = Builder.commit(SymbolWriter, CodeViewContainer::Pdb))
-        return EC;
-    }
-
-    // TODO: Figure out what GlobalRefs substream actually is and populate it.
-    if (auto EC = SymbolWriter.writeInteger<uint32_t>(0))
-      return EC;
-    if (SymbolWriter.bytesRemaining() > 0)
-      return make_error<RawError>(raw_error_code::stream_too_long);
   }
+
+  // Apply the string table fixups.
+  auto SavedOffset = SymbolWriter.getOffset();
+  for (const StringTableFixup &Fixup : StringTableFixups) {
+    SymbolWriter.setOffset(Fixup.SymOffsetOfReference);
+    if (auto E = SymbolWriter.writeInteger<uint32_t>(Fixup.StrTabOffset))
+      return E;
+  }
+  SymbolWriter.setOffset(SavedOffset);
+
+  assert(SymbolWriter.getOffset() % alignOf(CodeViewContainer::Pdb) == 0 &&
+         "Invalid debug section alignment!");
+  // TODO: Write C11 Line data
+  for (const auto &Builder : C13Builders) {
+    if (auto EC = Builder.commit(SymbolWriter, CodeViewContainer::Pdb))
+      return EC;
+  }
+
+  // TODO: Figure out what GlobalRefs substream actually is and populate it.
+  if (auto EC = SymbolWriter.writeInteger<uint32_t>(0))
+    return EC;
+  if (SymbolWriter.bytesRemaining() > 0)
+    return make_error<RawError>(raw_error_code::stream_too_long);
+
   return Error::success();
 }
 

lib/DebugInfo/PDB/Native/DbiStreamBuilder.cpp

@@ -18,6 +18,7 @@
 #include "llvm/DebugInfo/PDB/Native/RawError.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/BinaryStreamWriter.h"
+#include "llvm/Support/Parallel.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
@@ -394,10 +395,17 @@ Error DbiStreamBuilder::commit(const msf::MSFLayout &Layout,
     return EC;
 
   for (auto &M : ModiList) {
-    if (auto EC = M->commit(Writer, Layout, MsfBuffer))
+    if (auto EC = M->commit(Writer))
       return EC;
   }
 
+  // Commit symbol streams. This is a lot of data, so do it in parallel.
+  if (auto EC = parallelForEachError(
+          ModiList, [&](std::unique_ptr<DbiModuleDescriptorBuilder> &M) {
+            return M->commitSymbolStream(Layout, MsfBuffer);
+          }))
+    return EC;
+
   if (!SectionContribs.empty()) {
     if (auto EC = Writer.writeEnum(DbiSecContribVer60))
       return EC;