Fix legacy PE COFF symbol addresses

[plafosse]

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

• PE COFF Debug symbols are parsed incorrectly #4308
• Binary ninja fails to parse symbol tables in kotlin/native binary #1956
• https://learn.microsoft.com/en-us/archive/msdn-magazine/2002/march/inside-windows-an-in-depth-look-into-the-win32-portable-executable-file-format-part-2
• https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-image_coff_symbols_header
• https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#debug-directory-image-only

[plafosse]

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

[zznop]

Changes look good. Just recommend changing the way this is being tested so it runs under CI.

[zznop]

Do we need to check in this script as opposed to just checking in the binary that it outputs?

[zznop]

Did you mean to check in this README?

[zznop]

I'm surprised to learn that there is no flag in IMAGE_SYMBOL that identifies whether a value is section-relative or image-relative. This looks like a decent approach.

[zznop]

Recommend moving this to tests/special_cases and writing a regression test that asserts we're handling it correctly. Currently, none of the added test fixtures in the PR are tested under CI.