pdbdump: Write a free page map to page 1 or page 2.

[ruiu]

Summary

As I wrote in a comment, llvm-pdbdump seems to not be able to restore
stream 0 correctly. That is why we get different results for empty.pdb
and our pdb file.

This patch is to write only one FPM. So it still cannot produce large
PDBs which need more than one FPM page.

[ruiu]

I guess incremental patches are a preferred way, so in either way, don't you think submitting this is a good idea?

[zturner]

lib/DebugInfo/MSF/MSFBuilder.cpp

261

Can we use false instead of 0?

lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp

130

To fix the page 0 problem, how about this:

auto L = Layout.FreePageMap;
for (uint32_t BlockNumber : Layout.StreamMap[0].second)
  L[BlockNumber] = true;

and then write L instead of Layout.FreePageMap?

[ruiu]

Support split free page map blocks.

At first, I tried to add FreePageMapBlocks to MSFLayout (as analogous
to DirectoryBlocks) to store the block numbers for split FPMs. But
then I realized that that information can be computed later because the
block layout for the split FPM is fixed. So I moved the logic to
PDBFileBuilder.

[zturner]

lib/DebugInfo/MSF/MSFBuilder.cpp

259

Use msf::getNumFpmIntervals()

lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp

130–144

This whole block can be written as:

auto FpmStream = WritableMappedBlockStream::createFpmStream(Layout);
StreamWriter FpmWriter(FpmStream);
if (auto EC = FpmWriter.writeBytes(Layout.FreePageMap.getAsByteArray()))
  return EC;

You would need to create the function BitVector::getAsByteArray() and have it return an ArrayRef<uint8_t>

[ruiu]

• Define BitVector::getAsByteVector.

[ruiu]

We cannot return an ArrayRef to the internal buffer because of endianness issue. The internal vector is not a byte array, but an array of the word size (32 or 64 bits), so if we return it as a byte array, the result would be different on the BE than on the LE machine. One thing we could do is to return a std::vector as I did in this patch. I'm not sure if this is the right thing to do, though. The other approach would be to use byte instead of word as the internal buffer type. Do you want me to try it?

[ruiu]

• Updated to remove changes to BitVector.

lib/DebugInfo/MSF/MSFBuilder.cpp

if (NumDirectoryBlocks > DirectoryBlocks.size()) {
     DirectoryBlocks.resize(NumDirectoryBlocks);
   }
 
   // Don't set the number of blocks in the file until after allocating Blocks
   // for the directory, since the allocation might cause the file to need to
   // grow.
   SB->NumBlocks = FreeBlocks.size();
 
+  // Allocate discontiguous free page map blocks. If one page is too
+  // small to store the entire FPM, two or more pages are allocated
+  // for the FPM.
+  uint32_t NumFPMs = alignTo(FreeBlocks.size(), BlockSize) / BlockSize;
+  for (uint32_t I = 0; I != NumFPMs; ++I) {
+    FreeBlocks[I * BlockSize + kFreePageMap0Block] = false;
+    FreeBlocks[I * BlockSize + kFreePageMap1Block] = false;
+  }
+
+  // Copy free page map. Note that on-disk FPM is slightly different from
+  // in-memory FPM since the pages for stream 0 are marked unused on-disk.
+  // We need to do that for compatibility.
+  L.FreePageMap = FreeBlocks;
+  for (uint32_t BlockNumber : StreamData[0].second)
+    L.FreePageMap[BlockNumber] = false;
+
   ulittle32_t *DirBlocks = Allocator.Allocate<ulittle32_t>(NumDirectoryBlocks);
   std::uninitialized_copy_n(DirectoryBlocks.begin(), NumDirectoryBlocks,
                             DirBlocks);
   L.DirectoryBlocks = ArrayRef<ulittle32_t>(DirBlocks, NumDirectoryBlocks);
 
   // The stream sizes should be re-allocated as a stable pointer and the stream
   // map should have each of its entries allocated as a separate stable pointer.
   if (StreamData.size() > 0) {

lib/DebugInfo/PDB/Raw/PDBFileBuilder.cpp

PDBFileBuilder::build(std::unique_ptr<msf::WritableStream> PdbFileBuffer) {
   if (File->Info && File->Dbi && File->Info->getAge() != File->Dbi->getAge())
     return llvm::make_error<RawError>(
         raw_error_code::corrupt_file,
         "PDB Stream Age doesn't match Dbi Stream Age!");
 
   return std::move(File);
 }
 
+// Returns the bitmap as a vector in the little-endian byte order.
+// Because we don't know if the BitVector's internal buffer is in
+// the little endian, we copy bit-by-bit. It is not fast but is
+// negligible because we don't have that many bits in the FPM.
+static std::vector<uint8_t> getAsByteVector(const BitVector &BV) {
+  std::vector<uint8_t> V(alignTo(BV.size(), 8) / 8);
+  for (unsigned I = 0, E = BV.size(); I != E; ++I)
+    if (BV[I])
+      V[I / 8] |= (1U << (I % 8));
+  return V;
+}
+
 Error PDBFileBuilder::commit(const msf::WritableStream &Buffer) {
   StreamWriter Writer(Buffer);
   auto ExpectedLayout = finalizeMsfLayout();
   if (!ExpectedLayout)
     return ExpectedLayout.takeError();
   auto &Layout = *ExpectedLayout;
 
   if (auto EC = Writer.writeObject(*Layout.SB))
     return EC;
+
+  // Write free page map.
+  auto FpmStream = WritableMappedBlockStream::createFpmStream(Layout, Buffer);
+  StreamWriter FpmWriter(*FpmStream);
+  if (auto EC = FpmWriter.writeBytes(getAsByteVector(Layout.FreePageMap)))
+    return EC;
+
+  // Write block map.
   uint32_t BlockMapOffset =
       msf::blockToOffset(Layout.SB->BlockMapAddr, Layout.SB->BlockSize);
   Writer.setOffset(BlockMapOffset);
   if (auto EC = Writer.writeArray(Layout.DirectoryBlocks))
     return EC;
 
   auto DirStream =
       WritableMappedBlockStream::createDirectoryStream(Layout, Buffer);
   StreamWriter DW(*DirStream);
   if (auto EC =
           DW.writeInteger(static_cast<uint32_t>(Layout.StreamSizes.size())))
     return EC;
 
   if (auto EC = DW.writeArray(Layout.StreamSizes))
     return EC;
 
   for (const auto &Blocks : Layout.StreamMap) {
     if (auto EC = DW.writeArray(Blocks))
       return EC;
   }
 
   if (Info) {
     if (auto EC = Info->commit(Layout, Buffer))
       return EC;
   }
 
   if (Dbi) {
     if (auto EC = Dbi->commit(Layout, Buffer))
       return EC;
   }
 
   return Buffer.commit();
-}
+}
 

test/DebugInfo/PDB/pdbdump-fpm.test

+# RUN: llvm-pdbdump yaml2pdb -pdb=%t.pdb %s
+# RUN: llvm-pdbdump raw -page-stats %t.pdb | FileCheck %s
+
+# CHECK: Multiply Used Pages: []
+# CHECK: Use After Free Pages: []
+
+MSF:
+  SuperBlock:
+    BlockSize:       1024
+    FreeBlockMap:    2
+    NumBlocks:       2000
+    NumDirectoryBytes: 136
+    Unknown1:        0
+    BlockMapAddr:    24
+  NumDirectoryBlocks: 1
+  DirectoryBlocks: [ 23 ]
+  NumStreams:      1
+  FileSize:        2048000
+StreamSizes:     [ 40 ]
+StreamMap:
+  - Stream:          [ 8 ]

test/DebugInfo/PDB/pdbdump-readwrite.test

 RUN: llvm-pdbdump pdb2yaml -dbi-module-info -dbi-module-source-info \
 RUN:   -dbi-stream -pdb-stream -stream-directory -stream-metadata \
 RUN:   %p/Inputs/empty.pdb > %t.1
 RUN: llvm-pdbdump yaml2pdb -pdb=%t.2 %t.1
 
-RUN: llvm-pdbdump raw -headers %p/Inputs/empty.pdb | FileCheck %s
-RUN: llvm-pdbdump raw -headers %t.2 | FileCheck %s
+RUN: llvm-pdbdump raw -headers %p/Inputs/empty.pdb | FileCheck -check-prefix=HDR %s
+RUN: llvm-pdbdump raw -headers %t.2 | FileCheck -check-prefix=HDR %s
 
-CHECK:      FileHeaders {
-CHECK-NEXT:   BlockSize: 4096
-CHECK-NEXT:   FreeBlockMap: 2
-CHECK-NEXT:   NumBlocks: 25
-CHECK-NEXT:   NumDirectoryBytes: 136
-CHECK-NEXT:   Unknown1: 0
-CHECK-NEXT:   BlockMapAddr: 24
-CHECK-NEXT:   NumDirectoryBlocks: 1
-CHECK-NEXT:   DirectoryBlocks: [23]
-CHECK-NEXT:   NumStreams: 17
-CHECK-NEXT: }
-CHECK-NEXT: PDB Stream {
-CHECK-NEXT:   Version: 20000404
-CHECK-NEXT:   Signature: 0x54E507E2
-CHECK-NEXT:   Age: 1
-CHECK-NEXT:   Guid: {0B355641-86A0-A249-896F-9988FAE52FF0}
-CHECK-NEXT: }
-CHECK-NEXT: DBI Stream {
-CHECK-NEXT:   Dbi Version: 19990903
-CHECK-NEXT:   Age: 1
-CHECK-NEXT:   Incremental Linking: Yes
-CHECK-NEXT:   Has CTypes: No
-CHECK-NEXT:   Is Stripped: No
-CHECK-NEXT:   Machine Type: x86
-CHECK-NEXT:   Symbol Record Stream Index:
-CHECK-NEXT:   Public Symbol Stream Index:
-CHECK-NEXT:   Global Symbol Stream Index:
-CHECK-NEXT:   Toolchain Version: 12.0
-CHECK-NEXT:   mspdb120.dll version: 12.0.31101
-CHECK-NEXT: }
+HDR:      FileHeaders {
+HDR-NEXT:   BlockSize: 4096
+HDR-NEXT:   FreeBlockMap: 2
+HDR-NEXT:   NumBlocks: 25
+HDR-NEXT:   NumDirectoryBytes: 136
+HDR-NEXT:   Unknown1: 0
+HDR-NEXT:   BlockMapAddr: 24
+HDR-NEXT:   NumDirectoryBlocks: 1
+HDR-NEXT:   DirectoryBlocks: [23]
+HDR-NEXT:   NumStreams: 17
+HDR-NEXT: }
+HDR-NEXT: PDB Stream {
+HDR-NEXT:   Version: 20000404
+HDR-NEXT:   Signature: 0x54E507E2
+HDR-NEXT:   Age: 1
+HDR-NEXT:   Guid: {0B355641-86A0-A249-896F-9988FAE52FF0}
+HDR-NEXT: }
+HDR-NEXT: DBI Stream {
+HDR-NEXT:   Dbi Version: 19990903
+HDR-NEXT:   Age: 1
+HDR-NEXT:   Incremental Linking: Yes
+HDR-NEXT:   Has CTypes: No
+HDR-NEXT:   Is Stripped: No
+HDR-NEXT:   Machine Type: x86
+HDR-NEXT:   Symbol Record Stream Index:
+HDR-NEXT:   Public Symbol Stream Index:
+HDR-NEXT:   Global Symbol Stream Index:
+HDR-NEXT:   Toolchain Version: 12.0
+HDR-NEXT:   mspdb120.dll version: 12.0.31101
+HDR-NEXT: }
+
+RUN: llvm-pdbdump raw -page-stats %p/Inputs/empty.pdb | FileCheck -check-prefix=FPM1 %s
+RUN: llvm-pdbdump raw -page-stats %t.2 | FileCheck -check-prefix=FPM2 %s
+
+FPM1:      Msf Free Pages: [3, 4, 5, 8, 9]
+FPM1-NEXT: Orphaned Pages: []
+FPM1-NEXT: Multiply Used Pages: []
+FPM1-NEXT: Use After Free Pages: []
+
+;; We are not able to correctly restore stream 0.
+;; That's why we get a different result.
+FPM2:      Msf Free Pages: [3, 4, 5, 9]
+FPM2-NEXT: Orphaned Pages: [8]
+FPM2-NEXT: Multiply Used Pages: []
+FPM2-NEXT: Use After Free Pages: []