[Bram910]

What

Bound redundant work when sectionless ELF GNU-hash buckets repeat a start index or overlap a previously traversed chain. Remember successfully read chain entries and stop a later walk at an already validated suffix, instead of repeatedly scanning it. Symbol counting, import/export classification and existing file-backed range diagnostics are unchanged. Well-formed tables already have disjoint chains and are unaffected; this bounds redundant traversal of malformed tables.

Add deterministic regressions for repeated buckets, descending overlapping starts, separate chains, empty buckets, unterminated chains and partially file-backed final words. This is import-audit parser hardening, not a title runtime or FPS change.

Review cleanup changes tests only: rename the file-backed-chain test to describe its coverage rather than imply overlap-specific behavior; identify chain reads by format and offsets, not diagnostic text; derive chain offsets from ELF program headers, the LOAD mapping and GNU-hash header metadata in one fixture helper.

Tested

• Based on main 60465afd3a; the original implementation and validation below use head ae32db8df4. The review cleanup changes only tests; production code is unchanged. Python tooling only; no local title, full-runtime, GPU or macOS run.
• Review cleanup head 93fe4010620a4bde382f397bb4a3fc30e1ae51be: full import-audit suite succeeds on Windows x64 Python 3.13.5 (137 run, 134 passed, 3 existing skips) and Ubuntu 22.04 under WSL Python 3.10.12 (137 run, 136 passed, 1 Windows-only skip). All three adjusted regression methods pass five repetitions on Linux. The registered Windows import_audit CTest also passes five consecutive runs (14.81-15.46 seconds each), with no timeout changes.
• Review cleanup negative controls: all three adjusted methods still pass with the production chain-entry diagnostic renamed; injecting a chain read into empty buckets makes all three empty-bucket subcases fail. Against the original main parser, all three read-count subcases fail while the two coverage-only methods pass. A separate fixture probe confirms the offset helper follows relocated program headers, a different LOAD virtual base and a changed bloom count.
• Hosted CI on original head ae32db8df4: Linux, Windows and both macOS relinker variants, ASan, conventions, eligibility, intake and labels succeed. Full-product Linux/Windows, register-context and write-watch jobs are skipped, not passing full-product evidence. These completed runs precede the test-only review cleanup.
• Red/green: with main's parser the three traversal subcases read 1,024, 528 and 48 chain words for 32 entries; with this change each reads exactly 32. Parsed imports, exports and dependencies remain identical. These are counted reads, not timing measurements.
• Windows x64, Python 3.13.5, MinGW-w64 GCC 14.1.0 UCRT, Release, ANYPS5_RELINKER_ONLY=ON, BUILD_TESTING=ON: cmake --build build-relinker and ctest --test-dir build-relinker --output-on-failure --parallel 4, 238/238 CTest entries pass. The direct import-audit suite runs 137 tests successfully with three existing platform/permission skips.
• Ubuntu 22.04 under WSL, Python 3.10.12: the import-audit suite runs 137 tests successfully with one Windows-only skip. All three new regression methods also pass five consecutive repetitions.
• Real GCC 11.4.0 shared libraries with 64 exports and SysV-only, GNU-only and combined hash tables: imports, exports and dependencies match main in all six sectioned/sectionless views. Another 500 seeded synthetic bucket/chain layouts, including empty, overlapping and truncated ranges, preserve main's results or exact diagnostics.
• Compatibility with fix(tools): validate ELF import table file ranges #10352 and fix(tools): reject dynamic tables without DT_NULL in import_audit #10311: both merge orders succeed and yield identical trees for each pair. The merged parser/test suites run 142 and 139 tests respectively on Windows, with three existing skips each. The Git-blob harness loads the merged parser and tests in-process; subprocess CLI smoke checks use this branch's on-disk tool. These runs preceded the final rebase, which did not change either audit file.
• python tools/check_conventions.py --base origin/main --head HEAD and git diff --check origin/main...HEAD pass.
• tools/pr_overlap.py --branch fix/audit-gnu-hash-chain-walks reports no confirmed conflicts, but cannot compare fix(tools): recognize guest module identities in import audit #6171 and fix(tools): resolve $ORIGIN-prefixed converted module dependencies in import_audit #7454 because they already conflict with main. Their shared-file patches were reviewed manually: neither changes GNU-hash chain traversal. fix(tools): recognize guest module identities in import audit #6171 also adds module_names to the test fixture signature extended here with gnu_buckets; the later merge/rebase must retain both optional parameters. fix(tools): resolve $ORIGIN-prefixed converted module dependencies in import_audit #7454 changes dependency-name matching. Neither is a functional prerequisite. The overlap scan used this unchanged patch before rebasing onto its fetched base 60465afd3a; this does not claim the other PRs' existing conflicts are resolved.

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
• No notes, investigation .md files or images added to the repository (attach them to this PR)
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: none
• Fixes: none
• Unblocks: none
• GPUs tested: n/a
• AI-assisted: yes, Codex

[github-actions]

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request	Shared files not compared
PR 6171	tools/import_audit.py tools/tests/test_import_audit.py
PR 7454	tools/import_audit.py tools/tests/test_import_audit.py

[MarcZX10]

Reviewed the GNU-hash traversal change and the overlap warnings. The set contains only chain indices whose entries were successfully read, so a repeated or overlapping bucket can safely stop at a validated suffix; the suffix has already contributed to maximum. Bounds and terminator handling still occur on the first traversal, and the new fixtures cover repeated/descending starts, independent/empty buckets, unterminated chains, and truncated file-backed ranges. I also manually checked the flagged shared-file diffs in #6171 and #7454; neither changes GNU-hash traversal. I ran the PR head’s import-audit unittest suite: 137 tests passed, 2 skipped in this environment. Hosted eligibility, conventions, intake, labels, relinker matrix, and ASAN pass; platform full builds were skipped by eligibility.

[THEMANnt]

Reviewed head ae32db8df4f65dea48e19677d4192376e6bc3411 against pinned base 60465afd3ac4925ee3d0faa537794a61bb2dc34e. No actionable findings in this focused review.

In tools/import_audit.py, elf_program_dynamic_symbols now remembers successfully read GNU hash chain entries. Repeated or overlapping bucket walks stop at a previously validated suffix, whose maximum symbol index is already accounted for. This preserves symbol counting and import/export classification while avoiding repeated reads. I inspected both callers and the tests for repeated and descending starts, separate chains, empty buckets, unterminated chains and partially file-backed final words. The supported improvement is bounded redundant traversal; the old code already advanced sequentially and checked file-backed bounds.

Controller-recorded local runs of tools/tests/test_import_audit.py used Windows AMD64 and Python 3.11.9. Baseline reported 134 tests: 131 passed and three skipped. Pristine PR and candidate each reported 137: 134 passed and three skipped. Against old production code, the candidate tests failed three subcases of the read-count method with 1,024, 528 and 48 reads instead of 32; parsed results were checked before those assertions. This demonstrates the intended regression without a setup failure. Recorded whitespace and repository-conventions checks also succeeded.

GitHub-reported CI succeeded for relinker (ubuntu-latest, g++), relinker (windows-latest, g++), both named macOS relinker variants, relinker-asan, intake, labels, eligibility and conventions. windows, linux, write-watch and register-context were skipped, providing no passing product-test evidence. The author's additional build, WSL and compatibility claims remain separate from these independently inspected local results. No local game or GPU validation was performed. The evidence supports approval.

[roboterspinne]

Reviewed head ae32db8 merged onto upstream/main 137a8fa (2026-10-24); the merge is clean, also together with #10618 (same file, pe_exports), check_conventions.py passes and pr_overlap.py lists only the two base-conflicted PRs (#6171, #7454) the bot already names.

Environment: Windows 11 Home 10.0.26200, Python 3.13.9 (Anaconda); Python only, no build needed.

Results

• python -m unittest tools.tests.test_import_audit on the merged tree: 139 tests, 136 pass, 3 skipped, 8.4 s. Red/green: against main's import_audit.py the read-count test fails in all three layouts with exactly the body's numbers (1024 != 32, 528 != 32, 48 != 32); the empty-bucket and file-backed-chain tests already pass on main, so they are coverage, not behaviour. All three pass on the branch.
• Own probe with the branch's fixture, both tools imported side by side: for the 32-entry table main reads 1024, 528 and 48 chain words for the three bucket layouts and the branch 32 each; imports, exports and dependencies are identical in every case, and the default one-bucket table reads 32 words on both.
• The loop reads entries in the same order as main and skips only entries an earlier walk has already read successfully, so the first failing read is the same on both and the error messages do not change; maximum is unaffected because a shared suffix already raised it.

Nits

• test_import_audit.py:1070: test_sectionless_elf_gnu_hash_overlapping_buckets_require_file_backed_chains never gets past its first bucket: in all four subtests that bucket's own walk reaches the unbacked chain entry and raises, so the visited set is never consulted and the test passes unchanged on main. It is a useful guard against tolerating unbacked chain entries (no other test catches that mutant), but the overlap in its name plays no role; a rename, or a layout such as (1, 3) with entry 2 terminated so the error comes from a later bucket after the set is populated, would match the name to what it covers.
• test_import_audit.py:1068: both mock-based tests find chain reads by the label string "GNU hash chain entry" from import_audit.py:129. If that label is renamed, the count test fails loudly, but this assertFalse(any(...)) quietly checks nothing. A shared constant for the label, or matching on the chain offset range, would remove the coupling.
• test_import_audit.py:1053 and the same block in the overlap test: the DYNAMIC lookup at offset 120 and chain_offset = tags[0x6ffffef5] - 0x400000 + 24 + len(buckets) * 4 hard-code build_sectionless_elf's layout (LOAD at file offset 0 / 0x400000, 16-byte header plus one bloom word) twice; deriving it from the LOAD header as test_sectionless_hash_metadata_must_be_file_backed does (address - load[3] + load[2]) would drop the duplication and the magic base.

Notes, nothing to change here: in linker output the chains of different buckets are disjoint contiguous runs that partition [symoffset, nsyms), and glibc's do_lookup_x walks one bucket's chain per lookup, so repeated or overlapping buckets occur only in malformed tables and main was already linear on well-formed ones; the body calls this hardening but could say that well-formed input is unaffected in one sentence. The visited set costs about 70 bytes per chain word read where main used constant memory; only a malformed table with a long unterminated chain makes that large, and it stays linear in file size. Of the open PRs touching these files (#10618, #10352, #10159, #9377, #9372, #7460, #7454, #6171), only #6171 touches build_sectionless_elf's signature, as the body says; #10159 is hidden from gh pr list --json files by its 100-file cap but does not touch the hash walk either.

Not checked: real ELF files (no ELF toolchain on this machine, so the body's GCC 11.4 libraries and the 500 seeded layouts were not reproduced), Linux and macOS runs, the manual three-way merges with #6171 and #7454.

I found no blocking issue: the branch reads the same set of chain words as main, each at most once, so it raises exactly when main raises with the same message and otherwise returns the same triple (a loop-invariant argument, and a 50,000-seed differential run over randomized buckets, terminators, symoffset and truncations with zero mismatches); maximum is unaffected because a shared suffix already raised it; the counter-check holds. For disclosure: I have open PRs (#10667 on tools/tests/test_suggest_patches.py, and relinker PRs #10658, #10646, #10607, #10596, #10564, #10403, #10402, #10401, #10391, #10390); none touches these files.

Written with AI assistance (Claude Code). I ran the tests and the read-count probe above on my Windows 11 machine.

[roboterspinne]

Nit: in all four subtests the first bucket's own walk reaches the unbacked chain entry and raises, so no later bucket runs and the visited set is never consulted; the test passes unchanged on main. It still guards against tolerating unbacked chain entries, but the overlap in its name plays no role. A rename, or a layout such as (1, 3) with entry 2 terminated so the error comes from the second bucket after the set is populated, would make the name match.

[roboterspinne]

Nit: this assertion is tied to the label string in import_audit.py:129. If that label is renamed, the count test above fails loudly, but this assertFalse(any(...)) quietly checks nothing. A shared constant, or matching on the chain offset range, would remove the coupling.

[Bram910]

Addressed all three nits in 93fe4010620a4bde382f397bb4a3fc30e1ae51be:

• Renamed the coverage-only case to test_sectionless_elf_gnu_hash_requires_file_backed_chains. It protects the file-backed-range check; it does not claim to exercise a later bucket or distinguish main from this fix.
• Both mock-based cases identify 32-bit reads by their offsets rather than the diagnostic label. The empty-bucket assertion checks from the computed chain start through the remaining image, so an unexpected first chain read is not hidden by an empty expected range.
• A shared fixture helper derives program-header positions from the ELF header, maps the GNU-hash address through LOAD, and reads bucket/bloom counts from the hash header. The repeated hard-coded DYNAMIC offset and virtual base are gone from these cases.

Production code is unchanged. The PR description now explicitly says well-formed tables are unaffected and separates the original completed hosted CI from this test-only update.

The registered Windows import_audit CTest also passes five consecutive runs, 14.81-15.46 seconds each, without timeout changes.

Validation: the complete 137-test suite succeeds on Windows Python 3.13.5 (134 pass, 3 existing skips) and Ubuntu 22.04/WSL Python 3.10.12 (136 pass, 1 Windows-only skip). All three adjusted methods pass five repetitions on Linux. Negative controls confirm that changing the production diagnostic label leaves these tests passing, while an injected empty-bucket chain read fails all three layouts. The original main parser still fails the three read-count subcases; the two coverage-only cases pass. A separate probe checks the fixture helper with relocated program headers, a different virtual base and a changed bloom count. Conventions and whitespace checks pass.

[roboterspinne]

@Bram910 Thanks. Re-checked 93fe401 merged onto upstream/main 5357acd on Windows 11, Python 3.13.9: import_audit.py is unchanged, the suite passes 139 tests (3 skipped) in 8 s, the read-count test still fails against main in its three layouts while the two coverage cases pass there, and with the diagnostic label renamed in a scratch copy of the tool both mock-based tests still pass, so the offset-based detection does what you describe. The helper reads the hash position through the LOAD segment and the bloom and bucket counts from the header, which removes the fixed base. The three nits are resolved from my side.

Written with AI assistance (Claude Code). I ran the tests above on my Windows 11 machine.

[MarcZX10]

Re-reviewed exact head 93fe4010620a4bde382f397bb4a3fc30e1ae51be. The test cleanup now derives chain offsets from ELF program headers and identifies reads by format/offset instead of diagnostic wording; it also separates the file-backed-range coverage from the overlap-performance regression. Local test_import_audit passes (136 passed, 1 skipped), with diff-check and conventions clean. The hosted relinker matrix and ASan pass; the author’s Windows run also exercised the diagnostic-label mutant.