Repository navigation
fix(libc): look in prx, sce_module and sce_modules for a relinked module after its own directory - #10494
Conversation
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>
…lders RelinkedModulePath looked in prx, sce_module and sce_modules before the case-insensitive match in the module's own directory, so a stray nested file could shadow a module next to the title. Check the directory of the module first, then the nested folders, and match the nested names case-insensitively. Tests: a same-directory module with a differently cased name wins over an exact-case nested file, and a nested module is found case-insensitively. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
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.
|
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 6c52b64 and the relinker sources on main (NativeModules.cpp, GuestModuleBuilder.cpp):
- The new lookup matches the order agreed in #6544 (same directory, then the nested case-insensitive lookup, then the stem), and only covers directories the relinker itself writes modules to (
sce_module/sce_modules,prx). It merges cleanly with main. - I checked whether
prxandsce_modulecould both hold a different module of the same name. GuestModuleBuilder.cpp:161-166 already refuses that ("Ambiguous guest dependency name"), so the fixedprx→sce_module→sce_modulesorder can't choose between two relinked copies. The exception is names that differ only in case on a case-sensitive host, where the relinker's map is case-sensitive but this lookup ignores case and takesprxfirst. Probably not worth handling. - No overlap with #9415: that PR changes only DynamicLoader.cpp and matches by basename against any loaded module, and none of the review findings there apply to this lookup.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head e2dda5eba0037be211cb7b9489e461e1c12c4dbd. Lookup now checks the exact relinked path, then case-insensitive same-directory matches, then nested prx/sce_module/sce_modules, preserving the #8626 stem fallback. The tests cover nested resolution, case-insensitive nested lookup, and same-directory precedence over a conflicting nested file. The PR reports five-run focused red/green checks on Linux and Windows; hosted conventions, intake, eligibility, labels, relinker matrix, register-context, write-watch, and ASan checks pass. Product Linux/Windows builds were skipped, and I did not reproduce the local tests.
What
Based on #6544 (notaxeltv, head
83e675746e; the first commit keeps its authorship). The change answers the asks of the review of 17 October one by one, on top of the lookup order that #8626 introduced:prx,sce_moduleorsce_modules; the nested folders are tried only when the own directory has no match.The order of
RelinkedModulePathis now: exact name, own directory ignoring case,prx/sce_module/sce_modulesignoring case, then the stem fallback from fix(libc): find a relinked module by stem when the exact name misses (carries #6226) #8626. The first commit's nested lookup is unchanged apart from the new surrounding code. Two tests next to the original 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.Fixes: none
Tested
One base for all arms:
maine59364ea98, heade2dda5eba0(these two commits;git merge-treeagainstmainis clean). Same checkout for every arm, onlyGeneral.cppswapped and rebuilt, the tests of the head kept.ctest -R '^guest_dynamic_loader$', 5 runs per arm:General.cppof the first commit alone (nested folders first) 0 of 5 pass, the same-directory test fails (GuestDynamicLoader.cpp:248); withGeneral.cppofmain0 of 5 pass, the nested test fails (GuestDynamicLoader.cpp:231).General.cppof the first commit alone 5 of 5 pass, because the file system ignores case, so a Windows run cannot tell the two orders apart; withGeneral.cppofmain0 of 5 pass.tools/check_conventions.py --base upstream/mainpasses.Not tested: macOS, a title run, the variant with a case-sensitive nested lookup (the review of 23 October reports the case test fails there).
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)Co-authored by the original author of #6544.
AI-assisted (Claude Code).
🤖 Generated with Claude Code