Repository navigation
feat(libkernel): implement four pthread_attr forms - #9156
DavidSamir wants to merge 7 commits into
Conversation
…addr and affinity forms
… and affinity forms
Progress report (6cd36e6 → 550810d)📈 System libraries: 48.34% (+0.01%, +2 implemented, +2 declared, -4 removed functions) ✅ 2 implemented
🆕 2 declared as stubs
🗑️ 4 removed
Changed libraries (implemented exports)
|
|
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.
|
|
Overlap note: #9160 (opened 4 minutes after this PR) also implements |
The PS5 3.20 export lists have these pthread_attr forms in libkernel as well as libkernel_sys, and on Windows a libkernel import never reaches libkernel_sys. They move unchanged to libkernel/Pthread/Posix/AttrNp.cpp.
DavidSamir
changed the title
|
Linux: Ubuntu 24.04.1 LTS (WSL2), GCC 13.3.0, Release; head
Run from the PC (Windows host, Linux in WSL2), not from the PR author's machine. |
|
Hi @DavidSamir, this PR and #9573 (threads created suspended) meet in I'd suggest this PR goes first, which needs nothing from you: #9573 now says |
|
Thanks @zaryabzip, that order works for me: #9156 first, and #9573 rebases onto AI-assisted: yes |
…ttr forms Moved the four exports into the PR's libkernel/Pthread/Posix/AttrNp.cpp so libkernel exports them; kept our getaffinity/setaffinity (from boykopovar#9160: 32-byte set, tail bytes, clearing on a zero size as FreeBSD does) and getsolosched, took the PR's setstackaddr null checks (null attr EINVAL, null address throws). Dropped the PR's test (its 8-byte and byte-9 affinity cases contradict FreeBSD's 32-byte set) and its setaffinity debt line; guest_kernel_sys_threads now loads the four from libkernel.
harisibrar1
left a comment
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 99feaf0 and FreeBSD 11.4 lib/libthr/thread/thr_attr.c:
pthread_attr_setstackaddrwith a NULL address: FreeBSD 11.4 returnsEINVAL(_pthread_attr_setstackaddrchecksstackaddr == NULLtogether withattr/*attr, L545); it doesn't store it. So the throw inAttrNp.cpp:52and its debt bullet aren't needed.scePthreadAttrSetstackaddron main already returnsSCE_KERNEL_ERROR_EINVALfor a NULL address, whichPthreadAttrErrnomaps to 22.pthread_attr_setaffinity_nphas no lower bound oncpusetsize, so(attr, 1, "\x03")stores mask 0x3 and returns 0. FreeBSD returnsERANGEbelow the kernel set size (L609-611), and the getter here already uses 32 forERANGE. Could the setter use the same bound? The test atGuestKernelSysPthreadAttr.cpp:85(size 8 → 0) would change with it. Main'scpuset_getaffinity/cpuset_setaffinityuse 8 as the minimum, so it's worth deciding which bound both should use.- Small note: on an attribute that never had a set, FreeBSD's getter returns an all-ones set, while this returns main's 0x1FFF default. That seems reasonable for the sce layer; only mentioning it in case it matters.
docs/dev/debt/libkernel.mdnow conflicts with main, because the neighbouringsceKernelAioSetParambullet was rewritten, so it needs a rebase.- Optional:
Posix/Attr.cppusesPthreadAttr*andPosixThread::ToErrno/GUEST_EINVALfromCommon.hpp. Using those here instead of redeclaring them withvoid**and a localPthreadAttrErrnowould keep the two files consistent.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
@DavidSamir I agree the null-stack-address refusal is unnecessary: please delegate to the existing setter and return mapped EINVAL (22), correct the debt claim, and add a null-argument return-code case. Please also make the non-clearing setaffinity cpuset minimum coherent with the paired getter: its current minimum is 32, while setter accepts 1 and tests 8. Use that guest minimum with ERANGE/boundary/unchanged-mask checks unless measured guest evidence supports another contract, and explain any deliberate difference from the 8-byte cpuset syscalls. Keep unsupported clearing behavior separate; do not add throw-pinning tests. |
Reuse the existing SCE getter and preserve POSIX errors and caller errno. Adapter contract follows DavidSamir PR boykopovar#9156; alias ABI remains inferred. Tested with real Linux libraries, raw and NID-patched loading, pthread regressions and rejected mutations.
|
Measured on a PS5 (a payload calling the console's libthr; return values):
A fresh attribute's mask reads back as eight |
|
@DavidSamir The local declarations differ from main’s public headers: |
|
Correction to my measurement above: I hadn't tried sizes between 8 and 16. A fuller sweep (sizes 0, 1, 4, 7, 8, 9, 12, 16, 24, 32, 64, 128) shows |
|
@DavidSamir rooklz's original and corrected reports distinguish attribute APIs from thread/kernel cpuset APIs. For this PR's pthread_attr_get/setaffinity_np, the first report supports an 8-byte floor (8, 16, 32, 64 and 128 bytes succeed); please replace the getter 16-byte ERANGE assertion with success and add checks at 8 bytes and below 8 bytes, with unchanged-mask checks. Our earlier provisional 32-byte setter minimum is withdrawn; keep 8-byte setter success. The correction's 8-16 byte range is for cpuset_getaffinity/cpuset_setaffinity/pthread_getaffinity_np, without attr, so please don't apply its 16-byte ceiling to attribute calls or describe the syscalls as exactly 8. The reported size 0 attribute setter returns 0; the report doesn't state resulting mask/reset behavior, so that still needs clarification before defining clearing semantics. The null-stack mapped EINVAL/debt/test ask and public-header/signature reconciliation remain. No test should require an unsupported clearing path to throw. |
…tribute and initial-thread queries Kept the four attribute forms in libkernel (AttrNp.cpp, from the boykopovar#9156 port) instead of the PR's libkernel_sys copies, since libkernel_sys imports fall back to libkernel; took the PR's null-address EINVAL for setstackaddr (dropped our throw and its debt line), its Linux TID/PID pthread_main_np, the main_late_load test and the module path check; tests keep loading the four from libkernel. (cherry picked from commit 4f0c7d0780aa229981994ed8f8368a0e1b4658b6)
|
Hi @DavidSamir, the overlap check shows my open PR next to this one:
The changes are separate. Only the neighbouring lines overlap. Proposed merge order: this PR first, then #11026, which I'll rebase onto it after it merges. I've recorded that under |
What
Four pthread attribute exports in
libkernel_syswere discovery stubs that threwNotImplemented. They are now implemented in libkernel, inlibkernel/Pthread/Posix/AttrNp.cpp, and the libkernel_sys stubs are removed. They wrap libkernel's existingscePthreadAttr*calls, map the result to anerrnothe wayPosixThread::ToErrnodoes (& 0xFFFF), and follow FreeBSD 11lib/libthr/thread/thr_attr.c:pthread_attr_getsolosched_np(attr, solosched)scePthreadAttrGetsoloschedEINVALfor a NULLattr,*attror out pointer, as the existingpthread_attr_getstackaddrdoespthread_attr_setstackaddr(attr, addr)scePthreadAttrSetstackaddrEINVALfor a NULLattror*attr(_pthread_attr_setstackaddr)pthread_attr_getaffinity_np(attr, size, set)scePthreadAttrGetaffinityEINVALfor a NULLattror*attr;ERANGEforsizebelow the 32-byte kernel set (_pthread_attr_getaffinity_np); the bytes past the 64-bit mask are zeroed;EFAULTfor a NULL set, as the existingpthread_getaffinity_npdoespthread_attr_setaffinity_np(attr, size, set)scePthreadAttrSetaffinityEINVALfor a NULLattror*attr, and for nonzero bytes past the 64-bit mask, whichscePthreadAttrSetaffinitycan't holdTwo inputs throw and are recorded in
docs/dev/debt/libkernel.md:pthread_attr_setstackaddrwith a NULL address. FreeBSD stores it, butscePthreadAttrSetstackaddrrefuses it.pthread_attr_setaffinity_npwith a zerosizeor a NULL set. FreeBSD takes that as clearing the attribute's set, and noscePthreadAttr*call clears it.Moved to libkernel so libkernel imports reach it on Windows. The PS5 3.20 export lists have these four in libkernel as well as libkernel_sys, and on Windows a libkernel import never searches libkernel_sys (
WindowsEntryStubBuilder.cpp:175). With the libkernel exports and no libkernel_sys stubs, both imports reach them, and the duplicate-export rule holds. The code is unchanged by the move.Tested
Windows 11 Pro 26200, WinLibs GCC 15.2.0 r7, Release. These are CPU-side exports, so no GPU is involved.
guest_kernel_sys_pthread_attr(core/libs/tests/GuestKernelSysPthreadAttr.cpp):pthread_attr_getstackaddr;0x5, a 40-byte set zero-filled past the mask,ERANGEat 16 bytes,EFAULTfor a NULL set) and set (32- and 8-byte sets,EINVALfor a nonzero byte 9, the stored mask unchanged after it, read back throughscePthreadAttrGetaffinity), each with a NULL attribute as well.mainthe test aborts withpthread_attr_getsolosched_np_nid_postfix not implemented. On this branch it passes.guest_kernel_sysstill passes.tools/check_conventions.pypasses, including the duplicate-export rule.AttrNp.cppremoved from libkernel's sources, the test fails to link (undefined reference to pthread_attr_getsolosched_np_nid_postfix), so it reaches libkernel's exports, the only ones.tools/pr_overlap.pycrashes on this machine's partial clone, so it was replaced by a check against an index of every open PR's diff, plusgit merge-treeagainst the 8 open PRs that touch these files (feat(libkernel_sys): implement the scePthread scope, atfork and setconcurrency forms #9141, feat(libkernel): add missing pthread attribute accessors #8373, feat(libkernel_sys): implement the scePthreadBarrier, cpuclockid, concurrency and msync forms #8987, feat(libkernel_sys): implement chown, lchown, fchownat and linkat #9103, feat(libkernel_sys): implement pwrite through the shared POSIX provider #9019, feat(libkernel_sys): implement fstat #9008, feat(libkernel_sys): implement sceKernelTitleWorkaroundIsEnabled and sceKernelGetProcessType #9040, feat(libkernel_sys): implement sceKernelFlock over flock #9097). All 8 merge cleanly.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)