[PATCH] D22974: pdbdump: Dump Free Page Map contents.

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/DebugInfo/PDB/Raw/PDBFile.cpp:52
@@ +51,3 @@
+// because otherwise the result would be large, as most pages are unused.)
+Expected<std::vector<uint32_t>> PDBFile::getUsedBlockList() const {
+  uint64_t Offset = ContainerLayout.SB->FreeBlockMapBlock * getBlockSize();
----------------
ruiu wrote:
> majnemer wrote:
> > ruiu wrote:
> > > zturner wrote:
> > > > ruiu wrote:
> > > > > zturner wrote:
> > > > > > How about adding another field to the `MsfLayout` class called `FreePageMap`?  You will need to set the value in `MsfBuilder::build()` before returning, and you will also need to set it in `PDBFile::parseFileHeaders()`.
> > > > > > 
> > > > > > This way you wouldn't need to return an `Expected<std::vector<uint32_t>>` but rather a `const BitVector&`, and the reading / writing code would share the same common structures.
> > > > > What is a bit annoying is that BitVector doesn't provide a way to map itself onto an existing bitmap, so I need to examine each bit and flip a bit in a BitVector. But, because the number of bits we have here is small anyways, it is probably okay to copy bit by bit. I'll try to add BitVector to MsfLayout.
> > > > It seems like we could just add a new constructor to the `BitVector` class like this:
> > > > 
> > > > `BitVector(BitWord *Buffer, uint32_t NumBits)`;
> > > BitWord is an internal thing of BitVector, so it probably should be `uint8_t *` and NumBits, or ArrayRef<uint8_t>. I'll try to see if I can add a new constructor.
> > It already has `BitVector::setBitsInMask`.
> I took a look at the function and noticed that it takes a pointer to uint32_t. So the endianess matters here, so I cannot use it for an array read from file (unless swapping bytes before passing it to `setBitsInMask`).
You could just loop and call setBitsInMask.


https://reviews.llvm.org/D22974