Skip to content

feat(relinker): strict sce module search - #2708

Merged
boykopovar merged 7 commits into
mainfrom
feat/strict-sce-module-search
Oct 10, 2026
Merged

boykopovar merged 7 commits into
mainfrom
feat/strict-sce-module-search

Conversation

@boykopovar

@boykopovar boykopovar commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

What

Add --sce-module-path for the parent directory of bundled modules. Default to the input ELF directory. Resolve explicit relative paths from the working directory. Reject combination with --skip-sce-module. Document path resolution.

The removal of recursive search is intentional. We want the behavior to be consistent every time, instead of trying to find missing files elsewhere. I'm currently fixing the tests; their failure is expected.

Tested

Windows: relinker build, DS

Checklist

@boykopovar boykopovar changed the title Feat/strict sce module search feat(relinker): strict sce module search Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#1782 core/relinker/relinker/src/guest/GuestModuleBuilder.cpp L94-135, L143-174
#2119 core/relinker/cli/src/CliArgs.cpp L9
core/relinker/main.cpp L102
core/relinker/relinker/include/relinker/guest/GuestImage.hpp L60
core/relinker/relinker/src/guest/GuestModuleBuilder.cpp L52-53
docs/user/USAGE.md L44-60

@MarcZX10

Copy link
Copy Markdown
Contributor

Follow-up from issue #2663 and current relinker checks

I traced the new right.sprx report against the current discovery code and corrected my earlier issue comment. When a direct dependency is unresolved, the identity fallback scans the whole input tree for .prx / .sprx / .suprx files and calls ReadModuleNames before it knows whether a candidate matches. A malformed PT_DYNAMIC in an unrelated sce_sys/about/right.sprx therefore aborts discovery. This is concrete evidence for limiting the scan to the configured module root; the bounds check itself should remain strict.

I also checked the failures on this PR head. The same five relinker tests fail on Linux, Windows, and macOS:

Test CI symptom Follow-up
linux_load_alignment Expects the previous “module directory not found” diagnostic; the new diagnostic changes the assertion before the --skip-sce-module path is checked. Update the expected diagnostic and keep the no-module conversion case.
guest_runpath Expected needed.prx.guest.prx artifacts are absent for modules in Media/Modules and Media/Modules/runtime. Update coverage to use the new supported module-root option, or preserve/document this previously supported layout.
guest_needed_modules The module under Media/Modules is no longer discovered. Same path compatibility question.
guest_module_identity The renamed.sprx provider in Media/Modules is no longer discovered. Keep a test for whichever identity-search locations remain supported.
guest_needed_nested_modules Providers under prx/shipping and nested sce_module / sce_modules directories are no longer emitted. Please confirm whether dropping nested discovery is intentional; if it is, update these expectations and document the supported layout.

The latest relinker matrix reports these failures on all three OSes (run 38049698989). Conventions also still fails on the PR head: run 38049712502 reports the unchecked “Based on current main” template item; the earlier run additionally reported the non-Conventional title, which appears fixed since then.

The scoped module-root approach looks relevant to #2663, but the current head needs the existing test suite reconciled with the new discovery contract before its results establish that the change is ready.

@DavidSamir

Copy link
Copy Markdown
Contributor

Checked this head (f756225) against a synthetic reproducer of #2663: an executable that needs a library the title does not bundle, plus sce_sys/about/right.sprx with PT_DYNAMIC past the end of the file. main 197ffac stops with Dynamic segment out of bounds; this branch converts it, for Linux and Windows output (macOS host, relinker-only build). A malformed .sprx inside sce_module is still rejected with its path.

Details and the reproducer cases are in #2663. If nested discovery is kept after all, DavidSamir:fix/relinker-module-search-skips-sce-sys is the smaller change for the same report: it only leaves sce_sys out of the walks, with tests, 90/90.

AI-assisted: yes

@MarcZX10

MarcZX10 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Update after the author clarified the intended discovery contract

I saw the updated PR description: recursive discovery is intentionally removed, with --sce-module-path defining the module parent. That resolves my compatibility question; the rewritten tests now reflect the intended contract.

The narrower #2663 change still provides a useful comparison for reviewers: excluding sce_sys also fixes the reported malformed right.sprx, while retaining recursive discovery. The reporter's native Windows 11 run passed 93/93 relinker tests and launched the original game; my Linux current-main red/green regression and 97/97 suite are summarized in issue #2663. The two proposals therefore have different intended behavior, not merely different implementations of the same contract.

Current checks on head 0bd2e4c Result
Conventions (run 38052605429) Passed after the checklist update
Ubuntu register-context Debug/Release Passed
macOS register-context Debug/Release and write-watch Passed
Linux, Windows and macOS relinker matrix Passed
Full Linux and Windows builds Skipped by the fork eligibility gate

The broader module-root change now has the intended behavior documented and its eligible CI checks pass. The full Linux/Windows Build still needs the collaborator-triggered run described in CONTRIBUTING.md.

@boykopovar

Copy link
Copy Markdown
Owner Author

The decision to abandon recursive search is intentional. I want the behavior to be consistent every time. I'm currently fixing the tests; their failure is expected.

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.

3 participants