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

97–98

I believe these should be initialized to kInvalidStreamIndex.

97–99

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

239–242

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

288–289

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

290

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

97–99

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

239–242

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

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.

239–242

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

[ruiu]

lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp

49

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.

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

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);
+  void setDebugStreamIndex(DbgHeaderType Type, uint32_t Index);
 
   uint32_t calculateSerializedLength() const;
 
   Error addModuleInfo(StringRef ObjFile, StringRef Module);
   Error addModuleSourceFile(StringRef Module, StringRef File);
 
   Error finalizeMsfLayout();
 
private:
   StringMap<std::unique_ptr<ModuleInfo>> ModuleInfos;
   std::vector<ModuleInfo *> ModuleInfoList;
 
   StringMap<uint32_t> SourceFileNames;
 
   msf::WritableStreamRef NamesBuffer;
   msf::MutableByteStream ModInfoBuffer;
   msf::MutableByteStream FileInfoBuffer;
   std::vector<support::ulittle16_t> DbgStreams = {
       (size_t)DbgHeaderType::Max, (support::ulittle16_t)0};
 };
 }
 }
 
 #endif

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

public:
   Error initialize(uint32_t BlockSize);
 
   msf::MSFBuilder &getMsfBuilder();
   InfoStreamBuilder &getInfoBuilder();
   DbiStreamBuilder &getDbiBuilder();
   TpiStreamBuilder &getTpiBuilder();
   TpiStreamBuilder &getIpiBuilder();
 
+  // Add given bytes as a new stream. Returns the stream index.
+  Expected<uint32_t> addStream(ArrayRef<uint8_t> Data);
+
   Expected<std::unique_ptr<PDBFile>>
   build(std::unique_ptr<msf::WritableStream> PdbFileBuffer);
 
   Error commit(StringRef Filename);
 
 private:
   Expected<msf::MSFLayout> finalizeMsfLayout() const;
 
   BumpPtrAllocator &Allocator;
 
   std::unique_ptr<msf::MSFBuilder> Msf;
   std::unique_ptr<InfoStreamBuilder> Info;
   std::unique_ptr<DbiStreamBuilder> Dbi;
   std::unique_ptr<TpiStreamBuilder> Tpi;
   std::unique_ptr<TpiStreamBuilder> Ipi;
+  std::vector<std::pair<ArrayRef<uint8_t>, uint32_t>> Streams;
 };
 }
 }
 
 #endif

lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp

 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; }
 
+void DbiStreamBuilder::setDebugStreamIndex(DbgHeaderType Type, uint32_t Index) {
+  DbgStreams[(int)Type] = Index;
+}
+
 uint32_t DbiStreamBuilder::calculateSerializedLength() const {
   // For now we only support serializing the header.
   return sizeof(DbiStreamHeader) + calculateFileInfoSubstreamSize() +
          calculateModiSubstreamSize() +
          DbgStreams.size() * sizeof(DbgStreams[0]);
 }
 
 Error DbiStreamBuilder::addModuleInfo(StringRef ObjFile, StringRef Module) {
Error DbiStreamBuilder::finalize() {
   H->MFCTypeServerIndex = kInvalidStreamIndex;
   H->GlobalSymbolStreamIndex = kInvalidStreamIndex;
 
   Header = H;
   return Error::success();
 }
 
 Error DbiStreamBuilder::finalizeMsfLayout() {
   uint32_t Length = calculateSerializedLength();
   if (auto EC = Msf.setStreamSize(StreamDBI, Length))
     return EC;
   return Error::success();
 }
 
 Expected<std::unique_ptr<DbiStream>>
 DbiStreamBuilder::build(PDBFile &File, const msf::WritableStream &Buffer) {
   if (!VerHeader.hasValue())
     return make_error<RawError>(raw_error_code::unspecified,
                                 "Missing DBI Stream Version");
   if (auto EC = finalize())
Error DbiStreamBuilder::commit(const msf::MSFLayout &Layout,
   if (auto EC = Writer.writeStreamRef(FileInfoBuffer))
     return EC;
   if (auto EC = Writer.writeArray(ArrayRef<support::ulittle16_t>(DbgStreams)))
     return EC;
 
   if (Writer.bytesRemaining() > 0)
     return make_error<RawError>(raw_error_code::invalid_format,
                                 "Unexpected bytes found in DBI Stream");
   return Error::success();
 }

lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp

 }
 
 TpiStreamBuilder &PDBFileBuilder::getIpiBuilder() {
   if (!Ipi)
     Ipi = llvm::make_unique<TpiStreamBuilder>(*Msf, StreamIPI);
   return *Ipi;
 }
 
+Expected<uint32_t> PDBFileBuilder::addStream(ArrayRef<uint8_t> Data) {
+  auto ExpectedIndex = Msf->addStream(Data.size());
+  if (!ExpectedIndex)
+    return ExpectedIndex.takeError();
+  uint32_t Index = std::move(*ExpectedIndex);
+
+  Streams.emplace_back(Data, Index);
+  return *ExpectedIndex;
+}
+
 Expected<msf::MSFLayout> PDBFileBuilder::finalizeMsfLayout() const {
   if (Info) {
     if (auto EC = Info->finalizeMsfLayout())
       return std::move(EC);
   }
   if (Dbi) {
     if (auto EC = Dbi->finalizeMsfLayout())
       return std::move(EC);
if (auto EC = Tpi->commit(Layout, Buffer))
       return EC;
   }
 
   if (Ipi) {
     if (auto EC = Ipi->commit(Layout, Buffer))
       return EC;
   }
 
+  for (auto &Pair : Streams) {
+    ArrayRef<uint8_t> Data = Pair.first;
+    uint32_t Idx = Pair.second;
+    auto Stream = WritableMappedBlockStream::createIndexedStream(Layout, Buffer, Idx);
+    StreamWriter Writer(*Stream);
+    if (auto EC = Writer.writeArray(Data))
+      return EC;
+  }
+
   return Buffer.commit();
 }