Skip to content

radlink: fix MSF/PDB header page count and final-page extent - #935

Closed
honkstar1 wants to merge 1 commit into
EpicGames:devfrom
honkstar1:codex/msf-page-count-pr-20260903
Closed

radlink: fix MSF/PDB header page count and final-page extent#935
honkstar1 wants to merge 1 commit into
EpicGames:devfrom
honkstar1:codex/msf-page-count-pr-20260903

Conversation

@honkstar1

Copy link
Copy Markdown

Summary

Fix the MSF superblock's page_count so that page_count * page_size matches the serialized PDB length. This is a standalone correctness fix against dev; it does not depend on the optimization/compressed-OBJ PRs (#842, #892, #932) or the library lookup fix (#934).

A small Clang-built object linked with /debug:full exposed a PDB with 17 actual 4096-byte pages but a header declaring 31. Consumers which validate the superblock against file length reject that file with an unexpected-stream-length error.

Cause and fix

msf->page_count tracks allocator accounting, including preallocated free-page-map (FPM) pages beyond EOF. It is not the serialized file extent. msf_build_header() now obtains its page count from msf_get_save_size() after the stream table and root have been finalized. Allocation accounting is unchanged.

Boundary regressions also exposed two issues in that extent calculation:

  • Only interval slots 1 and 2 are reserved FPM pages. Slot 0 can contain stream data after the first interval, so skipping it can drop a live final page.
  • The minimum extent belongs to the absolute page number, not a bitmap-relative index. Applying it before the bitmap base can add two pages at a later bitmap's boundary.

Both are corrected so that agreement between the header and file size cannot merely conceal truncated stream data. See the MSF format documentation for the superblock and FPM layout.

Regression coverage

Tests are added to the existing src/linker/tests/linker_tests.c:

  • 22 in-memory cases cover both active FPMs, 512/4096-byte pages, sparse allocations, and before/at/after interval boundaries, including an entire bitmap's coverage.
  • Check the expected final page, header/file-size agreement, contiguous and page-list serialization, and byte-exact stream round trips.
  • Link a native COFF fixture and validate both full and stripped PDB file lengths. No external compiler is needed for these tests.

Validation

  • Confirmed the new in-memory regression fails on unmodified dev at the header/file-size assertion.
  • Standalone PR: release and debug, 8 passed, 0 failed/crashed/skipped (the two new regressions plus existing MSF, PDB-info, GSI, PSI and simple-link tests).
  • Same implementation in the combined development build: 163 passed, 0 failed/crashed, 12 MSVC-dependent tests excluded, in each of release and debug.
  • Clang probe: fixed PDB has 17 declared and 17 actual pages (69,632 bytes). LLVM pdbutil 20.1.8 reads full and stripped outputs at page sizes 512, 1024, 2048, 4096, 8192 and 16384.
  • Local integration link: DLL remained byte-identical; its 13,430,784-byte PDB differed only in the page-count field (3293 -> 3279).

Local builds used LLVM/Clang/LLD 20.1.8, not MSVC compiler/linker tools. Full application builds, UBA and symbol-upload infrastructure were not rerun for this fix. Existing affected PDBs need to be regenerated by relinking; this change does not patch them in place.

@NikitaSmith057

Copy link
Copy Markdown
Collaborator

Manually merged the pull request c690fd1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants