Define PDBFileBuilder::addStream to add stream.

[ruiu]

Summary

Previously, there is no way to create a stream other than
pre-defined special stream such as DBI or IPI. This patch
adds a new method, addStream, to add a stream of arbitrary
bytes to a PDB file. The new function takes a byte array
and returns the index of the new stream.

[ruiu]

• rename PDBFileBuilder::addStream to addDbgStream
• make addDbgStream to set the directory entry for a new debug stream.

[ruiu]

• Move addDbgStream to DbiStreamBuilder.

[zturner]

include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h

99–100 ↗

(On Diff #73990)

I believe these should be initialized to kInvalidStreamIndex.

99–101 ↗

(On Diff #73990)

I wonder if this could just be a llvm::SmallVector<ArrayRef<uint8_t>, DbgHeaderType::Max>. This way you don't need one array which indexes into another array, but rather you just have one array altogether that contains all the data.

lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp

246–249 ↗

(On Diff #73990)

Here you will need to loop through the various optional debug streams and call setStreamSize() on each one.

295–296 ↗

(On Diff #73990)

Here you should check if Idx == kInvalidStreamIndex, and continue if so.

297 ↗

(On Diff #73990)

Can you use a different variable name here? This Writer shadows the previous Writer, which could be confusing. Maybe calling it DbgStreamWriter.

[ruiu]

include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h

99–101 ↗

(On Diff #73990)

We need to distinguish an empty stream from nonexistent stream, so we need to store the stream number somewhere. I defined a new class for that purpose.

lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp

246–249 ↗

(On Diff #73990)

I don't think so. We create a new stream like this

auto ExpectedIndex = Msf.addStream(Data.size());

and because the size will never change, we don't need to adjust the size afterwards.

[ruiu]

• Updated as per the review comments.

[zturner]

lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp

49 ↗

(On Diff #74155)

I think we shouldn't call addStream here. Suppose someone called addDbgStream more than once for the same value of Type. Then you would end up with unreferenced streams in the file. Maybe instead we can just store the ArrayRef, overwriting any previous value of the ArrayRef that was there before. Then in finalizeMsfLayout we can add all the streams.

This way everything is semantically "atomic". No modification to the underlying file happens until you call finalize(), so you can do whatever you want until then without risk of messing anything up.

246–249 ↗

(On Diff #73990)

Ahh, I didn't see that before. I will go back and comment up there.

[ruiu]

lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp

49 ↗

(On Diff #74155)

How about this? I think this is atomic because we check everything that can fail before calling addStream in this function.

[ruiu]

• Updated as per the review comments.

[zturner]

Hmm, yea I guess you are right.

llvm/trunk/include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h

 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 
 #include "llvm/DebugInfo/MSF/ByteStream.h"
 #include "llvm/DebugInfo/MSF/StreamReader.h"
 #include "llvm/DebugInfo/PDB/PDBTypes.h"
 #include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Raw/RawConstants.h"
+#include "llvm/Support/Endian.h"
 
 namespace llvm {
 namespace msf {
 class MSFBuilder;
 }
 namespace pdb {
 class DbiStream;
 struct DbiStreamHeader;
public:
   void setVersionHeader(PdbRaw_DbiVer V);
   void setAge(uint32_t A);
   void setBuildNumber(uint16_t B);
   void setPdbDllVersion(uint16_t V);
   void setPdbDllRbld(uint16_t R);
   void setFlags(uint16_t F);
   void setMachineType(PDB_Machine M);
 
+  // Add given bytes as a new stream.
+  Error addDbgStream(pdb::DbgHeaderType Type, ArrayRef<uint8_t> Data);
+
   uint32_t calculateSerializedLength() const;
 
   Error addModuleInfo(StringRef ObjFile, StringRef Module);
   Error addModuleSourceFile(StringRef Module, StringRef File);
 
   Error finalizeMsfLayout();
 
   Expected<std::unique_ptr<DbiStream>> build(PDBFile &File,
                                              const msf::WritableStream &Buffer);
   Error commit(const msf::MSFLayout &Layout,
                const msf::WritableStream &Buffer);
 
 private:
+  struct DebugStream {
+    ArrayRef<uint8_t> Data;
+    uint16_t StreamNumber = kInvalidStreamIndex;
+  };
+
   Error finalize();
   uint32_t calculateModiSubstreamSize() const;
   uint32_t calculateFileInfoSubstreamSize() const;
   uint32_t calculateNamesBufferSize() const;
 
   Error generateModiSubstream();
   Error generateFileInfoSubstream();
 
private:
   StringMap<std::unique_ptr<ModuleInfo>> ModuleInfos;
   std::vector<ModuleInfo *> ModuleInfoList;
 
   StringMap<uint32_t> SourceFileNames;
 
   msf::WritableStreamRef NamesBuffer;
   msf::MutableByteStream ModInfoBuffer;
   msf::MutableByteStream FileInfoBuffer;
+  llvm::SmallVector<DebugStream, (int)DbgHeaderType::Max> DbgStreams;
 };
 }
 }
 
 #endif

llvm/trunk/include/llvm/DebugInfo/PDB/Raw/PDBFileBuilder.h

 //===- PDBFileBuilder.h - PDB File Creation ---------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_PDB_RAW_PDBFILEBUILDER_H
 #define LLVM_DEBUGINFO_PDB_RAW_PDBFILEBUILDER_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
+#include "llvm/DebugInfo/PDB/Raw/RawConstants.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 
 #include <memory>
 #include <vector>
 
 namespace llvm {

llvm/trunk/lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp

 //===- DbiStreamBuilder.cpp - PDB Dbi Stream Creation -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h"
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
 #include "llvm/DebugInfo/MSF/MappedBlockStream.h"
 #include "llvm/DebugInfo/MSF/StreamWriter.h"
 #include "llvm/DebugInfo/PDB/Raw/DbiStream.h"
 #include "llvm/DebugInfo/PDB/Raw/RawError.h"
 
 using namespace llvm;
 using namespace llvm::codeview;

 void DbiStreamBuilder::setPdbDllVersion(uint16_t V) { PdbDllVersion = V; }
 
 void DbiStreamBuilder::setPdbDllRbld(uint16_t R) { PdbDllRbld = R; }
 
 void DbiStreamBuilder::setFlags(uint16_t F) { Flags = F; }
 
 void DbiStreamBuilder::setMachineType(PDB_Machine M) { MachineType = M; }
 
+Error DbiStreamBuilder::addDbgStream(pdb::DbgHeaderType Type,
+                                     ArrayRef<uint8_t> Data) {
+  if (DbgStreams[(int)Type].StreamNumber == kInvalidStreamIndex)
+    return make_error<RawError>(raw_error_code::duplicate_entry,
+                                "The specified stream type already exists");
+  auto ExpectedIndex = Msf.addStream(Data.size());
+  if (!ExpectedIndex)
+    return ExpectedIndex.takeError();
+  uint32_t Index = std::move(*ExpectedIndex);
+  DbgStreams[(int)Type].Data = Data;
+  DbgStreams[(int)Type].StreamNumber = Index;
+  return Error::success();
+}
+
 uint32_t DbiStreamBuilder::calculateSerializedLength() const {
   // For now we only support serializing the header.
   return sizeof(DbiStreamHeader) + calculateFileInfoSubstreamSize() +
-         calculateModiSubstreamSize();
+         calculateModiSubstreamSize() + DbgStreams.size() * sizeof(uint16_t);
 }
 
 Error DbiStreamBuilder::addModuleInfo(StringRef ObjFile, StringRef Module) {
   auto Entry = llvm::make_unique<ModuleInfo>();
   ModuleInfo *M = Entry.get();
   Entry->Mod = Module;
   Entry->Obj = ObjFile;
   auto Result = ModuleInfos.insert(std::make_pair(Module, std::move(Entry)));
Error DbiStreamBuilder::finalize() {
   H->Flags = Flags;
   H->PdbDllRbld = PdbDllRbld;
   H->PdbDllVersion = PdbDllVersion;
   H->MachineType = static_cast<uint16_t>(MachineType);
 
   H->ECSubstreamSize = 0;
   H->FileInfoSize = FileInfoBuffer.getLength();
   H->ModiSubstreamSize = ModInfoBuffer.getLength();
-  H->OptionalDbgHdrSize = 0;
+  H->OptionalDbgHdrSize = DbgStreams.size() * sizeof(uint16_t);
   H->SecContrSubstreamSize = 0;
   H->SectionMapSize = 0;
   H->TypeServerSize = 0;
   H->SymRecordStreamIndex = kInvalidStreamIndex;
   H->PublicSymbolStreamIndex = kInvalidStreamIndex;
   H->MFCTypeServerIndex = kInvalidStreamIndex;
   H->GlobalSymbolStreamIndex = kInvalidStreamIndex;
 
Error DbiStreamBuilder::commit(const msf::MSFLayout &Layout,
   StreamWriter Writer(*InfoS);
   if (auto EC = Writer.writeObject(*Header))
     return EC;
 
   if (auto EC = Writer.writeStreamRef(ModInfoBuffer))
     return EC;
   if (auto EC = Writer.writeStreamRef(FileInfoBuffer))
     return EC;
+  for (auto &Stream : DbgStreams)
+    if (auto EC = Writer.writeInteger(Stream.StreamNumber))
+      return EC;
+
+  for (auto &Stream : DbgStreams) {
+    if (Stream.StreamNumber == kInvalidStreamIndex)
+      continue;
+    auto WritableStream = WritableMappedBlockStream::createIndexedStream(
+        Layout, Buffer, Stream.StreamNumber);
+    StreamWriter DbgStreamWriter(*WritableStream);
+    if (auto EC = DbgStreamWriter.writeArray(Stream.Data))
+      return EC;
+  }
 
   if (Writer.bytesRemaining() > 0)
     return make_error<RawError>(raw_error_code::invalid_format,
                                 "Unexpected bytes found in DBI Stream");
   return Error::success();
 }