[PATCH] D41734: [DebugInfo][PDB] Fix too many FPM blocks being written in some cases

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 4 11:25:06 PST 2018

zturner added a comment.

So here's the relevant code:


  CB cbpg = cbPg();    // (1)
  // Calc number of bytes needed to represent FPM, aligned to FPM::native_word sizes
  CB cbFpm = (pnMac() + CHAR_BIT - 1) / CHAR_BIT;   // (2)
  cbFpm = sizeof(FPM::native_word) * ((cbFpm + sizeof(FPM::native_word) - 1) / sizeof(FPM::native_word));   // (3)
  // Calc number of pages needed to represent that number of bytes
  UPN cpnFpm = (cbFpm + cbpg - 1) / cbpg;  // (4)

The way I read this code:

At (1), `cbpg = 4096`, the number of bytes in a page (always hardcoded to 4096 since we only support what they refer to as "BigMsf")
At (2), `cbFpm = ceil(# of pages / 8)`  Conceptually, this is since each byte refers to 8 pages, so this is really "# of bytes of fpm data needed given the number of pages we need to write"
At (3), 'cbFpm = # of bytes of fpm data needed rounded up to a multiple of pointer size`
At (4), `cpnFpm = # of bytes of fpm data needed rounded up to a multiple of pointer size and then rounded up to a multiple of block size`

rounding up to a multiple of pointer size and *then* rounding up to a multiple of block size seems to be redundant, so I interpret this as just:

- # of bytes of fpm data needed rounded up to a multiple of block size

But the question I kind of skipped over is "What is # of pages?  Does it include the FPM and super block."  I believe it does, For example, I ran `llvm-pdbutil` against `clang.pdb` which was generated by MSVC, and I get this:

  D:\src\llvmbuild\cl\Debug\x64>bin\llvm-pdbutil.exe dump --summary bin\clang.pdb
    Block Size: 4096
    Number of blocks: 167547
    Number of streams: 1442
    Signature: 1514918418
    Age: 3
    GUID: {D1F84556-D25E-7045-BF75-5D4A1D9274AB}
    Features: 0x1
    Has Debug Info: true
    Has Types: true
    Has IDs: true
    Has Globals: true
    Has Publics: true
    Is incrementally linked: true
    Has conflicting types: false
    Is stripped: false
  D:\src\llvmbuild\cl\Debug\x64>dir bin\clang.pdb
   Volume in drive D is New Volume
   Volume Serial Number is 2EC5-6F01
   Directory of D:\src\llvmbuild\cl\Debug\x64\bin
  01/03/2018  10:57 AM       686,272,512 clang.pdb
                 1 File(s)    686,272,512 bytes
                 0 Dir(s)   4,718,669,824 bytes free

We have to assume this PDB is authoritative since it was generated by MSVC, so whatever it does it correct by definition.

686272512 / 4096 = 167547, which matches exactly the number of blocks in the MSF header.

So based on this analysis, it looks like:

1. The "# of pages" field in the MSF header definitely includes the first 3 reserved pages.
2. The calculation currently used matches the calculation in the reference implementation.

So either my analysis is wrong (very likely, it's always hard analyzing this kind of dense code when we can't step through it in a debugger or even run the real code), or the bug lies elsewhere (perhaps in the write function?).

Interested to hear your thoughts though since you've already spent much more time thinking about this then I have :)



More information about the llvm-commits mailing list