This is an archive of the discontinued LLVM Phabricator instance.

Enable pdbpagesize to allow support for PDB file sizes > 4GB
ClosedPublic

Authored by chrdavis on Dec 3 2021, 10:18 AM.

Details

Summary

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.

Diff Detail

Event Timeline

chrdavis created this revision.Dec 3 2021, 10:18 AM
chrdavis requested review of this revision.Dec 3 2021, 10:18 AM

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)

chrdavis updated this revision to Diff 391965.Dec 5 2021, 10:04 PM

Updated pdbpagesize test

chrdavis updated this revision to Diff 392132.Dec 6 2021, 11:14 AM

Fix for pdbpagesize unit tests

thakis accepted this revision.Dec 6 2021, 12:42 PM

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.

This revision is now accepted and ready to land.Dec 6 2021, 12:42 PM
chrdavis updated this revision to Diff 392184.Dec 6 2021, 2:24 PM

Update size overflow error text

@thakis - Yes if you could commit on my behalf I would appreciate it.

thakis added a comment.Dec 6 2021, 3:09 PM

Is --author='Chris Davis <chrdavis@microsoft.com>' good? Or should I use something else?

That works thanks!

This revision was landed with ongoing or failed builds.Dec 6 2021, 3:22 PM
This revision was automatically updated to reflect the committed changes.