[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
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
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