Repository navigation
feat(libkernel): add pthread_rwlockattr_settype_np and scePthreadMutexattrSetkind - #9632
Conversation
The POSIX name moves from a libkernel_sys stub to libkernel and stores the type through its SCE twin scePthreadRwlockattrSettype, converting the result as the other POSIX rwlock calls do. guest_kernel_sys_pthread_np_rwlockattr checks types 0, 1 and 2 through both names, a rwlock built from the attribute, and the throws for a destroyed or null attribute.
The SCE name moves from a libkernel_sys stub to libkernel. Kinds MutexType::ErrorCheck (1) to MutexType::Adaptive (4) are the mutex types FreeBSD 11 libthr's pthread_mutexattr_setkind_np stores, and are set through scePthreadMutexattrSettype, which maps the same values onto those types. Other kinds and a null or uninitialized attribute throw, as their console answer is not known. guest_kernel_sys_pthread_np_kind round-trips kinds 2, 3, 4 and 1 through pthread_mutexattr_gettype, checks that kind 1 makes a second lock return EDEADLK and kind 2 lets it succeed, and checks the throws for kinds 0, 5 and -1 (the type unchanged) and for a destroyed or null attribute.
Progress report (b90d009 → 3a47ea6)📈 System libraries: 50.1% (+0.01%, +2 implemented, +1 declared, -3 removed functions) ✅ 2 implemented
🆕 1 declared as stubs
🗑️ 3 removed
Changed libraries (implemented exports)
|
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 4a17c71, shadPS4 rwlock.cpp@e0a4667 (L258-289), FreeBSD releng/11.0 thr_mutexattr.c/pthread.h, and ps5-payload-dev/sdk sce_stubs:
scePthreadMutexattrSetkindlooks right to me. Kinds 1..4 map to FreeBSD's ERRORCHECK/RECURSIVE/NORMAL/ADAPTIVE_NP, are stored as the type the way_pthread_mutexattr_setkind_npdoes it, and actually change locking. Throwing for other kinds and for a null attribute is the conservative choice.pthread_rwlockattr_settype_np(Posix/Rwlock.cpp:60) returns 0 for any int, for examplesettype_np(&attr, 3)or-1. That is becausescePthreadRwlockattrSettypestores anything andscePthreadRwlockInitignores the attribute. The shadPS4 source you cite treats only 0..2 as valid: its gettype_np and settype_np both returnPOSIX_EINVALwhen the stored type is> 2. Since the console result for other values is unknown, could types outside 0..2 throw instead of reporting success?- The debt entry (libkernel.md:16) says shadPS4 returns EINVAL only for a null or uninitialized attribute, but it also has that
type > 2check. And since 0..2 are stored but never applied, the entry probably belongs under "Silent stubs" rather than "Unknown function info". - Both names are in ps5-payload-dev/sdk
sce_stubs/libkernel.c, so moving them out of libkernel_sys's discovery stubs is supported.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
…emented pthread_rwlockattr_settype_np stored any type and returned 0. The console's answer to a type outside 0 to 2 is unknown, and shadPS4 returns EINVAL once the stored type is above 2, so such a type now goes through NotImplemented_nid_no_patch. Types 0 to 2 are stored but not applied, so the debt entry moves to the silent stubs.
# Conflicts: # core/libs/prx/libkernel_sys/Export.Discovery.cpp
|
Thanks, @harisibrar1, agreed on all three. In fa3cbbe:
On GitHub's runners (run) the extended test fails with the previous head's code and passes 10 runs in a row on Linux and Windows. |
|
Consistency question; otherwise clean.
Verified from the diff and from main: the delegate chain ( |
…implemented A null or uninitialized attribute, and a kind outside 1 to 4, threw std::invalid_argument. They now go through NotImplemented_nid_no_patch, as pthread_rwlockattr_settype_np's unmeasured types do. FreeBSD 11 libthr stores an unknown kind unchecked and answers a null attribute with -1 and errno EINVAL, so scePthreadMutexattrSettype's SCE_KERNEL_ERROR_EINVAL would match neither answer.
|
@fogydes thanks for checking the delegate chain. |
There was a problem hiding this comment.
The current changes address the earlier review findings: rwlock type 3/-1 are no longer silently accepted, the debt entry was corrected and moved under Silent stubs, and mutex attribute invalid inputs now use NotImplemented_nid_no_patch rather than introducing a new std::invalid_argument convention. The body reports focused Windows/Linux failure controls and 10/10 passes on both platforms after the recent changes; the earlier Linux precise_wait failure reproduces on unchanged main. Latest hosted eligibility, conventions and labels checks pass, but product build/test jobs are skipped on this head; the platform run evidence in the body is from the earlier e8172f8078 run and should be refreshed after this push before merge. I did not rerun locally in this environment.
I’ve noticed that holding this PR because of #5239 isn’t necessary. It’s fine for PRs to keep coming in; they’ll be handled in due course. Please disregard my earlier request to hold or defer it.
MarcZX10
left a comment
There was a problem hiding this comment.
I confirmed the author’s clarification: the focused Windows/Linux runs are on current head e8172f8, so my earlier concern that they were stale was mistaken. I independently ran git diff --check, tools/check_conventions.py against current main, and tools/check_test_registration.py --scripts; all pass (171 scripts registered). The reported focused tests exercise both new entry points and compare against failing stubs; the one Linux precise_wait failure reproduces on unchanged main.
I am withdrawing my earlier #5239 intake hold as a reason to defer this PR. One repository requirement remains: the PR targets d6eafa5, while current main is 15 commits ahead at 8b3a6ba; CONTRIBUTING.md requires a current-main base. The changed pthread and debt files themselves have no intervening main changes, so the rebase appears straightforward. Please update the base before merge and refresh the overlap/conventions check. Hosted product jobs are skipped by path filters; I did not run the full project test suite locally.
harisibrar1
left a comment
There was a problem hiding this comment.
Rechecked at e8172f8:
pthread_rwlockattr_settype_np(Posix/Rwlock.cpp:61) now throws throughNotImplemented_nid_no_patchfor types outside 0..2, sosettype_np(&attr, 3)and-1no longer report success. The test covers both.- The debt entry (libkernel.md:8) now records shadPS4's stored-type
> 2EINVAL and sits under Silent stubs. It matches the code: 0..2 are stored butscePthreadRwlockInitignores them, and a null or uninitialized attribute throws, asscePthreadRwlockattrSettypedoes. - Moving
scePthreadMutexattrSetkind's bad inputs toNotImplemented_nid_no_patch(Mutex.cpp:142-144) keeps it consistent with the rest of the PR, and its debt entry says so.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
Thanks, @MarcZX10 and @harisibrar1. The branch now has |
|
I’ve noticed that my earlier request to hold this PR based on #5239 was unnecessary. It’s fine to keep submitting PRs; maintainers will handle them in due course. Please disregard that request. |
# Conflicts: # docs/dev/debt/libkernel.md
# Conflicts: # core/libs/CMakeLists.txt
|
I checked both functions against the PS5 firmware 13.42
AI-assisted: yes (Claude Code). |
BunnyFl measured these functions against firmware 13.42's libkernel and libkernel_sys in the PR thread (behaviour only): - pthread_rwlockattr_settype_np stores only types 1 and 2. Any other type, 0 included, and a null attribute or object return EINVAL and store nothing; type 0 was stored before. - scePthreadMutexattrSetkind returns 0x8001FFFF, sceKernelError(-1), for a null attribute or object instead of throwing. Kinds 1 to 4 are set as before; other kinds still throw, since the firmware stores them unchecked and a mutex's behaviour with them is not measured. - scePthreadMutexattrGetkind takes the attribute itself and returns sceKernelError of its kind, or 0x8001FFFF for a null one.
|
@BunnyFl thank you, this caught a real mistake: the PR stored rwlock type 0, which the firmware refuses. ca39134 takes what you measured:
Kinds outside 1 to 4 still throw in |
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed PR #9632 (head 96c21c0).
Environment: Windows (win32), Python 3.11.9, MinGW64 toolchain.
Checks:
- Baseline, pristine PR, and candidate builds/tests passed (100% for
guest_kernel_sys_pthread_npprofile). - Negative control confirmed old stubs failed on baseline head.
- Whitespace and conventions checks passed.
Verification:
scePthreadMutexattrSetkindandpthread_rwlockattr_settype_npcorrectly replace stubs.- Bounds checks and null/destroyed attribute handling verified in source.
- CTest registration and debt documentation updated correctly.
No actionable findings. Local-only tests; no GPU or game execution performed.
…ettype_np and scePthreadMutexattrSetkind Took the PR's pthread_rwlockattr_settype_np (types 1 and 2 only, as measured) in libkernel over our libkernel_sys/Pthread.cpp one (any type stored) with its debt line, plus scePthreadMutexattrSetkind/Getkind and the PR's test; dropped our duplicate null-attribute settype check from GuestKernelSys and the two libkernel_sys stubs.
scePthreadRwlockattrSettype stored any type and threw on a null attribute. Measured against firmware 13.42's libkernel by BunnyFl in #9632, it is the same function as pthread_rwlockattr_settype_np with the error converted to an SCE code: types 1 and 2 are stored and return 0, and any other type, 0 included, or a null attribute or object returns SCE_KERNEL_ERROR_EINVAL and stores nothing.
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
Main's boykopovar#9573 implements scePthreadResume right after the scePthreadMutexattrSetkind stub this branch removes from libkernel_sys; the merge takes main's scePthreadResume and leaves the stub out. Main's public-header audit wants one header declaration per export, so the merge declares this branch's scePthreadMutexattrSetkind, scePthreadMutexattrGetkind and pthread_rwlockattr_settype_np in libkernel's Api.hpp, drops their stub declarations from libkernel_sys's Api.hpp, has Mutex.cpp take sceKernelError from libkernel's Api.hpp, and has the test include the libkernel headers instead of its own declarations.
|
@EagleBearerYS #11253 edits a debt entry next to one this PR changes (PS5 measurements of module and process queries), so git may report a conflict there. Whichever merges second keeps both; I'll rebase #11253 if this one lands first. |
|
Checked the rwlock type range (1 and 2 stored, everything else and null |
What
This moves three libkernel_sys stubs into libkernel, the way
mainplaces the libkernel_sys functions that libkernel already implements.pthread_rwlockattr_settype_npstores types 1 and 2 through its SCE twinscePthreadRwlockattrSettype, and converts the result as the other POSIX rwlock calls do. Any other type, 0 included, and a null attribute or object returnEINVALand store nothing, as BunnyFl measured on firmware 13.42's libkernel. The stored type isn't applied, sincescePthreadRwlockInitignores its attribute, so the debt entry is under Silent stubs.scePthreadMutexattrSetkindaccepts kindsMutexType::ErrorCheck(1) toMutexType::Adaptive(4) and sets them throughscePthreadMutexattrSettype, as FreeBSD 11 libthr'spthread_mutexattr_setkind_npstores the kind as the type. The bounds are the tree's existingMutexTypeconstants. A null or uninitialized attribute returns 0x8001FFFF,sceKernelError(-1), as BunnyFl measured. Other kinds throw throughNotImplemented_nid_no_patch: the firmware stores any kind as 16 bits of the type field unchecked, but what a mutex does with such a kind isn't measured.scePthreadMutexattrGetkindtakes the attribute itself and returnssceKernelErrorof its kind, 0x80020000 plus the kind, or 0x8001FFFF for a null one, as measured. The firmware returns 0 for kind 0, which no setter here stores, so that case is untested.Still throwing:
pthread_mutex_setname_np,pthread_cond_setname_np,pthread_rwlock_setname_np,pthread_barrier_setname_npandsem_setname: a silent stub needs a title that calls them.scePthreadMutexIsowned: two conventions are on record, FreeBSD's 1/0 boolean and shadPS4's wrapped0x80020001.scePthreadMain: waits on feat(libkernel_sys): identify the initial pthread #9160's main-thread helper.pthread_timedjoin_npandscePthreadTimedjoin: wait on feat(libkernel): implement pthread_timedjoin_np #8890.Related:
pthread_mutexattr_setkind_np/getkind_npand merges cleanly with this PR.MutexTypevalues, and both throw for other kinds.errnoEINVAL. The SCE form here returns 0x8001FFFF, as measured.scePthreadMutexattrSetkindcan keep its attribute check and forward topthread_mutexattr_setkind_npthroughPosixThread::ToSce.Export.Discovery.cpp.GuestKernelSysPthread.cpp, which is why this PR's test file has a separate name.Tested
guest_kernel_sys_pthread_np_rwlockattrandguest_kernel_sys_pthread_np_kindfail ×2 on both runners withmain's stubs and pass ×5 on both.precise_wait, a timing test that fails in that run withmain's code too.check_test_registration.pyand MinGW-fsyntax-onlyof the changed files are clean.guest_kernel_sys_pthread_np_rwlockattrgains types 3 and -1, which must throw. With the previous head'sRwlock.cppit fails on both runners, since both types returned 0; with the change both tests pass 10 runs in a row on both. The libc and libkernel suites pass on Windows (142, 207) and libkernel on Linux (197); libc's timing testprecise_waitfailed on Linux with and without the change.mainat d6eafa5 (b23cbb5, run): one conflict, inlibkernel_sys/Export.Discovery.cpp, wheremainhad removed thepthread_set_defaultstacksize_npdiscovery stub (now inlibkernel_sys/Export.cpp) next to the two stubs this PR removes. The merge keeps all three removals, and this PR's diff againstmainis unchanged.guest_kernel_sys_pthread_np_rwlockattr,guest_kernel_sys_pthread_np_kindandmain'sguest_kernel_sys_thread_defaultspass 10 runs in a row on both runners. The libc and libkernel suites pass on Windows (143, 207) and libkernel on Linux (197); libc'sprecise_waitfailed once on Linux.scePthreadMutexattrSetkind's null or uninitialized attribute and its kinds outside 1 to 4 go throughNotImplemented_nid_no_patchinstead ofstd::invalid_argument. With the previous head'sMutex.cpp,guest_kernel_sys_pthread_np_kindfails on both runners with the old throw ("Mutex kind 0 is not a mutex type"). With the change both tests pass 10 runs in a row on both. The libc and libkernel suites pass on both (116 and 197 on Linux, 143 and 207 on Windows).main3ca07d0 merged in (clean; the PR's own diff is unchanged), on GitHub's runners (run):guest_kernel_sys_pthread_np_kind,guest_kernel_sys_pthread_np_rwlockattrandguest_kernel_sys_thread_defaultspass 10 of 10 on Linux and Windows, none skipped. On its one run withmain's implementation,guest_kernel_sys_pthread_np_kindandguest_kernel_sys_pthread_np_rwlockattrfail on both runners andguest_kernel_sys_thread_defaultspasses. The libc and libkernel suites pass on both.scePthreadMutexattrSetkindand the newscePthreadMutexattrGetkind. Red, the commit's tests on 96c21c0's code (run): on both runners_rwlockattrand_kindfail on exactly the three changed cases (type 0 is stored; the null attribute throws;Getkindis a stub), andguest_kernel_sys_thread_defaultspasses. Green (run): the three tests pass 10 runs in a row on Linux and on Windows, none skipped, and the libc and libkernel suites pass on both, run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.maina145c6a merged in, on GitHub's runners (run). One conflict, indocs/dev/debt/libkernel.md, wheremain's newsceCoredumpSetUserDataTypeentry (fix(libkernel): give sceCoredumpSetUserDataType its signature and result #8906) and this PR'spthread_rwlockattr_settype_npentry are added at the same place. The merge keeps both,main's first. The PR's changed lines are the same as before the merge (git diff -U0); only their context in the debt file moved.guest_kernel_sys_pthread_np_kind,guest_kernel_sys_pthread_np_rwlockattrandguest_kernel_sys_thread_defaultspass 10 of 10 on Linux and Windows, none skipped, and the two new tests fail withmain's implementation on both. The libc and libkernel suites pass on both. The libc and libkernel suites ran withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.main9a030c2 merged in, on GitHub's runners (run). One conflict, incore/libs/CMakeLists.txt, wheremain's newguest_kernel_sys_module_internalregistration and this PR'sguest_kernel_sys_pthread_npregistrations are added at the same place. The merge keeps both, this PR's first. The PR's other files are unchanged by patch-id, and its changed lines are the same as before the merge; only their context inCMakeLists.txtmoved.guest_kernel_sys_pthread_np_kind,guest_kernel_sys_pthread_np_rwlockattr,guest_kernel_sys_thread_defaultsandmain'sguest_kernel_sys_module_internalpass 10 of 10 on Linux and Windows, none skipped, and the two new tests fail withmain's implementation on both. The libc and libkernel suites pass on both (117 and 198 on Linux, 144 and 208 on Windows). They ran withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.main50bc72b merged in, on GitHub's runners (run). One conflict, inlibkernel_sys/Export.Discovery.cpp, wheremainhad removed thescePthreadResumediscovery stub, which it now implements, next to the stubs this PR removes. The merge keeps both removals.main's rule that each export is declared once, in its library'sinclude/Api.hpp. libkernel'sApi.hppdeclares the three exports with their real signatures, and libkernel_sys's drops its(void)declarations of them.Mutex.cpptakessceKernelErrorfrom libkernel'sApi.hppinstead of declaring it, and the test includes the headers instead of its ownextern "C"block. The code's behaviour is unchanged.public_headersand the^guest_kernel_systests, 10 runs in a row. Linux: 19 run, 19 pass, 0 skip, 0 fail. Windows: 17 run, 15 pass, 0 skip, 2 fail. This PR's two tests pass on both.main'sguest_kernel_sys_threads_mainandguest_kernel_sys_threads_main_patched, not this PR's tests. In this verify's Windows job they fail withLoadLibraryExW D:/a/AnyPS5/AnyPS5/build/core/libs/libs/unpatched\libkernel_sys.prx failed: 127.main88f9967, with only a line added that prints that error, fails them the same way there (run). A local Windows build of this merge passes both, built clean and incrementally.APS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.check_conventions.pypasses, with no comment lines added.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)docs/dev/debt/libkernel.md): whichever merges second keeps both