Load the PE COFF symbol tests from fixtures instead of packing them

[zardus]

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Problem

tests/test_pe_coff_symbols.py builds the image it tests. _coff_symbol packs 18-byte COFF symbol records and _make_pe assembles a PE out of nothing:

def _make_pe(raw_data: bytes = b"", exports=()) -> Any:
    pe: Any = object.__new__(PE)
    pe._arch = archinfo.ArchAMD64()
    pe._raw_data = raw_data
    pe._pe = SimpleNamespace(
        FILE_HEADER=SimpleNamespace(PointerToSymbolTable=0, NumberOfSymbols=len(raw_data) // 18),
        sections=[SimpleNamespace(VirtualAddress=0x1000)],
        DIRECTORY_ENTRY_EXPORT=SimpleNamespace(symbols=exports),
    )

The symbol table starts at file offset 0, the image has one section, there is no string table and there are no auxiliary records; of the 110 PE images angr/binaries tracks, none of the 24 that declare symbols puts its table at offset 0. Break the reading of names too long for the eight-byte field -- 380 of the records in tests/x86_64/cfg_0_pe, which the new tests load, take that path -- and both tests still pass.

Root cause

PE._load_symbols_from_coff_header walks records that may name any of the image's sections, may take their name from the string table behind the table, and may be followed by auxiliary records. The mock reaches none of that, so it cannot reach the mistakes either: the IndexError this walk raises on a table numbered for some other section list, which #832 guards, was found by loading real images.

Fix

Read the same behaviour off committed fixtures. tests/x86_64/cfg_0_pe is a MinGW image whose 1341 records mix function and object types with external and local definitions; the walk visits 987 of them and takes 380 names out of the string table. tests/x86/windows/packed_pe32.exe keeps the unpacked file's symbol-table pointer and count in a file 0xbe00 bytes shorter, so its table ends past the end of the file and none of it loads. tests/x86_64/windows/msvcr120.dll exports 1925 symbols with no table to type them. coff_export_types.dll, added in angr/binaries#237, is the only image that carries both an export directory and a symbol table.

One assertion does not survive the move: that a local definition lends no type to an export. No image under tests/ puts an export at an address whose only COFF definition is local, so that one was only assertable against a manufactured table, and the map it inspected is internal to the loader.

Testing

Five tests replace the two. They are not vacuous, and the trade is visible: with name reading from the string table broken, the old file passed 2 of 2 and the new one fails 2 of 5; with the walk removed altogether, the old file fails 1 of 2 and the new one 2 of 5. #832 changes the same file and conflicts with this branch in it, so whichever lands first, the other needs a rebase. The fixture comes from angr/binaries#237

Validation: #846 (comment)

🤖 Generated with Claude Code

session: sharpen

[zardus]

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

What each version of tests/test_pe_coff_symbols.py notices, with three defects put back into cle's PE backend one at a time: names longer than eight bytes coming back empty from the string table, every storage class counting as an external definition, and the COFF symbol table not being read at all. Both sides are the complete output of the same probe, run against cle at 7c5e1a2c1e25524863ed5e778eef63b4d8bc56a1.

Before — the two tests read a table the test packed itself, and no record in it takes its name from the string table, so breaking that path changes nothing. All 23 readable symbol tables in angr/binaries' PE images do take at least one name from it, tests/x86_64/cfg_0_pe 380 of the 987 records its walk visits:

tests/test_pe_coff_symbols.py at cle master 7c5e1a2

old_tests.py, against cle at 7c5e1a2c with each defect put back one at a time

cle as it is: 2 pass, 0 fail
    pass                   test_coff_symbol_type_hints_only_include_external_definitions
    pass                   test_exports_inherit_only_unambiguous_coff_symbol_types

names longer than eight bytes come back empty: 2 pass, 0 fail
    pass                   test_coff_symbol_type_hints_only_include_external_definitions
    pass                   test_exports_inherit_only_unambiguous_coff_symbol_types

every storage class counts as an external definition: 1 pass, 1 fail
    FAIL AssertionError    test_coff_symbol_type_hints_only_include_external_definitions
    pass                   test_exports_inherit_only_unambiguous_coff_symbol_types

the COFF symbol table is not read at all: 1 pass, 1 fail
    FAIL AssertionError    test_coff_symbol_type_hints_only_include_external_definitions
    pass                   test_exports_inherit_only_unambiguous_coff_symbol_types

After — the five tests read committed fixtures, so a broken string table fails two of them; the storage-class filter is the one thing the manufactured table could check and the fixtures cannot, because no image under tests/ places an export at an address whose only COFF definition is local:

tests/test_pe_coff_symbols.py with this change

new_tests.py, against cle at 7c5e1a2c with each defect put back one at a time

cle as it is: 5 pass, 0 fail
    pass                   test_a_forwarded_export_is_a_function_and_names_its_library
    pass                   test_a_symbol_table_outside_the_file_loads_no_symbols
    pass                   test_coff_symbols_carry_the_type_the_table_declares
    pass                   test_exports_are_functions_when_no_symbol_table_types_them
    pass                   test_exports_take_the_type_of_their_coff_definition

names longer than eight bytes come back empty: 3 pass, 2 fail
    pass                   test_a_forwarded_export_is_a_function_and_names_its_library
    pass                   test_a_symbol_table_outside_the_file_loads_no_symbols
    FAIL AssertionError    test_coff_symbols_carry_the_type_the_table_declares
    pass                   test_exports_are_functions_when_no_symbol_table_types_them
    FAIL AssertionError    test_exports_take_the_type_of_their_coff_definition

every storage class counts as an external definition: 5 pass, 0 fail
    pass                   test_a_forwarded_export_is_a_function_and_names_its_library
    pass                   test_a_symbol_table_outside_the_file_loads_no_symbols
    pass                   test_coff_symbols_carry_the_type_the_table_declares
    pass                   test_exports_are_functions_when_no_symbol_table_types_them
    pass                   test_exports_take_the_type_of_their_coff_definition

the COFF symbol table is not read at all: 3 pass, 2 fail
    pass                   test_a_forwarded_export_is_a_function_and_names_its_library
    pass                   test_a_symbol_table_outside_the_file_loads_no_symbols
    FAIL AssertionError    test_coff_symbols_carry_the_type_the_table_declares
    pass                   test_exports_are_functions_when_no_symbol_table_types_them
    FAIL AssertionError    test_exports_take_the_type_of_their_coff_definition

[zardus]

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Validation record for head d132ea83287d842b84f3226842c0b470786518bd against baseline 7c5e1a2c1e25524863ed5e778eef63b4d8bc56a1.

Every row below was measured with the tree at that head.

• Regression: three defects put back into PE one at a time, each version of the test file run against each. With _read_from_string_table returning nothing, the old file passes 2 of 2 and the new one fails 2 of 5; with every storage class counting as an external definition, the old file fails 1 of 2 and the new one passes 5 of 5; with the COFF walk removed, the old fails 1 of 2 and the new 2 of 5. The complete output of both runs is in the before/after comment on this pull request
• Focused: pytest --import-mode=append -q tests/test_pe_coff_symbols.py — 5 passed
• Full suite: pytest over tests/ — 279 passed, 9 skipped in 14.83s
• Lint/type: pylint and pyright per changed file against the merge base, the way this repository's CI scores them — tests/test_pe_coff_symbols.py 10.00 to 10.00, and 0 pyright errors to 0
• Workspace gate: every suite ran over archinfo, pypcode, pyvex, pysoot, cle, angr and angr-management. Every suite that reads this repository passed. One of the fourteen failed, a test of the development workspace's own tooling that compares two lists of component names in that workspace and reads nothing here

Caveats: the new fixture tests/x86_64/windows/coff_export_types.dll comes from angr/binaries#237 and the two tests that read it cannot pass until it merges. The one assertion the old file made that no fixture here can make is that a local COFF definition lends no type to an export: no image under tests/ places an export at an address whose only COFF definition is local.

[angr-bot]

Corpus decompilation diffs can be found at angr/dec-snapshots@master...angr/cle_846

[zardus]

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Closing this pull request under the campaign scope: it does not establish qualifying impact on binaries recorded by our sweeps. We are withdrawing it from this campaign.

The change is confined to tests/test_pe_coff_symbols.py and alters no loader code, so there is no before and after to measure on a swept binary. That is the eligibility limitation here, and it says nothing about whether the change is worth having.