Skip to content

fix(libc): prefer nested prx/*.guest.prx for RelinkedModulePath - #6544

Closed
notaxeltv wants to merge 1 commit into
boykopovar:mainfrom
notaxeltv:feat/libc-prefer-prx-guest-module
Closed

notaxeltv wants to merge 1 commit into
boykopovar:mainfrom
notaxeltv:feat/libc-prefer-prx-guest-module

Conversation

@notaxeltv

Copy link
Copy Markdown
Contributor

What

RelinkedModulePath only rewrote path → path.guest.prx in the same directory. Titles that keep unbound Engine.*.prx next to eboot.bin while the converted PE lives under prx/*.guest.prx (the layout #3123 documents) then LoadLibrary the same binary twice on Windows; the second image's vtables stay unbound and Job0 in Engine.Render.Core2 jumps to null.

After the same-directory .guest.prx miss, look under prx/, sce_module/ and sce_modules/ next to that path for the same filename before the existing case-insensitive match. Orthogonal to #6226 (stem match in the same directory).

Tested

Windows 10, WinLibs GCC 15.2.0. guest_dynamic_loader passes (covers nested prx/Engine.Render.prx.guest.prx). tools/check_conventions.py --base upstream/main passes. PPSA03541 with separate unbound root copies and nested guests: past Job0, presents HDR10, then aborts on COPY_DATA GDS (not implemented) — no null rip on Job0.

Checklist

Made with Cursor

Unbound copies next to eboot concurrently LoadLibrary the same PE and leave Job0 with a null vtable; reuse the already-loaded nested guest module instead.

Co-authored-by: Cursor <cursoragent@cursor.com>

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux run (Ubuntu 26.04, Intel i7-8700, GCC 15.2, full build, non-GPU tests), head 83e675746 merged onto main 44df21632.

  • Builds; guest_dynamic_loader and guest_dynamic_loader_fixed_base pass, including the new nested prx/Engine.Render.prx.guest.prx case.
  • With General.cpp from main and the PR's test, both abort at GuestDynamicLoader.cpp:109 (Require(nested != nullptr), the nested load), so the test protects the change on Linux too.

Non-GPU suite (ctest -E "^agc"): same result as main; only file_writer_binary and file_writer_text fail, and they fail on main too on this machine (Ubuntu 26.04 coreutils).

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

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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 5021 core/libs/prx/libc/src/General.cpp
PR 7201 core/libs/prx/libc/src/General.cpp
PR 9371 core/libs/prx/libc/src/General.cpp
PR 9693 core/libs/prx/libc/src/General.cpp
PR 10243 core/libs/tests/GuestDynamicLoader.cpp
PR 10487 core/libs/prx/libc/src/General.cpp
PR 10494 core/libs/prx/libc/src/General.cpp
core/libs/tests/GuestDynamicLoader.cpp
PR 10509 core/libs/prx/libc/src/General.cpp
PR 10975 core/libs/prx/libc/src/General.cpp

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks, the change looks right for the layout it targets (a converted copy in prx/ or sce_module/, the PPSA03541 double load on Windows). Two asks and a rebase:

  1. Same directory first. In the motivating layout, a load that already succeeds on main now resolves differently. Please check the same directory first and fall back to the nested lookup only when that misses, so layouts that work today keep their behaviour.
  2. Case-insensitive nested lookup, to match the existing same-directory matching.

#6226 changes the same function (RelinkedModulePath) for a different layout. The intended order once both land is: exact, then same directory ignoring case, then nested, then stem.

The branch is about 600 commits behind main, so please rebase onto current main before we run CI.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@notaxeltv a heads-up for the rebase: #8626 has just merged and changed RelinkedModulePath in core/libs/prx/libc/src/General.cpp (exact match, then case-insensitive, then a guarded stem fallback). This branch now conflicts with main in that function, so the nested prx/*.guest.prx preference needs to be re-applied on top of the new lookup order. The earlier asks above still apply.

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

xXPBSXx commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

@notaxeltv: oneandonlydean's review of 17.10. asks for the same directory first and a case-insensitive nested lookup, and #8626 changed RelinkedModulePath since. I prepared a branch with both applied, so you can take it over or drop it: carry/6544-relinked-nested in xXPBSXx/AnyPS5, two commits on main a2d750122b. Your commit keeps your authorship (rebased onto the new lookup of #8626); the second commit is mine.

The order is now: exact name, then the module's own directory ignoring case, then prx/sce_module/sce_modules ignoring case, then the stem fallback. Two tests added next to your nested test: a same-directory module with a differently cased name wins over an exact-case nested file, and a nested module is found when its name differs in case.

Measured with ctest -R '^guest_dynamic_loader$', three runs each:

  • WSL2 Ubuntu 24.04 (virtualised kernel, not bare-metal), GCC 13.3, Release: with the branch passes 3/3; with General.cpp from main a2d750122b fails 3/3 (aborts; that variant also lacks your nested lookup, so it shows the tests need the change, not which test fails first).
  • Windows 10 19045, WinLibs GCC 15.2.0, Release: passes 3/3 with the branch and 3/3 with only your commit's lookup order. The Windows file system ignores case, so the new same-directory test cannot fail there; it only distinguishes the orders on a case-sensitive file system.
  • tools/check_conventions.py: passes.

Not tested: macOS, a title run.
Your PR stays yours: if you push these changes, I will drop mine. Otherwise I can open mine as "based on its head" after 24 h without an answer.
AI-assisted (Claude Code).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@xXPBSXx thanks, I went through carry/6544-relinked-nested (fecd240378). It does what I asked on 17 Oct: the module's own directory is checked first (exact, then ignoring case), then prx, sce_module and sce_modules with the file name matched ignoring case, then the stem fallback from #8626. The first commit is @notaxeltv's change unchanged apart from the new surrounding code. Your two tests each catch one of the asks on Linux. With the first commit's lookup order alone, the same-directory test fails. With the nested lookup made case-sensitive, the case test fails. Both pass with the branch.

@notaxeltv you can take it as is by resetting your branch to it (that needs a force-push, since it's rebased onto current main). If you'd rather not, xXPBSXx's plan works for us too: a new PR from that branch after 24 hours, keeping your commit and authorship, and we'll close this one in its favour. Either way, the next step is a full CI run on that head.

@xXPBSXx

xXPBSXx commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

notaxeltv: as offered above and agreed in the review, with no answer for over 24 hours I opened #10494 from the branch carry/6544-relinked-nested: your commit keeps its authorship, the second commit applies the review asks. If you push your own update here, I will close #10494.

AI-assisted (Claude Code).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@notaxeltv This PR now conflicts with main in core/libs/prx/libc/src/General.cpp, core/libs/tests/GuestDynamicLoader.cpp. Main’s relinked-module name matching and loader tests overlap this PR’s nested-path lookup changes. Could you reconcile the nested-module selection rules with the current lookup behavior?

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@notaxeltv I'm closing this in favour of #10494, which carries your commit unchanged (with your authorship) and adds what I asked for on 17 Oct: own directory first, then prx, sce_module and sce_modules, then the stem fallback. Thanks for the original change. If there's something here #10494 doesn't cover, reopen it or ping me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc needs rebase Conflicts with main, rebase needed unblocks title Names a title ID on the Unblocks line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants