Repository navigation
refactor(libSceLibcInternal): serve siginterrupt from libSceLibcInternal - #10749
Conversation
siginterrupt is a libSceLibcInternal function that imports _sigaction on the console. libSceLibcInternal links libkernel, so it can call _sigaction and write the _sigintr set directly; the sigaction registration libc needed, its static initializer in Signals.cpp and the throw for a call before registration go away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Progress report (1e65852 → 24497b2)➖ System libraries: 26.65% (+0.0%, +1 implemented, -1 removed functions) ✅ 1 implemented
🗑️ 1 removed
Changed libraries (implemented exports)
|
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the relocation and the reported Windows coverage. The console evidence and the sequence match the stated FreeBSD implementation; the layout assertions and use of _sigaction before indexing the signal set are useful safeguards. I have not run the PR's Windows tests locally; the focused Windows results in Tested are author-reported, and hosted product jobs are currently skipped.
There are two unresolved integration points that affect the build/test result:
- The open #9001 test currently links only
libkernelandlibc. Once this PR removessiginterruptfromlibc, that test cannot link until its target also linkslibSceLibcInternal. The PR description notes this, but there is no confirmed order with #9001's author, and the checklist still saysDepends on: none. Please coordinate the target-link change with that author and record the agreed order/dependency in both PR descriptions. - I ran
git merge-treeon the current heads of this PR and #9499. The combined result has content conflicts incore/libs/prx/libkernel/System/src/Signals.cppanddocs/dev/debt/libkernel.md. The described resolution appears possible (retainRegisteredForJumps, omit the obsolete sigaction-registration initializer, and retain both debt updates), but the heads do not merge as-is. Please confirm the order/resolution with #9499's author and record it in the checklist before this can be approved.
The implementation may be correct independently, but the known open test and code conflicts need an agreed integration path. The relinker/product matrix is still running, so I will recheck it after these coordination points are resolved.
|
@ycbuzi #9499 (a signal blocked while its handler runs, and
They compose. #9499 goes after this PR (merge order only) and keeps both sides when it merges |
|
@ycbuzi heads-up: #9001 adds a siginterrupt test target, |
|
@oneandonlydean will do: once #9001 is on @EagleBearerYS thanks, that order works: #9499 goes after this one, keeping |
|
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 rechecked the current integration point against open #9001. The dependency in the body is now accurate: #9001 remains open and links |
|
@MarcZX10 the order is settled: @oneandonlydean decided above that #9001 merges first, so #9001 needs no change. The integration path is this branch merging |
|
The merge-base advanced: #10706 is now on |
Brings in boykopovar#10706's export headers. SignalList.cpp now takes GuestSigaction and _sigaction_nid_postfix from libkernel/include/Api.hpp, the siginterrupt_nid_postfix declaration moves from libc/include/Api.hpp to libSceLibcInternal/include/Api.hpp, and GuestSignals.cpp includes that header for its siginterrupt checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@MarcZX10 both integration points from your review are settled, and the combined path is now built and tested:
Could you take another look when you have a moment? AI-assisted: yes (Claude Code) |
|
@ycbuzi Following the integration review on #10953, I moved its sigvec registration into a separate kernel source/CMake entry and restored The updated #10953 head |
|
@AZLisme thanks for checking the combination. No other change is planned here that touches your registration. The only remaining edit to #10749 is the one recorded in the description for after #9001 lands: |
…kopovar#9001 test boykopovar#9001's guest_signal_interrupt_tests links libkernel and libc and includes libc's Api.hpp. With siginterrupt served from libSceLibcInternal, the test links libSceLibcInternal and includes its Api.hpp for the declaration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
#9001 has merged, so On Windows the following pass on this head: @MarcZX10, this is ready for another look whenever you have time. AI-assisted: yes (Claude Code) |
|
Thanks, I checked |
MarcZX10
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 32c682c803c4a11bd4d137c95f77e2e723f9911b against current main f99d7fece316e31878f0195f8bbc2ad2fbe635e4.
The earlier overlap items are resolved: #9001 and #9499 are merged, and #11504 now declares Depends on: #10749. The current-main merge preview is clean; git diff --check and tools/check_conventions.py pass against current main. I found no new issue in the relocation itself.
I cannot clear the review yet because the tested tree is still based on e0cd1259a9, while current main has moved substantially and changed the related libkernel/System/src/Signals.cpp initialization and signal-debt entries. Please sync this branch to current main and rerun the focused signal tests, public-header/export checks, and conventions on that integrated head. The current hosted Windows relinker matrix also has one red shader_cache_native_paths timeout (368/369 passed); it is unrelated by path, but remains a failed check. Please obtain a green rerun on the updated head.
The author-reported Windows/macOS results on the prior integrated heads are useful, but I did not run a local product build or tests. Product Linux CI is skipped. My existing changes-requested review remains for current-main validation and the failed hosted check.
|
Correction: please disregard the report that was here. It was written by my local assistant and it was wrong: it described a different function ( I'm syncing the branch with current AI-assisted: yes (Claude Code) |
Re-checked — test report is unrelated to this PRThe October 29 report says it tested The current head is still Sources |
…-libc-internal # Conflicts: # docs/dev/debt/libSceLibcInternal.md
|
@MarcZX10 I merged current On that head, on Windows: the 22 focused tests ( The push starts a new hosted run; I'll check that the Windows relinker job is green this time. AI-assisted: yes (Claude Code) |
|
@MarcZX10 I merged current On that head, on Windows 11, WinLibs GCC 15.2.0, Release:
I did not rerun the export-table comparison on this head. The push starts a new hosted run, and I'll check the Windows relinker job on it. The Tested section has the line. Could you re-review once that run is in? AI-assisted: yes (Claude Code) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@MarcZX10 I merged current The two Tested on Windows 11, WinLibs GCC 15.2.0, Release, on the previous head
AI-assisted (Claude Code). |
Picks up 929671a, which restores the check_skips.py that boykopovar#14881 overwrote. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolves docs/dev/debt/libc.md: main's updated _Thrd entry, and siginterrupt stays documented in libSceLibcInternal.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (13, not counted against this PR)Validation IDs already present on main (4 IDs in 751 tests)Skipped (59)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Resolves docs/dev/debt/libkernel.md: main's sigaction entry (SIGPIPE outside Windows) and this branch's signal entry, which points to libSceLibcInternal for siginterrupt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
Follow-up to #8872, as suggested by @harisibrar1 in its review:
siginterrupt(TsrS8nGDQok) moves from libc tolibSceLibcInternal/SignalList.cpp. On the console it is alibSceLibcInternal.sprxfunction (STT_FUNC, 128 bytes, absent from the kernel modules) that imports_sigintrandUDCI-WazohQ, which hashes from_sigaction(@mazen320's dynsym parse and Ghidra decompile on #8872, and the NID check there).libSceLibcInternallinks libkernel and already calls libkernel exports directly (utimesinFileTime.cpp,fcntlandcloseinDirectory.cpp). Sosiginterruptnow calls_sigaction_nid_postfix(UDCI-WazohQ, the import the console function uses; onmainit forwards tosigaction_nid_postfix,Signals.cpp:513-514) and sets or clears its bit in_sigintr_nid_postfixitself. The sequence is unchanged: FreeBSD 11.4lib/libc/gen/siginterrupt.c. Read the disposition, set or clear the bit withSA_RESTARTcleared or set, then install it again.EINVALfor a signal outside 1 to 128 comes from the first_sigaction, before the set is touched.This removes what #8872 needed while
siginterruptlived in libc:SigactionFn_nid_no_patchandRegisterSigaction_nid_no_patchin libc'sGeneral.hpp;Process.cpp, including the throw for a call before registration;Signals.cppthat registeredsigaction.On
mainthe relinker gives a module that importslibSceLibcInternalbothlibSceLibcInternal.prxandlibc.prx(GuestModuleBuilder.cpp:368-371), so a title'ssiginterruptimport now resolves inlibSceLibcInternal.prx. With #10706's export headers,SignalList.cpptakesGuestSigactionand_sigaction_nid_postfixfromlibkernel/include/Api.hpp, and thesiginterrupt_nid_postfixdeclaration moves fromlibc/include/Api.hpptolibSceLibcInternal/include/Api.hpp, sopublic_headersstill finds one declaration per export.guest_signal_testslinkslibSceLibcInternalandGuestSignals.cppincludes its header for the existingsiginterruptchecks. The debt entry moves fromlibc.mdtolibSceLibcInternal.md.#9001 (@Queuse, test only) merged first, as @oneandonlydean asked on this PR. Its
guest_signal_interrupt_testslinkedlibkernel libcand tooksiginterrupt_nid_postfixfrom libc'sApi.hpp. With this PR it linkslibSceLibcInternaltoo (target_link_libraries(guest_signal_interrupt_tests PRIVATE libkernel libSceLibcInternal libc)), andGuestSignalInterrupt.cppincludesprx/libSceLibcInternal/include/Api.hppnext to its libc include. Those two lines are in29c52418f2, after mergingmain.Overlap: #9499 (@EagleBearerYS) touches the same spots without changing what it does. In
Signals.cppitsRegisteredForJumpsregistration goes in the anonymous namespace whoseRegisterSignalsOnLoadthis PR removes, and inlibkernel.mditssigactionentry sits next to thesignalentry changed here. #9499 merged first after all; mergingmainhere keeps itsRegisteredForJumpsregistration in that namespace and dropsRegisterSignalsOnLoad, and keeps itssigactionentry next to this PR'ssignalentry. #10706 and #9001 landed first;mainis merged in with the header moves and the two lines above. #10615 and #10648 only touch the neighbouring lines oflibc.md.Tested
Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0, Release, head
68a0d205c0(main809c7fa2dfmerged in): 24 focused tests (thelibSceLibcInternaltests, the signal tests,guest_user_contextandguest_user_context_exit) andpublic_headers(65 tests) pass 2 of 2;tools/check_conventions.pypasses.Windows 11 Home 10.0.26200, WinLibs MinGW-w64 GCC 15.2.0 (ucrt-posix-seh r7), Release. Head
1c060e99b9(mainefd368fec6, with #9001, #10706 and #9499, merged in):libkernel,libc,libSceLibcInternaland the test executables with no errors. The one warning ismain's existing_sigintr_nid_postfix"initialized and declared 'extern'" atSignals.cpp:24.ctest --repeat until-fail:2:guest_signal_interrupt(test(libc): cover siginterrupt state boundaries #9001's test),guest_signal_handler_mask(fix(libkernel): block a signal while its handler runs and honour SA_RESETHAND #9499's),guest_signal_sets,guest_signals, the 17 libSceLibcInternal tests andguest_user_context(22 tests) pass both runs.public_headersand its 64 compile batches (65 tests) pass.tools/check_conventions.py --base origin/mainpasses.objdump -p): onlylibSceLibcInternal.prxexportssiginterrupt_nid_postfix;libkernel.prxexports_sigaction_nid_postfixand_sigintr_nid_postfix;libc.prxhas none of them.main88795ed448(head432aed7b7b, 0 behind): the same 22 tests pass 2 of 2,public_headers(65 tests) passes,tools/check_conventions.py --base origin/mainpasses and the export tables are the same. The only warning is stillmain's_sigintr_nid_postfixone, now atSignals.cpp:26.maine0cd1259a9(head32c682c803, 0 behind; the one conflict was the neighbouringcollate<char> vtableentry indocs/dev/debt/libc.md, kept asmainhas it): the same 22 tests pass 2 of 2, the 64public_headers_compile_*batches pass,tools/check_conventions.py --base origin/mainpasses and the export tables are the same. Thepublic_headersaudit passes (368 libraries, 12811 exports) run directly, but took 120 s on this machine against the test's 90 s timeout while another build ran, so under ctest it timed out.main420b6a064e(headf430c9ec4e, 0 behind; the one conflict was feat(libc): throw std::filesystem::filesystem_error from _Xfilesystem_error as the console's libc does #11504's removal of the_Xfilesystem_errorentry next to this one indocs/dev/debt/libSceLibcInternal.md, kept asmainhas it): the same 22 tests pass 2 of 2,public_headers(65 tests) passes,tools/check_conventions.py --base origin/mainpasses and the export tables are the same. The only warning is stillmain's_sigintr_nid_postfixone, now atSignals.cpp:30.Before #9001 merged, on
main2dc85609fc: itsGuestSignalInterrupt.cppatb5e40d1d3e(all four set words, extreme flags, errno, neighbouring bits, thesignalinterplay, invalid signals) passed both runs on this branch withlibSceLibcInternaladded to its link line, and linked aslibkernel libcit failed with undefined references.libSceLibcInternal.prximports_sigaction_nid_postfixand_sigintr_nid_postfixfromlibkernel.prx.Not tested: Linux, macOS, a title.
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)🤖 Generated with Claude Code