Repository navigation
feat(libkernel_sys): implement sigqueue - #9389
GitamarT045T3R wants to merge 3 commits into
Conversation
|
Exports with a name or library/NID already present in
|
|
Linux run, the platform the description doesn't list. Head
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
Independent Windows run of this PR (data only). Environment: Windows 10 Pro 10.0.19045, Ryzen 7 5800X3D, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release, BUILD_TESTING=ON. Merge-tree of the PR head against PR head tested: Results (3 runs each):
GPU: the run output has no Not run: other platforms, the other tests in the same family, the full repository CTest, and any title or console run. AI-assisted (Claude Code). |
There was a problem hiding this comment.
Changes requested The new implementation is registered only in libkernel_sys, but sigqueue is exported by both libkernel_sys and libkernel in the PS5 3.20 export inventory (same NID, TJG6tf+yJlY). A title importing that NID from libkernel will therefore still hit the unimplemented path. The current Windows import fallback adds libkernel when resolving imports from libkernel_sys or libkernel_web; it does not add libkernel_sys when resolving imports from libkernel. The current tree likewise has no sigqueue implementation/export in core/libs/prx/libkernel. Please cover the libkernel export too, with a test that imports through each module, or provide repository-specific evidence that the libkernel export is not required and adjust the stated scope accordingly. The independent PS5 export-inventory references are in the Linux test report comment above; the relevant fallback is core/relinker/elfpatcher/src/windows/WindowsEntryStubBuilder.cpp. ## Base and overlap coordination
I withdraw the queue-based intake hold. It is fine to keep submitting PRs; maintainers will handle them in due course.
The PR is also substantially behind current main (the checked base is d6732b51a64f4322799af89b876b224d3aed6efc; this head is 588 commits behind and 1 ahead). Please rebase/update and rerun checks before requesting another review. The overlap report also flags the shared libkernel_sys debt entry with #9195 and cannot compare multiple shared CMake/Export.cpp files due to base conflicts; manually coordinate those changes after updating. ## Checks observed - python3 tools/check_conventions.py --base upstream/main: passed. - git diff --check upstream/main...refs/review/9389: passed. - Local Linux guest_sigqueue_tests target: built successfully. - Local guest_sigqueue runtime: could not execute; the host CPU lacks LZCNT, BMI1, BMI2, MOVBE, AVX2, and FMA, which the project requires. - Existing hosted eligibility, conventions, intake, labels, and report checks passed on this PR head. Product build jobs were skipped. - The PR and independent Windows/WSL2 reports provide additional platform runs; I did not reproduce those locally.
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.
sigqueue follows FreeBSD 11 sys_sigqueue: EINVAL for a signal above 128 or a pid of 0 or below, ESRCH for another process, signal 0 only checks, and the guest's own pid gets the signal through libkernel's raise. The value is dropped, as libkernel delivers signals without siginfo.
…_sys to it The PS5 3.20 libkernel exports sigqueue under the same NID as libkernel_sys (TJG6tf+yJlY), and the Windows import fallback only looks in libkernel for libkernel_sys imports, not the other way. The implementation moves next to kill in libkernel as sigqueue_nid_disambig1, as fstat and pwrite are there, libkernel_sys's export forwards to it, the test runs every check through both exports and the debt entry moves to libkernel.
02d83c7 to
bddbe58
Compare
|
@MarcZX10 thanks for catching the libkernel export. Rebased onto
Tested:
On intake: no title is known to need |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Ran bddbe58 on native Linux (Ubuntu 26.04, glibc 2.43, x86_64 i7-8700, GCC 15.2.0, Release, merged --no-commit onto origin/main 7e2ba73). guest_sigqueue, guest_kill, guest_signals, guest_kernel_sys and patched_libraries pass.
One problem with the new forwarder on the NID-patched libraries. After patching, libkernel_sys.prx both exports TJG6tf+yJlY and imports it (its sigqueue_nid_postfix is jmp TJG6tf+yJlY@plt):
$ readelf -W --dyn-syms build/core/libs/libs/libkernel_sys.prx | grep TJG6tf+yJlY
76: 0000000000000000 0 FUNC GLOBAL DEFAULT UND TJG6tf+yJlY
357: 000000000000df30 9 FUNC GLOBAL DEFAULT 13 TJG6tf+yJlY
There's no symbol versioning, so that import binds to the first TJG6tf+yJlY in the lookup scope. If libkernel_sys.prx comes before libkernel.prx, it binds to itself and sigqueue never returns. The relinker keeps the title's DT_NEEDED order, so a title that lists libkernel_sys first hits this. A minimal executable linked against the patched .prx files, which looks up TJG6tf+yJlY and calls it with (-1, 1, nullptr):
DT_NEEDEDlibkernel_sys.prx, libkernel.prx: resolves tolibkernel_sys.prx, spins untiltimeout 15kills it.DT_NEEDEDlibkernel.prx, libkernel_sys.prx: resolves tolibkernel.prx, returns -1.
guest_sigqueue can't see this because it links the unpatched libraries, where the two names are still different. Calling a helper that patching leaves alone would avoid it, for example Sigqueue_nid_no_patch in libkernel's Signals.cpp called by both exports, the way #9693 uses SetLogin_nid_no_patch across the same two modules.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
After NID patching libkernel_sys's sigqueue forwarder imported the NID it exports itself, so on ELF hosts it could bind to itself when libkernel_sys comes before libkernel in the lookup order. Both exports now call GuestSigqueue_nid_no_patch in libkernel, which patching does not rename, as GuestAlarm_nid_no_patch is shared with libSceLibcInternal.
|
@jotitadev thank you, that self-binding is real: after patching, the forwarder's import and the export are both On the NID-patched libraries (Windows build), |
|
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. |
Took the PR's structure (libkernel helper GuestSigqueue_nid_no_patch, libkernel's sigqueue_nid_disambig1 export, libkernel_sys's sigqueue calling the helper) but with our implementation, which queues the value with SI_QUEUE through GuestSignalQueue where the PR raises without it; moved our debt line to libkernel.md in place of the PR's; the PR's test now runs only the libkernel export (guest_kernel_sys_ipc covers the libkernel_sys one).
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.
|
Hi @GitamarT045T3R, 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 |
|
Re-checked head It needs two changes to build on current main:
With those two changes, AI-assisted: yes (Claude Code); results above were run on this machine. |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Re-ran the self-binding case from my earlier review on 3a75bf461: it is fixed.
Machine: Linux x86_64, i7-8700, Ubuntu 26.04, glibc 2.43, GCC 15.2, Release, BUILD_TESTING=ON, full build. Head 3a75bf461 merged --no-commit onto main abed03546, with the two changes @wachi35676 described to make it build (drop the sigqueue and settimeofday stubs in Export.Discovery.cpp, and the three-argument sigqueue_nid_postfix declaration in libkernel_sys/include/Api.hpp).
- NID-patched libraries:
libkernel_sys.prxexportsTJG6tf+yJlYand no longer imports it. Its only related import isGuestSigqueue_nid_no_patch, whichlibkernel.prxexports next to its ownTJG6tf+yJlY. - Same minimal executable as before,
dlsym(RTLD_DEFAULT, "TJG6tf+yJlY")called with(-1, 1, nullptr):DT_NEEDEDlibkernel_sys.prx, libkernel.prx: resolves tolibkernel_sys.prx, returns -1.DT_NEEDEDlibkernel.prx, libkernel_sys.prx: resolves tolibkernel.prx, returns -1.
- Control, with
Signals.cppandlibkernel_sys/Export.cppas inbddbe58a2(the forwarder):libkernel_sys.prximportsTJG6tf+yJlYagain, and the libkernel_sys-first executable spins untiltimeout 15kills it.guest_sigqueuestill passes on that control, since it links the unpatched libraries. guest_sigqueue3/3.guest_sigqueue,guest_kill,guest_signals,guest_signal_list,guest_signal_sets,guest_kernel_sys,patched_librariesandconventions_unknown_nidspass, 8/8.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
@GitamarT045T3R Thanks. This no longer merges with |
What
sigqueuein libkernel_sys was aNotImplementedstub. It now follows FreeBSD 11'ssys_sigqueue:_SIG_MAXSIG), or negativeEINVALEINVALEINVAL: only a single processEINVALpfindfails,ESRCHESRCHpksignalwithSI_QUEUEand the valueraise, so ignored, blocked, default and handled signals behave as they do forkillThe signal check comes before the pid check, as in FreeBSD.
Gaps, in a new libkernel_sys debt entry: the value is dropped. libkernel delivers signals without
siginfo, sosi_codeandsi_valueare not set and anSA_SIGINFOhandler throws, as it does forraise. The limit of 128 pending queued signals per process (EAGAIN) is not modelled.Tested
Windows 10 Pro 22H2 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Ninja, Release,
BUILD_TESTING=ON, onmaincecb540a6:guest_sigqueuecovers:errnoalone;EINVALfor signals -1 and 129 to any pid, and for pids 0, -1 and the negated guest pid;ESRCHfor other pids, including with signal 0;sigqueuereturns;mainthe first call throwssigqueue_nid_postfix not implemented.guest_killandguest_kernel_sysstill pass.python tools/check_conventions.py --base origin/mainpasses.getpidandraise.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)