Skip to content

fix(libkernel): return EINVAL from scePthreadCondattrSetclock and Destroy for a null attribute - #7029

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/condattr-setclock-einval
Oct 17, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/condattr-setclock-einval

Conversation

@milescoviello

@milescoviello milescoviello commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

What

scePthreadCondattrSetclock and scePthreadCondattrDestroy threw std::invalid_argument for a null or uninitialized attribute. FreeBSD 11's libthr _pthread_condattr_setclock and _pthread_condattr_destroy return EINVAL for one, as the other scePthread attribute calls here already do (scePthreadAttrDestroy, scePthreadAttrSetdetachstate, ...). Both now return SCE_KERNEL_ERROR_EINVAL (0x80020016), which pthread_condattr_setclock maps to EINVAL.

The clock id is still stored as given. libthr accepts only CLOCK_REALTIME, CLOCK_VIRTUAL, CLOCK_PROF and CLOCK_MONOTONIC. But there's no console evidence for which ids PS5's scePthreadCondattrSetclock takes, and pthread_cond_timedwait here serves the other ids clock_gettime supports, so this PR leaves them working (see the review below).

I don't know of a title that hits this. I found it while comparing the condition-variable attribute calls with libthr.

Tested

Ubuntu 24.04, GCC 13.3, Release, on main 6a32550.

Not checked: macOS, a console.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

🤖 Generated with Claude Code

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@milescoviello thanks, the review passed and this matches FreeBSD 11 libthr (thr_condattr.c accepts only REALTIME/VIRTUAL/PROF/MONOTONIC).

One question before we merge. On main, setclock stores any id, and timedwait then uses it through clock_gettime, which also serves 5, 7, 11, 12, 14 and 15 (UPTIME, the _PRECISE/_FAST variants and the CPU clocks). With this PR those clocks now return EINVAL. A title that ignores the error would keep REALTIME while it computes deadlines on its uptime clock, so its timedwait would time out immediately.

Do you have console evidence (a hardware trace or a homebrew test) that PS5's scePthreadCondattrSetclock rejects those clocks too? If Sony's libthr accepts any of them, we'd want to keep those working.

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 4972 core/libs/prx/libkernel/CMakeLists.txt
PR 5021 core/libs/prx/libkernel/CMakeLists.txt
PR 6044 core/libs/prx/libkernel/CMakeLists.txt
PR 6222 core/libs/prx/libkernel/CMakeLists.txt
PR 6287 core/libs/prx/libkernel/CMakeLists.txt
PR 6487 core/libs/prx/libkernel/CMakeLists.txt

…troy for a null attribute

Both threw std::invalid_argument for a null or uninitialized attribute.
FreeBSD's libthr returns EINVAL for one, as the other scePthread
attribute calls here already do. The clock id is still stored as
given.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@milescoviello milescoviello changed the title fix(libkernel): reject condition variable clocks libthr rejects fix(libkernel): return EINVAL from scePthreadCondattrSetclock and Destroy for a null attribute Oct 17, 2026
@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks for checking. No, I don't have console evidence. The PR only cited FreeBSD 11's libthr, and I'd marked the PS5 side as unchecked. You're right that rejecting 5, 7, 11–15 could break a title that uses one of them and ignores the error, where it works today.

I've dropped the clock check (62ac8d6, force-pushed as one commit). scePthreadCondattrSetclock stores any id again, as on main. What's left is EINVAL from scePthreadCondattrSetclock and scePthreadCondattrDestroy for a null or uninitialized attribute, where main throws. The test now also checks that 5 and 12 are accepted. I updated the title and description to match.

AI-assisted: yes (Claude Code)

@screamm

screamm commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Windows run in #7023 (comment) on the trimmed head 62ac8d6: guest_pthread_condattr 3/3, and clocks 0, 1, 2, 4, 5 and 12 are accepted, so the clock ids oneandonlydean asked about keep working. With the source reverted: "Condition attributes are not initialized".

Written with Claude Code assistance (validation by a subagent, reviewed in this session).

@oneandonlydean oneandonlydean self-assigned this Oct 17, 2026
@oneandonlydean
oneandonlydean merged commit 4827f36 into boykopovar:main Oct 17, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants