Skip to content

fix(relinker): audit dependencies without imported functions - #4916

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
harry-z-kang:codex/issue-2776-dependency-audit
Oct 14, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
harry-z-kang:codex/issue-2776-dependency-audit

Conversation

@harry-z-kang

@harry-z-kang harry-z-kang commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

import_audit.py can report a converted title as complete when it still needs a library with no imported functions, such as Party.prx in #2776. The import registry has no entries for that dependency.

With --registry, the relinker now writes a companion <prefix>.needed.json for each converted executable (including --executable outputs) and guest module. It contains the host dependencies retained after bundled guest providers and their aliases are resolved. The audit reads these companions and reports missing dependencies with zero imports. Existing registry arrays and audits of older registries remain supported.

Companion files receive the same input-preservation checks as import registries, and a failed Windows PE patch preserves existing registries and companions. This improves the diagnostics associated with #2776; it does not claim to fix Resident Evil 4's runtime stops. The audit conservatively flags a missing dependency even when the Windows loader can omit it because it has no imports.

Overlap coordination

#4425, #4559, and #4606 are merged. This PR is rebased onto main 0defec438. The JSON-report conflict retains dependency_registries, needed_libraries, stops, and module-aware reference counts, and passes the complete result to main's atomic write_json(args.json, result). Upstream input validation, output-directory checks, Linux runpath validation, Windows resource handling, conversion diagnostics, and progress reporting are preserved.

#5269 remains the only merge-order dependency, following its author's proposed order. Its call-registry test overlaps our companion assertion. The tested combined resolution keeps weak: false in the expected strong-import entry, the weak: true fixture, and the .needed.json assertion for lib.so. Its commits are not included in this branch; this PR will be rebased after it lands.

Tested

  • Head 22b190ae0, based on main 0defec438.
  • Windows, MinGW-w64 GCC 15.2.0, Release relinker-only build: 151 CTest tests passed, 1 skipped (long_paths, because Windows long paths are disabled), 0 failed.
  • Linux, Ubuntu 24.04/WSL2, GCC 13.3.0, Release relinker-only build: 157/157 CTest tests passed.
  • Fixtures cover zero-import dependencies on Linux and Windows, strict unused-library filtering, SONAME aliases, multiple executables and shared modules, legacy registries, malformed companion metadata, and input/output collisions. The suites also include the merged atomic-write failure regressions and upstream input/output validation cases.
  • A separate integration checkout at 7420d0e89 combines this head with fix(tools): do not report weak imports as a title's current stop #5269 at b9acf3480. Release relinker builds and all seven relevant CTest targets passed on Windows and Linux: call_registry, guest_registry, needed_libraries, weak_imports, input_preservation, import_audit, and conversion_diagnostics. Its 62 audit tests completed successfully on each OS, with 2 POSIX-only skips on Windows and 1 Windows-only skip on Linux. This validates the pinned combined checkout, not a future rebase.
  • Conventions and whitespace checks passed. No full emulator build, title runtime, or GPU testing was performed.

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: fix(tools): do not report weak imports as a title's current stop #5269
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (Codex)

@harry-z-kang

Copy link
Copy Markdown
Contributor Author

@Lorkydey @MarcZX100 @Nixitov

I've listed #4425, #4559, and #4606 in this PR's Depends on. I will take #4916 after all three, following the existing #4425-before-#4606 agreement; #4559 can land at any point before #4916. I'll rebase this PR once those changes are in main. Please let me know if that conflicts with your planned order.

I checked a separate integration checkout on main e51428bf2, combining #4425 at 05d52e396, #4559 at acd821f7c, #4606 at 7eae85946, and this PR at d92f47c90. The conflict resolution preserves:

All 58 focused audit tests completed successfully on Windows (56 passed, 2 POSIX-only skips) and Linux (57 passed, 1 Windows-only skip). The zero-import executable/guest-module relinker fixtures passed on both platforms as well. This is validation of the pinned integration checkout, not of a future rebase or a full build of the other PRs.

The published branch carries only our contribution. I made our dependency parameter keyword-only to avoid consuming #4559's module argument positions, and replaced our placeholder module fixture with a valid ELF that #4559 can inspect. After rebasing onto main e51428bf2, the five affected CTest targets pass on Windows and Linux, and conventions/whitespace checks pass.

AI-assisted: yes (Codex).

@MarcZX10

Copy link
Copy Markdown
Contributor

No conflict with my planned order. The current heads of #4425, #4559, and #4606 are still open, and I agree with the proposed sequencing: keep #4425 ahead of #4606, let #4559 land before #4916 at any point, then rebase #4916 after all three are in main. Thanks for checking the combined integration and preserving each PR's changes.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@Nixitov

Nixitov commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Confirmed on our side. #4606 only adds a stops section (the first unresolved import, in app.registry.json order) and its tests; it does not change the registry order or anything #4425/#4559 touch. So #4425 → #4606, with #4559 landing before #4916 at any point, works for us.

Thanks for checking the combined checkout and for preserving the stops report and registry order in the integration.

@harry-z-kang
harry-z-kang force-pushed the codex/issue-2776-dependency-audit branch from d92f47c to 687be2d Compare October 13, 2026 17:49
@harry-z-kang

Copy link
Copy Markdown
Contributor Author

Rebased #4916 onto current main 18c438b10 and resolved the conflicts in main.cpp, GuestModuleBuilder.cpp, and import_audit.py. The new head is 687be2d8b.

#4559 and #4606 are now merged. Their module imports/exports, module-aware reference count, registry order, and stops report are preserved alongside the companion dependency metadata. The relinker's new conversion pipeline and progress reporting are also preserved. Companions now follow the current executable/module indexing, including each --executable output and shared guest modules.

I added regression cases for an executable and a secondary executable with different zero-import dependencies, followed by resolving the dependency through a shared bundled module, in both Linux and Windows output formats.

Full build/test validation ran on 0b167a486 against main 8cf6a0bc3. The final rebase to main 18c438b10 includes only an unrelated PlayGo change; the relinker, audit, and associated test files are identical in the published head.

  • Windows Release relinker-only build: 150 CTest tests passed, 1 skipped (long_paths, because Windows long paths are disabled), 0 failed.
  • Ubuntu 24.04/WSL2 Release relinker-only build: 156/156 CTest tests passed.
  • Local conventions and whitespace checks passed on the published head. No title runtime or GPU testing was performed.

Depends on now lists only #4425, which remains open. I also checked a separate integration checkout combining tested head 0b167a486 with #4425 at 05d52e396, preserving its atomic write_json(args.json, result) together with all report fields: the 58 focused audit tests completed successfully on Windows (56 passed, 2 POSIX-only skips) and Linux (57 passed, 1 Windows-only skip), and the expanded relinker fixtures passed on both. Those dependency commits are not carried in this PR; a rebase will still be needed when #4425 lands.

AI-assisted: yes (Codex).

@harry-z-kang
harry-z-kang force-pushed the codex/issue-2776-dependency-audit branch from 687be2d to 7e40354 Compare October 13, 2026 18:16
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@Nixitov

Nixitov commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Flagging from the other side of the overlap bot: #5269 adds a weak field to each call-registry entry ({"weak": true|false}), and this PR adds the companion dependency metadata to the same test's expected entry. It is a same-file overlap only — resolve by keeping both fields.

#5269 is independent and already clean, while this PR waits on #4425, so #5269 can land first; I'll rebase it after #4916 if you'd rather go that way. The published head of #4916 keeps the stops report and registry order, which #5269 relies on.

AI-assisted: yes

@harry-z-kang

Copy link
Copy Markdown
Contributor Author

Agreed, @Nixitov: #5269 can land first. #4916 now lists both #4425 and #5269 in Depends on; those two are independent, and I will rebase after both land.

I reproduced the #5269 conflict in a separate checkout. The resolution keeps the expected strong entry's weak: false, the new weak: true fixture, and our .needed.json == ["lib.so"] assertion. The weak-import stop filtering, registry order, module-aware reference counts, and companion metadata are all retained. Combining #4425 also preserves all report fields and calls write_json(args.json, result).

Pinned validation used main dacd78525, #4916 at 2e2452968, #5269 at 8f7edccb3, and #4425 at 05d52e396, producing integration checkout 042c4923f:

  • Fresh Release relinker builds passed on Windows (MinGW-w64 GCC 15.2.0) and Ubuntu 24.04/WSL2 (GCC 13.3.0).
  • All six relevant CTest targets passed on each OS: call_registry, guest_registry, needed_libraries, weak_imports, input_preservation, and import_audit.
  • All 59 focused audit tests completed successfully: Windows 57 passed / 2 POSIX-only skips; Linux 58 passed / 1 Windows-only skip.

The published branch is rebased onto main dacd78525 and carries only our contribution, retaining the configured Git author/committer. Its relinker, audit, test, and build files are unchanged from the fully validated head documented in the PR; conventions and whitespace checks pass. The dependency commits and combined conflict resolution remain in the separate checkout until they land. This is pinned compatibility validation, with no title runtime or GPU testing.

AI-assisted: yes (Codex).

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@harry-z-kang
harry-z-kang force-pushed the codex/issue-2776-dependency-audit branch from 2e24529 to 22b190a Compare October 14, 2026 02:46
@harry-z-kang

Copy link
Copy Markdown
Contributor Author

Rebased onto main 0defec438; the new head is 22b190ae0.

#4425 is now merged. I resolved the JSON-report conflict by retaining dependency_registries, needed_libraries, stops, and the other existing report fields, then calling write_json(args.json, result). The rebase also preserves main's newer input validation, output-directory checks, Linux runpath handling, Windows resource handling, and conversion diagnostics.

Validation on the rebased head, using Release relinker-only builds:

  • Windows / MinGW-w64 GCC 15.2.0: 151 CTest tests passed, 1 skipped (long_paths, because Windows long paths are disabled), 0 failed.
  • Ubuntu 24.04/WSL2 / GCC 13.3.0: 157/157 CTest tests passed.
  • Conventions and whitespace checks passed. The configured Git author and committer are preserved.

Depends on now lists only #5269. I refreshed the separate integration checkout against its head b9acf3480, producing 7420d0e89. Both OS builds and all seven relevant CTest targets pass, including conversion diagnostics and the combined weak-import and companion assertions. The 62 audit tests pass with platform skips (Windows 60 passed / 2 skipped; Linux 61 passed / 1 skipped). The published branch does not carry #5269's commits; a final rebase remains after it lands.

No full emulator build, title runtime, or GPU testing was performed.

AI-assisted: yes (Codex).

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@jotitadev

Copy link
Copy Markdown
Contributor

macOS data point, as a third host next to the Windows and Linux runs.

Apple M2 Max, macOS 27.2 (26B5091g), AppleClang, Ninja Release, relinker-only. Head 22b190ae0 merged without conflicts into main 84b01f9cd.

  • arm64: ctest --parallel 8 --timeout 30 157/157 pass, no skips. import_audit, needed_libraries, call_registry, guest_registry, weak_imports, input_preservation and conversion_diagnostics all pass.
  • x86_64 under Rosetta 2 (-DCMAKE_OSX_ARCHITECTURES=x86_64, x86_64 Python 3.9): the same seven pass. 6 other tests fail (pr_overlap_encoding, progress_history, stubgen_stdout_newlines, release_package, linux_fixed_base, guest_module_args), the same 6 that fail on unmodified main on this machine (five are Python 3.9 incompatibilities of the only x86_64 Python here; guest_module_args is the fixture test(relinker): keep the guest_module_args fixture's GOT out of executable memory #5362 fixes).

Not checked here: the combined checkout with #5269.

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

@adriadam10 adriadam10 self-assigned this Oct 14, 2026
@adriadam10
adriadam10 force-pushed the codex/issue-2776-dependency-audit branch from 22b190a to b2c9e91 Compare October 14, 2026 19:04
@adriadam10

Copy link
Copy Markdown
Collaborator

#5269 is merged, so I rebased this onto current main. The only conflict was in test_call_registry.py; resolved as proposed in the thread: kept #5269's weak: true fixture and your .needed.json == ["lib.so"] assertion. Relinker build and the import_audit, call_registry, guest_registry, needed_libraries, weak_imports, input_preservation, conversion_diagnostics tests pass on the rebased head. Merging once the full Build is green.

@adriadam10
adriadam10 merged commit 411e5e6 into boykopovar:main Oct 14, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants