[PATCH] D25356: Define PDBFileBuilder::addStream to add stream.

[Rui Ueyama via llvm-commits]

On Thu, Oct 6, 2016 at 11:42 PM, Zachary Turner <zturner at google.com> wrote:

> Ok I think I understand this more now.  I don't think we should expose
> this method, because for one thing just writing the data to this stream is
> not sufficient to link it up to the PDB.  It has to be referenced from the
> DBI stream, and is one of the "optional debug streams" that comes at the
> end of the DBI stream.
>
> So instead, how about adding something to DbiStreamBuilder.  It could be
> called
>
> void setDbgStreamBytes(DbgHeaderType Type, ArrayRef<uint8_t> Bytes);
>

I tried this but I think I don't like the result. The problem is that this
method has to add a new stream to a PDB file, so DbiStreamBuilder is now
builds not only a DBI stream but other streams. I'd instead add this method
to PDBFileBuilder so that new streams are owned by a PDBFileBuilder.
PDBFileBuilder is now responsible to keep the debug indices consistent.

Then you call this as
>
> setDbgStreamBytes(DbgHeaderType::SectionHdr, SectionHeaderBytes);
>
> Then, in DbiStreamBuilder::commit(), it does the right thing for each
> optional debug info stream that has data (i.e. goes through the MsfLayout
> to create a new indexed stream, etc).  Also in DbiStreamBuilder::commit()
> you will need to write the optional debug info data at the very end of the
> dbi stream.
>
> On Thu, Oct 6, 2016 at 8:52 PM Zachary Turner <zturner at google.com> wrote:
>
>> Ahh, i see the other patch now.
>>
>> Wouldn't it be possible to create SectionHeaderStreamBuilder with a well
>> defined interface, then call MsfBuilder.getSectionHeaderBuilder()?
>> On Thu, Oct 6, 2016 at 8:50 PM Zachary Turner <zturner at google.com> wrote:
>>
>> What will this be used for? I tried to approach this with the mindset of
>> making it impossible to construct invalid pdb data, because your only
>> interface to writing is via a closed api. Allowing writing of arbitrary
>> bytes breaks this. What use did you have in mind?
>>
>>
>> On Thu, Oct 6, 2016 at 8:25 PM Rui Ueyama <ruiu at google.com> wrote:
>>
>> ruiu created this revision.
>> ruiu added a reviewer: zturner.
>> ruiu added a subscriber: llvm-commits.
>>
>> 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.
>>
>>
>> https://reviews.llvm.org/D25356
>>
>> Files:
>>   include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h
>>   include/llvm/DebugInfo/PDB/Raw/PDBFileBuilder.h
>>   lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp
>>   lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp
>>
>>
>> Index: lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp
>> ===================================================================
>> --- lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp
>> +++ lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp
>> @@ -66,6 +66,16 @@
>>    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())
>> @@ -189,5 +199,14 @@
>>        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();
>>  }
>> Index: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp
>> ===================================================================
>> --- lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp
>> +++ lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp
>> @@ -44,6 +44,10 @@
>>
>>  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() +
>> Index: include/llvm/DebugInfo/PDB/Raw/PDBFileBuilder.h
>> ===================================================================
>> --- include/llvm/DebugInfo/PDB/Raw/PDBFileBuilder.h
>> +++ include/llvm/DebugInfo/PDB/Raw/PDBFileBuilder.h
>> @@ -44,6 +44,9 @@
>>    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);
>>
>> @@ -59,6 +62,7 @@
>>    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;
>>  };
>>  }
>>  }
>> Index: include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h
>> ===================================================================
>> --- include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h
>> +++ include/llvm/DebugInfo/PDB/Raw/DbiStreamBuilder.h
>> @@ -44,6 +44,7 @@
>>    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;
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161007/707e4dad/attachment.html>