Enable the pdbpagesize flag to allow linking of PDB files > 4GB. Also includes a couple small fixes to change to uint64_t to support the larger file sizes. I updated the max file size check in MSFBuilder.cpp to take into account the page size.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nice!
Before we turn this on, we should probably have some tests for /pdbpagesize: -- at least checking that llvm-pdbutil can read pdbs with different page size. If you have a pdb > 4gb sitting around locally, it'd also be good to verify that llvm-pdbutil dump or llvm-pdbutil pretty --native can open and read those large pdbs.
I'll add some unit tests over the weekend.
Here are some examples of a > 4GB PDB with 16k page size
D:\llvm-project\build\Debug\bin\llvm-pdbutil.exe pretty --native d:\scratch\4GBPDBWork\unit_tests\16384\unit_tests.exe.pdb
Summary for d:\scratch\4GBPDBWork\unit_tests\16384\unit_tests.exe.pdb Size: 4531617792 bytes Guid: {257AC4E5-274E-9019-4C4C-44205044422E} Age: 1 Attributes: HasPrivateSymbols
D:\llvm-project\build\Debug\bin\llvm-pdbutil.exe dump -summary d:\scratch\4GBPDBWork\unit_tests\16384\unit_tests.exe.pdb
Summary ============================================================ Block Size: 16384 Number of blocks: 276588 Number of streams: 18124 Signature: 628802789 Age: 1 GUID: {257AC4E5-274E-9019-4C4C-44205044422E} Features: 0x1 Has Debug Info: true Has Types: true Has IDs: true Has Globals: true Has Publics: true Is incrementally linked: false Has conflicting types: false Is stripped: false
(from the presubmit bots, looks like this needs an update to an existing test too: https://buildkite.com/llvm-project/premerge-checks/builds/68334#25aa8614-2e89-4dd7-a7ae-229dd5a587d3 => Failed Tests (1): lld :: COFF/pdbpagesize.test)
This basically lgtm. One comment to fix a diagnostic below.
I patched this in and checked that lld now indeed produces PDBs > 4GiB when needed (with /pdbpagesize:8192, with the repro in crbug.com/1179085) and that llvm-pdbutil can dump the summary of such large PDBs.
Do you need me to commit this for you?
llvm/lib/DebugInfo/MSF/MSFBuilder.cpp | ||
---|---|---|
360–361 | Take a look at llvm/lib/DebugInfo/MSF/MSFError.cpp: case msf_error_code::size_overflow: return "Output data is larger than 4 GiB."; The lld linker for > 4GB files is: lld-link: error: Output data is larger than 4 GiB. File size would have been 4,593,000,448 But we'll use the "larger than 4 GiB" text even with page size 8192. The easiest fix is probably to rename the current size_overflow to size_overflow_4096 and add size_overflow_8192, size_overflow_16384, size_overflow_32768. (Maybe there's a nicer fix too, but that'd work.) Maybe the string returned by MSFError.cpp could mention ", the maximum for a PDB with page size XXX" so that users get the idea that the page size is now configurable. |
Is --author='Chris Davis <chrdavis@microsoft.com>' good? Or should I use something else?
Take a look at llvm/lib/DebugInfo/MSF/MSFError.cpp:
The lld linker for > 4GB files is: lld-link: error: Output data is larger than 4 GiB. File size would have been 4,593,000,448
But we'll use the "larger than 4 GiB" text even with page size 8192.
The easiest fix is probably to rename the current size_overflow to size_overflow_4096 and add size_overflow_8192, size_overflow_16384, size_overflow_32768. (Maybe there's a nicer fix too, but that'd work.)
Maybe the string returned by MSFError.cpp could mention ", the maximum for a PDB with page size XXX" so that users get the idea that the page size is now configurable.