Repository navigation
Conversation
|
Independent Linux check at exact head The relinker-only suite passes 165/165 (28.88 s), and the real production getter returns the proposed default block in a separate native probe. There is a gap in the new regression, though: it remains green with the runtime fix removed, and also with a null default pointer in its own provider. Controls, rebuilt and run with
The size mutation confirms the fixture ran and rebuilt. The two false greens have separate causes: This small test-only change catches the null pointer and also checks --- a/core/relinker/elfpatcher/tests/GuestModuleArgsHost.cpp
+++ b/core/relinker/elfpatcher/tests/GuestModuleArgsHost.cpp
@@ -48,9 +48,10 @@
std::snprintf(line, sizeof(line), "args=%016zx argp=%016zx\n", args,
static_cast<std::size_t>(reinterpret_cast<std::uintptr_t>(argp)));
Write(line);
- if (args == sizeof(DefaultModuleArgs) && argp == &kDefaultArgs) {
+ if (args == sizeof(DefaultModuleArgs)) {
+ if (argp == nullptr) Fail("Missing default argument block");
const auto* defaultArgs = static_cast<const DefaultModuleArgs*>(argp);
- if (defaultArgs->size != 0x10 || defaultArgs->version != 0x200) Fail("Default arguments corruption");
+ if (defaultArgs->size != 0x10 || defaultArgs->version != 0x200 || defaultArgs->context != nullptr) Fail("Default arguments corruption");
}
}
It still needs a regression that calls the real runtime getter. I checked that independently without the full SDL build: compile the actual All temporary source edits were restored, the stock provider rebuilt, and |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
macOS run of the new --load-default path, which has its own darwin branch (RTLD_GLOBAL) that the Linux runs don't cover.
Machine: Apple M2 Max, macOS 27.2, Xcode 27 toolchain, relinker-only Release.
Head: 950bc7c32, merged onto main @ ab9a47684 (clean).
The test only executes natively on macOS when the interpreter is x86_64 (platform.machine() == 'x86_64'). Under ctest on Apple silicon, both the arm64 build and the x86_64 build print native execution skipped, because /usr/bin/python3 starts as arm64. So I ran it by hand from the x86_64 build:
arch -x86_64 /usr/bin/python3 core/relinker/relinker/tests/test_guest_module_args.py <build>/core/relinker/relinker <build>/core/relinker/guest_module_args_host.prx
| Change | Result (x86_64 under Rosetta, Python 3.9.6) |
|---|---|
| None | pass |
Fixture provider GuestModuleArgsHost.cpp restored from main |
fail: args=0000000000000000 argp=0000000000000000 |
--load-default with DEFAULT_MODE instead of RTLD_GLOBAL |
fail: dlopen(...): symbol not found in flat namespace '_RecordArgs' |
So the darwin branch works and the RTLD_GLOBAL is required there. As @simonbbby showed on Linux, what the test checks is the fixture's own provider, not the production __aps5_get_pending_module_args_nid_no_patch in Module.cpp. That is the same on macOS: Module.cpp is not part of the relinker-only build.
Full relinker-only suite with the PR: arm64 163/163; x86_64 + Python 3.9.6 159/163, with the same 4 failures as main.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Thanks @jotitadev and @simonbbby for the thorough independent validation on macOS and Linux! Carried the suggested mutation control in commit
Verified locally with a negative mutation control (setting Written with AI assistance (Gemini). |
|
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.
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.
|
|
@Nubi-Prey This PR now conflicts with main in |
XairRrR
left a comment
XairRrR
left a comment
There was a problem hiding this comment.
Reviewed — changes requested
The head conflicts with current main. The fallback also replaces the arguments of explicit sceKernelLoadStartModule calls, not only DT_NEEDED initializers. The new regression still passes when the production change is reverted.
What I ran
- PR head:
0cd9877c6d3224c6207ed58948bc102330350648 main:2960e9a6e7e1fc99c4ed180c71495b5eb83464c7- Machine: Arch Linux 7.2.11-zen, GCC 16.2.1, Intel Core i9-14900KF, NVIDIA GeForce RTX 4070 Ti SUPER (driver 615.78.08), in a clean worktree with a Release/Ninja build.
git fetch -q origin main pull/6149/head:refs/review/6149
git checkout -q --detach origin/main
git merge -q --no-ff --no-edit refs/review/6149 # conflicts, aborted
git checkout -q --detach refs/review/6149
git diff --check origin/main...refs/review/6149
python3 tools/check_conventions.py --base origin/main
cmake --build build -j10 --target relinker guest_module_args_host
ctest --test-dir build -R '^guest_module_args$' --output-on-failure
cmake --build build -j10 --target libs
# negative control
git checkout 92e50418f33b8950ceea950cb37331a941f5be0e -- core/libs/prx/libkernel/Module/src/Module.cpp
cmake --build build -j10 --target libs relinker guest_module_args_host
ctest --test-dir build -R '^guest_module_args$' --output-on-failure
git checkout refs/review/6149 -- core/libs/prx/libkernel/Module/src/Module.cppResults
| Check | Result |
|---|---|
Merge onto main |
Conflict in core/relinker/relinker/tests/test_guest_module_args.py and docs/dev/debt/libkernel.md |
git diff --check |
clean |
tools/check_conventions.py --base origin/main |
passed |
Build relinker, guest_module_args_host (PR head) |
passed |
Build libs (PR head) |
passed |
guest_module_args (PR head) |
passed, 0.11 s |
guest_module_args with production Module.cpp reverted |
passed, 0.11 s |
Merge conflict
CONFLIT (contenu) : Conflit de fusion dans core/relinker/relinker/tests/test_guest_module_args.py
CONFLIT (contenu) : Conflit de fusion dans docs/dev/debt/libkernel.md
On main, the --load branch of the test now drives __aps5_guest_lifecycle_nid_no_patch itself, and the Linux setup copies the provider into libs/libkernel.prx. The PR's --load-default branch and its load_and_record_default calls were written against the earlier version of that code.
Negative control
1/1 Test #1224: guest_module_args ................ Passed 0.11 sec
100% tests passed out of 1
Negative control
I restored only core/libs/prx/libkernel/Module/src/Module.cpp from the PR's merge base 92e50418f3, then rebuilt libs, relinker and guest_module_args_host. guest_module_args still passes. I used the merge base rather than main because the PR head does not merge with main. This confirms, on the current head, what @simonbbby reported for 950bc7c32. The later null-pointer check in the fixture catches a null pointer in the fixture's own provider. It does not make the test exercise the production getter.
Findings
Blocking
-
Conflicts with
main. The PR conflicts incore/relinker/relinker/tests/test_guest_module_args.pyanddocs/dev/debt/libkernel.md. @oneandonlydean flagged this on 2026-10-25, and the head has not changed since.mainhas also changed the code this PR depends on. Linux explicit runtime directories now use deferred initialization, which reads the getter inInitializeLinuxGuestModule, and the debt doc has a Linux runtime module initialization entry. After the rebase, the claim that aDT_NEEDEDmodule runs its entry at process start before anysceKernelLoadStartModuleshould be checked again against the deferred path. -
The fallback rewrites explicit load arguments, not only
DT_NEEDEDinitializers.Module.cpp:276-281substitutes{0x10, &kDefaultModuleArgs}wheneverargp == nullptr. However,sceKernelLoadStartModulestores the caller's{args, argp}verbatim at line 294. @oneandonlydean raised theargs == 0case. One more input changes as well: an explicitsceKernelLoadStartModule(path, 8, nullptr, ...)reaches the module entry as(0x10, &kDefaultModuleArgs), so the caller'sargsvalue is replaced as well asargp. This is a value the title did not pass, and its console behaviour is unverified. The same happens for a guestdlopenon a thread without pending arguments, becausependingModuleArgsisthread_local.
Suggested fix: record whether an explicit start is in flight, for example a separate thread-local flag set and cleared arounddlopen_nid_postfixinsceKernelLoadStartModule. The default block should apply only when no explicit start is in progress. Explicit loads would then always pass the caller's arguments unchanged. Update the debt entry to say exactly which loads receive the block. -
The regression does not cover the production change. The fixture provider
GuestModuleArgsHost.cpp:65-70contains its own copy of the fallback, so--load-defaultchecks the test's copy. The negative control above passes with the production fix reverted. Two options would fix this. A test could call the real__aps5_get_pending_module_args_nid_no_patchfromModule.cpp: @simonbbby and @JJenkx both did this outside CTest, with a probe and with ctypes. Or the fixture could forward to the production object instead of reimplementing it. Either test should also cover the explicit-load case from finding 2, checking that(8, nullptr)reaches the entry unchanged.
Non-blocking
- Scope of the description. "Closes #5621" and "Unblocks: PPSA06464 #5621" are claimed, but the description does not say whether Cult of the Lamb ran past
UnifyPS5Plugin.prxinitialization on this head. @MarcZX10 and @oneandonlydean asked this, and it is still unanswered. @JJenkx's PPSA16018 run and @socket1001's analysis show a plugin that callscontextas a function and crashes on the null value. I did not run a title, so I cannot add evidence either way. The description should say what was reproduced. The debt entry could also note that some plugins dereferencecontext, because a later reader would otherwise assume that a null context is safe.
Not verified
- No title was run. I have no evidence on whether PPSA06464 gets past the initializer.
- I did not probe the production getter at runtime myself. I relied on code reading, and on the probes @simonbbby and @JJenkx describe, which show it returns the 16-byte block.
- The merged tree was not built or tested because of the conflict. Everything above ran on the PR head, which is based on
92e50418f3. - I did not run macOS or Windows.
To get this to a good state
- Rebase onto current
mainand resolve the test and debt-doc conflicts against the deferred lifecycle path. - Limit the default block to initializers that run without an explicit
sceKernelLoadStartModule. Explicit loads should pass the caller's{args, argp}unchanged, including(0, nullptr)and(N, nullptr). - Make the regression exercise the production getter, and show that it fails with the
Module.cppchange reverted. - Scope
Unblocksand the debt entry to what was reproduced on the current head.
0cd9877 to
d8be19d
Compare
|
Thank you @XairRrR, @simonbbby, and @oneandonlydean for the detailed review and guidance. All points have been addressed in the rebased commit:
|
|
should-fix Findings
Freshness Envelope
Cross-PR Ledger
|
Re-checked — checklist entry still missingAt head |
|
@Nubi-Prey thanks, this resolves the main point of the earlier review: the default block is now only used when no explicit
Reviewed at d8be19d. |
Provide a fallback 16-byte default arguments block {0x10, 0x200, nullptr} in
__aps5_get_pending_module_args_nid_no_patch() when no explicit module load
is in progress.
On Linux, libraries listed under DT_NEEDED execute their DT_INIT at process start
before sceKernelLoadStartModule runs. Unity's UnifyPS5Plugin.prx (Cult of the Lamb,
PPSA06464, boykopovar#5621) reads argp[0] unconditionally expecting {size: 0x10, version: 0x200, context}
or {0x58, 0x103}, which faulted when argp was null.
Limit the default block strictly to initializers running without an in-flight
sceKernelLoadStartModule call, preserving explicit caller arguments including
(0, nullptr) and (N, nullptr).
Move the pending module args state and getter/setter into a dedicated translation
unit (ModuleArgs.cpp) compiled by both libkernel and guest_module_args_host,
ensuring regression tests directly exercise production logic. Extend the test
fixture to verify default argument delivery and explicit null argument preservation.
Document technical debt and known console gaps in docs/dev/debt/libkernel.md.
Closes boykopovar#5621.
d8be19d to
a407a5e
Compare
github-actions
Bot
removed
the
unblocks title
|
Rebased onto current
Architectural note on #5621 / #6586 / #12621: Following the later disassembly and string dumps in #5621 ( AI-assisted: yes (Gemini 4 Argon) |
github-actions
Bot
removed
needs rebase
|
Correction: In the previous review (at head OK Findings
Freshness Envelope
Cross-PR Ledger
|
|
We hit the same problem while running Sonic Superstars (PPSA06888) on Linux, and this PR's default argument block fixes it. What we saw without it: a guest module started through The code we tested was a smaller version of the same idea (a zeroed block returned from Two things we observed that may matter for this PR:
If it helps, we can test a rebased version of this PR against the title on our side. |
What
core/libs/prx/libkernel/Module/src/ModuleArgs.cppandModuleArgs.hpp(SetPendingModuleArgs_nid_no_patch,GetPendingModuleInitResult_nid_no_patch,SetModuleInitResultObserverForTest_nid_no_patch) and link it into bothlibkernelandguest_module_args_hostso the relinker test exercises the production implementation directly.sceKernelLoadStartModulecall is active viathread_local bool explicitLoadInProgress(with exception-safe reset arounddlopen_nid_postfixinModule.cpp), preserving explicit(args, nullptr)pairs whenexplicitLoadInProgressistrueand returning a 16-byte default block{size: 0x10, version: 0x200, context: nullptr}(args = 16) only whenexplicitLoadInProgressisfalse.docs/dev/debt/libkernel.md: what the console passes outsidesceKernelLoadStartModuleor toDT_NEEDEDdependencies pulled in duringsceKernelLoadStartModule, that{0x10, 0x200}is a guess from Linux: Cult of the Lamb (PPSA06464) faults at process start in UnifyPS5Plugin.prx, a needed library started with a null arguments block #5621 whilecontextis unknown (nullptrfaults if called during init), and thatexplicitLoadInProgressremainstrueacrossdlopen_nid_postfix.Tested
-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON), rebased onmain(474030e584): builtrelinkerandguest_module_args_hostand ranctest --test-dir build-relinker -R "guest_module_args|guest_deferred_dependencies" --output-on-failure -V(2/2 passed:load_and_record,load_and_record_default,load_and_record_explicit_null, andguest_deferred_dependencies).SceKernelDefaultModuleArgs::versionto0x201inModuleArgs.hppfailsguest_module_argswithGuest module argument fixture: Default arguments corruption.kEmptyPendingArgswhenpendingModuleArgs.argp == nullptrinModuleArgs.cppfailsload_and_record_explicit_nullinguest_module_args.python3 tools/check_conventions.py --base upstream/mainpasses with 0 warnings and 0 errors.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)