Repository navigation
fix(libkernel): report thread attribute inheritance and null-output errors as measured on a PS5 - #10130
Conversation
…ibute queries as measured on a PS5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-attr-measured-ps5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-attr-measured-ps5
MarcZX10
left a comment
There was a problem hiding this comment.
The PS5 probe evidence supports the destroyed condition marker, null-destroy behavior, inherited scheduling query, and null-output error changes; GCC 12.2 syntax checks passed for the changed implementation and tests, as did conventions, diff-check, and main-sync (5 ahead/29 behind; merge clean). The author reports the three directly affected tests and 14 related Windows thread/condition tests passing three times. Please split the condition-variable changes (Cond.cpp and destroyed-object tests) from the independent thread-attribute changes (Attr.cpp, inheritance state/query tests) into separate topic branches, as required by the repository’s one-branch-per-topic rule. The attribute portion also conflicts with open PR #10133 in docs/dev/debt/libkernel.md; coordinate a single combined entry and landing order with that author, preserving both sets of measured attribute behavior. Add Depends on if either branch needs the other first, then refresh and rerun focused tests and checks. Hosted policy checks pass; product jobs are skipped by the workflow.
|
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.
|
|
I checked the condition-variable and
I didn't run AI-assisted: yes (Claude Code). |
…l request Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
Re-review of current head 886a2b249cf0dcbe75f954e89dbdaadea960e1ef. The condition-variable implementation and its test were successfully moved out to #10177, leaving the thread-attribute behavior as this PR's topic. #10133 now declares Depends on: #10130, so the debt-file landing order is recorded. The remaining review issue is that this PR's Tested section still attributes guest_pthread_destroyed_objects and the condition-variable cases to this change, and says all three affected tests fail on main and pass with it. Those condition changes are no longer in this PR's file diff; please update the test report to identify the attribute tests actually changed here and distinguish any checks that now belong to #10177. The focused attribute evidence can remain; the report should match the exact current diff. Hosted product jobs are path-skipped.
|
@MarcZX10 Updated the Tested section to the current head |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@zaryabzip: #9573 changes the same lines as this PR in |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
|
Checked the inheritance reported by |
What
Thread-attribute edge cases measured on a retail PS5 (firmware 13.60) with three homebrew probes calling
libkernel_sys.sprxfrom a payload process. The sources and console logs are in this gist (pcond,pcond2,pattr).The condition-variable change moved to #10177, as requested in review.
scePthreadAttrGetreports the inheritance a thread was created with: 0 for an explicit-scheduling thread and 4 for an inheriting one or one created from a freshly initialised attribute. Before, it always reported 4.pthread_attr_get_npnow copies the same inheritance, as FreeBSD's copies the whole attribute; before, it left the caller's value. A null attribute at creation was not measured. The policy and priority it reports already matched:scePthreadAttrGetinheritschedwith a null output returnsEINVAL, asscePthreadAttrGetschedpolicydoes. Before, it threw. Both also returnEINVALfor a null or uninitialized attribute, which AnyPS5 already did. A new attribute holds policy 1, inheritance 4 and priority 700, also as before.Already matching, so nothing in this diff changes them:
scePthreadCondTimedwaitreturnsEPERM(0x80020001) for an unowned mutex of each type (default, error-checking, recursive, normal, adaptive), the adaptive static initializer (slot 1), a null slot and a mutex held by another thread.EINVALfor the destroyed mutex (slot 2).0x8002003c, also for 0 µs.The debt entries are updated.
Not modelled: a slot of 2 or 3 crashed the console's condition calls, and so does AnyPS5, which dereferences it.
pthread_attr_get_npreturnedEINVALfor the payload's main thread, so its answer for other threads stays unmeasured.Tested
Windows 11, MinGW (winlibs), and Linux (Ubuntu on WSL 2, GCC), at head
3345e2047d(main merged in after #9573, keeping itscreateSuspendedfield next to this PR's inheritance).guest_pthread_attrchecks the inheritancescePthreadAttrGetandpthread_attr_get_npreport for an explicit and an inheriting thread.guest_pthread_attr_errorschecksEINVALfor a nullscePthreadAttrGetinheritschedoutput.main'sPthreadsources and pass with this branch's.guest_pthread_attr,guest_pthread_attr_errors,guest_pthread_attr_stack,guest_pthread_create_suspended,guest_pthread_schedparam,guest_pthread_start,guest_pthread_join_detachandguest_kernel_sys_thread_defaults.guest_pthread_destroyed_objects, belong to fix(libkernel): mark a destroyed condition variable with 1 and accept destroying a null one, as measured on a PS5 #10177 and are reported there.tools/check_conventions.pypasses.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)