Skip to content

feat(libkernel): implement sigsuspend on the pending-signal set - #5666

Merged
adriadam10 merged 3 commits into
boykopovar:mainfrom
Kurenaiiiii:feat/libkernel-sigsuspend
Oct 14, 2026
Merged

adriadam10 merged 3 commits into
boykopovar:mainfrom
Kurenaiiiii:feat/libkernel-sigsuspend

Conversation

@Kurenaiiiii

Copy link
Copy Markdown
Contributor

What

Adds sigsuspend (KZ-4qlqlpmo) to libkernel's signal model: swaps the process-wide blocked mask for its argument, waits as pause does for a pending signal that is not blocked and deliverable, restores the mask, delivers what the restore unblocked as _sigprocmask does, and returns -1 with EINTR; a null set returns -1 with EFAULT. Extends the pause, sigwait, sigwaitinfo debt entry and guest_signal_tests (null set, plus a suspender/sender pair covering delivery on the suspender, mask restore and EINTR, mirroring the sender pattern of guest_sigtimedwait_tests).

Tested

No full build in this container (no submodules/toolchain for it). Verified: the new wait path compiled and ran natively against the real Signals.cpp TU 10/10 (swap, pending take under the new mask, handler delivery on the suspender, mask restore, EINTR); both touched TUs parse clean; python3 tools/check_conventions.py passes. Full build and guest_signal_tests left to CI / reviewer run.

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: PPSA99869 Java 25 LAN / DNS (PPSA99869) #5501 (FAIL: unresolved ELF import KZ-4qlqlpmo from libkernel.prx, 4th loader blocker after sysctlnametomib/fegetenv/mincore)
  • GPUs tested: n/a
  • AI-assisted: yes

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One mask-width gap: GuestSignalSet has four 32-bit words and MaxSignal is 128, but sigsuspend copies only set->bits[0] into the 32-bit blockedMask; SignalBlocked also returns false for every signal above 32. A caught/pending signal represented in bits 1–3 therefore remains deliverable during sigsuspend even when the supplied mask includes it. The existing _sigprocmask shares this state-width limitation, so fixing the new entry point fully may require widening the shared mask representation and exercising a high-word signal in the regression test. The debt entry describes pending/wait support for all 128 signals.

@Kurenaiiiii

Copy link
Copy Markdown
Contributor Author

Thanks for the review — you're right about the width gap. I kept the 32-signal mask on purpose: blockedMask, SignalBlocked, _sigprocmask and sigpending all share that representation (recorded debt), so widening it means changing the shared state plus the tests that assert words 1–3 stay zero — a separate-PR-sized change touching heavily-tested code. I recorded the limit in the debt entry instead ("words 1 to 3 of the argument are not applied ... widening the shared mask is a follow-up"). If you'd rather have the widening in this PR, say so and I'll do it.

@MarcZX10

Copy link
Copy Markdown
Contributor

Follow-up note from #5669: that PR keeps the scalar low-word blockedMask for source compatibility, while adding words 1–3 to sigprocmask and sigpending. This makes high-numbered blocked signals active in the shared model. sigsuspend_nid_postfix currently swaps only set->bits[0], so it will need to save, replace, and restore all four words to preserve the full mask semantics once #5669 lands. I kept that separate from the mask-width PR; linking here for integration.

@Kurenaiiiii

Copy link
Copy Markdown
Contributor Author

Noted, thanks — and thanks for keeping #5669 source-compatible. Plan on my side: once #5669 lands I'll rebase this branch, extend the swap/restore to all four mask words, and add a high-word signal case to the test (the regression coverage you asked for in the first review). Until then this PR stays as-is against current main — no action needed here.

@pipe1os

pipe1os commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Full build and guest_signal_tests left to CI / reviewer run.

Did that on Windows: 95634182f merged onto main 0b6d033b8 (no conflicts), Windows 11, WinLibs GCC 15.2.0, Ninja Release. The new head ba13bf210 only changes the debt entry, so the code is the same.

The wait itself looks right to me: the first check runs under the pending lock and MarkPending takes the same lock, so nothing gets lost between the swap and the wait. Two things, separate from the mask width @MarcZX100 raised (#5669):

  1. Null set. sigsuspend goes through libthr's __thr_sigsuspend (thr_syscalls.c L640, thr_sig.c L621-L644), which copies *set in userland before the syscall (thr_sig.c L128), so a null set faults there instead of returning EFAULT. A local probe on the merge gets sigsuspend(NULL) = -1, errno 14. A throw would fit the "exact or throw" rule better; the EFAULT check in guest_signals would change with it.
  2. Duplicated loops. The wait is a copy of pause's loop and the restore a copy of the delivery loop in _sigprocmask. Moving pause's loop into a helper that can swap the mask, and restoring through _sigprocmask itself, would keep them from drifting apart. Just a suggestion.

In case it's useful, I put a test on a branch on top of your head: pipe1os:test/sigsuspend-freebsd-checks (git cherry-pick 79673e4f2). It adds to guest_sigtimedwait the null-set throw, a signal that is already pending when the wait starts, the old mask coming back afterwards, a 50 ms sender, and 500 rounds of a sender with no delay. On your head it aborts at the null check. With the null check throwing, it passes 10/10 here, the 500 rounds included.

AI-assisted.

@Kurenaiiiii

Copy link
Copy Markdown
Contributor Author

Did both. Null now throws std::invalid_argument, same as _sigprocmask for a bad argument. The wait loop is shared with pause, and the restore goes through _sigprocmask itself. The test covers the throw, a main-thread wake, and a sender wakeup, all on signal 30 so no host signal is involved. I looked at your branch but wrote the cases here to match this file. Left the mask width for #5669.

@Kurenaiiiii
Kurenaiiiii force-pushed the feat/libkernel-sigsuspend branch from 8733e94 to 1ce5c2c Compare October 14, 2026 06:14
@pipe1os

pipe1os commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Checked 1ce5c2c4e on Windows (merged onto main a94675fb1): guest_signals and guest_sigtimedwait pass 10/10, and the cases from my branch (null throw, a signal pending before the wait, the old mask coming back, a 50 ms sender, 500 rounds with no delay) pass on it as well. Java 25 LAN / DNS (PPSA99869) with #5502 + #5563 + #5643 + this head on main 107190d11 still gets past KZ-4qlqlpmo to its menu; starting the VM from there stops at getrlimit: unsupported resource 8, which is #5187. Looks good to me, thanks for taking it.

@github-actions

Copy link
Copy Markdown

Progress report (6546a33 → 1ce5c2c)

➖ System libraries: 88.22% (+0.0%, +1 implemented functions)

✅ 1 implemented
Library Name
libkernel sigsuspend_nid_postfix

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on Linux (merged onto main): libkernel builds and guest_sig* pass; without the Signals.cpp change the test doesn't link. Windows run by @pipe1os confirmed too. The 32-signal mask limit is recorded in the debt entry and is covered by the #5669 follow-up. Merging.

@adriadam10
adriadam10 merged commit eb59c76 into boykopovar:main Oct 14, 2026
8 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.

4 participants