Skip to content

Fix legacy PE COFF symbol addresses#8334

Open
plafosse wants to merge 3 commits into
devfrom
test_pe_coff_symbol_rvas
Open

Fix legacy PE COFF symbol addresses#8334
plafosse wants to merge 3 commits into
devfrom
test_pe_coff_symbol_rvas

Conversation

@plafosse

@plafosse plafosse commented Jul 19, 2026

Copy link
Copy Markdown
Member

Summary

  • Detect legacy IMAGE_DEBUG_TYPE_COFF symbol tables by validating their IMAGE_COFF_SYMBOLS_HEADER against the PE header's symbol-table pointer and count.
  • Treat IMAGE_SYMBOL.Value as an RVA only for that validated legacy form.
  • Preserve section-relative symbol handling for ordinary PE COFF tables.
  • Add a deterministic 5.6 KB synthetic PE32 regression fixture and its generator under view/pe/tests/fixtures.

Root cause

The PE loader always added the symbol's section RVA to IMAGE_SYMBOL.Value. Visual C++ 6 /DEBUGTYPE:COFF images can instead store an image-relative value in the legacy COFF debug table, so the section RVA was added twice. In the issue sample, _FXCLI_DebugDispatch was consequently created at 0x57eb80 instead of 0x57db80.

The discriminator is deliberately structural rather than heuristic: the debug entry must be IMAGE_DEBUG_TYPE_COFF, its raw data must contain a complete IMAGE_COFF_SYMBOLS_HEADER, and that header's symbol count and first-symbol location must exactly match the PE header. PointerToRawData is used because AddressOfRawData may be zero when debug data is outside mapped sections.

Fixes #4308.

Compatibility

The normal section-relative behavior introduced for #1956 remains the fallback. This avoids changing ordinary COFF tables, including MinGW-produced images without the legacy debug wrapper.

Validation

  • Built view_pe with cmake --build cmake-build-relwithdebinfo --target view_pe -j 8.
  • Verified the issue sample resolves:
    • _FXCLI_DebugDispatch to 0x57db80
    • _FXCLI_IF_NotifyOraBR to 0x57db01
  • Verified the Binary ninja fails to parse symbol tables in kotlin/native binary #1956 sample remains section-relative:
    • pre_c_init at 0x401010
    • pre_cpp_init at 0x401130
    • __tmainCRTStartup at 0x401180
  • Verified the synthetic fixture with llvm-readobj:
    • PE32/i386 with entry point RVA 0x1020
    • a separate legacy function symbol whose value is RVA 0x1010
    • IMAGE_DEBUG_TYPE_COFF, AddressOfRawData = 0, and PointerToRawData = 0x1600
    • SHA-256 7d3d2b3a45405e542d4c5644712cd11b27d5c08b968f7020e2960ecffb8dcba7
  • Confirmed the synthetic fixture reproduces the regression visibly:
    • Binary Ninja 5.3 creates _start at 0x401020 but incorrectly creates legacy at 0x402010
    • this branch creates distinct functions named legacy at 0x401010 and _start at 0x401020, with no symbol or function at 0x402010
  • Regenerated the fixture byte-for-byte from its script and ran git diff --check.

References

@plafosse

Copy link
Copy Markdown
Member Author

Here is a after/before of the same binary with/without this patch.

image

@plafosse
plafosse requested a review from negasora July 19, 2026 13:52
@plafosse
plafosse marked this pull request as ready for review July 19, 2026 13:52
@plafosse plafosse self-assigned this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PE COFF Debug symbols are parsed incorrectly

1 participant