Repository navigation
Conversation
Progress report (801d5b9 → 581d45c)📈 System libraries: 48.84% (+0.01%, +2 implemented functions) ✅ 2 implemented
Changed libraries (implemented exports)
|
|
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.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Requesting changes for the open integration conflict. The sigvec conversion itself matches the pinned FreeBSD compat-43 path, and the focused tests cover flag/mask round-trips, aliasing, invalid input and missing registration. However, I ran git merge-tree between the current #10953 head and open #10749; it reports a content conflict in core/libs/prx/libkernel/System/src/Signals.cpp. If #10749 lands first, its removal/move of the RegisterSignalsOnLoad callback setup can also drop this PR's RegisterSigvec_nid_no_patch registration unless that is explicitly carried into the new owner path.
The PR description anticipates preserving that callback, but I could not find an agreed merge order with #10749's author. Please coordinate the order/resolution with that author and record the result (and any needed Depends on entry) before approval. The submitted Linux checks pass; this is specifically about maintaining a buildable and registered integration after the shared-file conflict is resolved.
ahmadmuj
left a comment
ahmadmuj
left a comment
There was a problem hiding this comment.
Checked head fa4e8f1698fe merged onto main a5a780eaaa7f (clean merge). Linux, GCC 13.3, Release.
What I ran:
- built
guest_sigvec_tests,guest_signal_tests,guest_signal_set_tests;ctest -R '^(guest_sigvec|guest_signals|guest_signal_sets)$'is 3/3 green - mutation: dropping the
^ 2on install makesguest_sigvecfail with "legacy interrupt flag reverses restart flag", so the flag loop really catches a wrong conversion - walked
LegacySignals.cppagainst FreeBSD 11.4lib/libc/compat-43/sigcompat.csigvec()line by line: same^ SV_INTERRUPTboth ways, empty mask plus low word on install,osvwritten only whenret == 0, nullsv/osvpassed through. Convertingsvbefore the call also keeps the aliased case right compute_nid("sigvec")fromtools/nid_names.pygivesyhxKO9LYc8A
One thing to change: the progress report counts sigvec as a stub (libc 1340/1351 -> 1340/1352 in the bot comment above). tools/progress.py marks any APS5_VABI body that calls NotImplemented_nid_no_patch as todo, and the missing-backend guard does that. siginterrupt handles the same missing registration with a plain throw (Process.cpp:309), which counts as done. With
if (forward == nullptr) throw std::runtime_error("sigvec: libkernel has not registered sigaction");progress.py --module libc gives 1341 done / 11 todo with sigvec done, and guest_sigvec still passes (NotImplemented_nid_no_patch throws std::runtime_error too, so the MissingBackend catch is unchanged).
Not checked: which PS5 module exports sigvec (no dump here), Windows/macOS. The #10749 ordering question in the earlier review still stands; I didn't look into that part.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I found one guest-memory output path that needs guarding before this is ready. When previous is non-null, sigvec_nid_postfix writes handler, mask, and flags directly into that caller buffer after the forwarded sigaction call. If previous points into GuestArena-backed memory, these stores bypass the GuestArena::HostWrite bookkeeping used to notify shared backing/write-watch and do not handle a rejected read-only Windows mapping. Please open a writable scope for the full GuestSignalVector output before mutating it and handle Open() failure without partially writing the result; add a guest-backed or read-only-view regression if the harness supports it. The current test uses stack GuestSignalVector objects, so it does not exercise that path. I checked the frozen-head diff with git diff --check and python3 tools/check_conventions.py --base upstream/main --head refs/pr/10953; both pass. The hosted eligibility, intake, conventions, labels and report checks pass; hosted product builds are skipped. The 5/5 Linux CTests in the PR description are author-reported, not independently rerun here, and PS5/Windows/macOS behavior remains unverified.
|
Correction to the check summary in my review: I re-ran |
fa4e8f1 to
9a8104e
Compare
|
Fixed in
The branch itself also passed 5/5 focused Linux checks, conventions and whitespace checks. No new hosted product-build or Windows/macOS/console execution is claimed. Please re-review the updated head. A maintainer full Linux/Windows CI run would exercise the new Windows rejection cases before merge. |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact updated head 9a8104e0296bf7d7f882b99f5749744557298fe7. The previous-vector buffer is guarded across its full range before calling the forwarded sigaction, so a rejected guest write returns -1/EFAULT without changing the disposition or partially updating the vector. Guest-backed tests cover cross-page output, input/output aliasing, canaries, and Windows read-only rejection. The missing backend now throws an ordinary runtime error, so progress no longer classifies this implementation as a stub.
The registration setup was moved to a separate kernel translation unit; the author reports git merge-tree is clean with #10749 and a combined libc/kernel/Internal build with 5/5 focused tests, including the relocated siginterrupt registration. I independently ran python3 tools/check_conventions.py --base upstream/main --head HEAD and git diff --check upstream/main...HEAD; both pass. Windows rejection cases are present but not reported as executed, and broad hosted product jobs are skipped. The previously raised guest-output and integration-conflict issues are addressed.
|
Firmware 13.42 matches on the PS5 side.
The |
|
@AZLisme Windows run of
Each side built with one warning, which I didn't capture. (The first audit run on the |
|
@BunnyFl Thank you for checking the firmware layout, mask/flag conversion, aliasing and forwarding results. I am keeping that evidence labeled as execution of the 13.42 library with stubbed imports, separately from console hardware measurement. Your module identification exposed a packaging gap: the current head defines the export in libc, while lookup through the Internal handle on Linux succeeds only by finding its libc dependency. Internal owns no patched |
9a8104e to
9869d58
Compare
AZLisme
changed the title
|
Implemented in BunnyFl’s report exposed a real native-module gap: the previous Internal lookup could resolve libc through ELF dependency fallback. Both patched modules now define The complete previous-vector HostWrite is checked before action mutation, including aliasing. Missing-backend reporting uses the requested plain runtime_error. Independent sigvec registration is preserved; an actual clean union with current #10749 ( The own-tree actual-module suite passed 6/6 on Linux. Its standalone lookup test verifies actual NID ownership before calls in both load orders, cross-module state, errors and mapped output; an old Internal binary is rejected specifically for dependency fallback. Windows ownership/read-only branches are present but were not executed locally. Header/convention checks and independent peer review passed. Thank you @BunnyFl for identifying the native owner and @ycbuzi for the Windows run. The latter covers prior head Ready for re-review. Maintainers, please run the full Linux/Windows CI on this revised head and merge when satisfied. |
|
A Windows run of Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT, posix-seh), Release, merged twice: onto
What I tried locally, none of it pushed:
So I don't have a working Windows setup for this test to suggest. A smaller option, if the Windows path isn't needed in this PR, would be registering it for Linux only (line 181 has AI-assisted: yes (Claude Code) |
|
Thank you @ycbuzi for testing the revised The Windows module test is currently broken at this head; the Linux pass and skipped hosted product jobs do not establish Windows coverage. Please wait for the corrected head before merging. I will post the exact build/test scope and request a Windows retest once the change is ready. |
9869d58 to
664732c
Compare
|
@ycbuzi Thank you for the exact-head Windows reproduction. The Windows report identified real fixture mistakes: the standalone host inherited guest DWARF flags, its mixed-ABI catch could enter the reported GCC SEH code-generation path, and the runner set only LD_LIBRARY_PATH. The correction is committed at Fresh Linux actual-runtime validation passed 8/8, including the new probe path; the old Internal binary still fails the explicit ownership assertion. The current #10749 Your five repeated Windows test passes and header audits are credited specifically to Please rebuild the six focused executables with |
|
The Windows retest of As pushed, the build stops in both trees (onto Line 232 is the end of Two local changes make it pass (not pushed). They were run on the #10749 + - set_source_files_properties(${LIBS_INCLUDE_DIR}/tests/GuestSigvecModules.cpp PROPERTIES COMPILE_OPTIONS -fno-exceptions)
+ set_source_files_properties(${LIBS_INCLUDE_DIR}/tests/GuestSigvecModules.cpp PROPERTIES COMPILE_OPTIONS "-fno-exceptions;-fno-asynchronous-unwind-tables") ~Module() {
#ifdef _WIN32
- FreeLibrary(handle);
+ (void)handle;
#else
So the crash comes from unloading the patched modules on Windows. My guess, not checked, is that AI-assisted: yes (Claude Code) |
|
@ycbuzi Thanks for the detailed retest and the two verified local adjustments. The source-level unwind override was missing from For module lifetime, the fixture now pins each actual loaded Windows image with GetModuleHandleExW(PIN|FROM_ADDRESS), checks handle identity, and retains the normal FreeLibrary calls for its original LoadLibrary references. The images intentionally remain until process exit. This preserves both cold load orders in separate processes and avoids adding a speculative kernel shutdown change. The unload crash's cause remains unknown; explicit DLL unload is outside this fixture's coverage. The changed host fixture rebuilt and all eight Linux focused/header checks passed. Qualified production runtime/probe/patcher inputs remain byte-identical to the prior validation. The old Internal negative still fails for dependency fallback, conventions has zero errors, and independent peer review passed. Your 7/7 twice result is credited specifically to Ready for re-review. Please merge when the exact-head Windows retest, maintainer CI and review are satisfactory. |
|
Windows retest of
AI-assisted: yes (Claude Code) |
|
Thanks @ycbuzi, for the native Windows retest with no local changes. The PR now records your exact The final commit The prior code-identical Linux eight-check suite and peer review remain applicable; final conventions and whitespace checks pass. This is ready for merge following the usual maintainer CI/review process. |
|
@AZLisme The full CI build fails on both Linux and Windows at The test is registered in |
|
@oneandonlydean Fixed in
On Linux (normal source-root CMake, GNU 16.2.1, Release, Ninja), the actual registration audit passes with both this branch's checker and the checker from main That accounts for configured executable targets; it is not a 1,361-test runtime run. After a fresh targeted source build, the six actual CTests Production signal code, API and debt are unchanged. Static merging with main Please rerun full Linux/Windows CI on this head and re-review; once the required checks pass, please merge. |
|
@AZLisme A native Windows run of
AI-assisted: yes (Claude Code) |
|
@ycbuzi Thanks. That's the Windows check the new wrapper needed, and it's good that it found the modules with no |
|
Fixed the Linux compile failure from run 40359537854 in Fresh normal-root Linux validation used a private merge of exact CI base The preceding Please rerun the required full CI and re-review this head; merge once the required checks and review pass. |
|
Private five-topic Linux validation on Earlier |
|
Manual CI run Please re-review the wrapper/fixture fixes and merge when review and current integration checks are satisfactory. Existing prerequisite holds remain unchanged. |
e0382c6 to
581d45c
Compare
|
Rebased SIG onto actual main Fresh normal-root Linux Release validation passes all 68 selected CTests: three SIG/module/exit fixtures and 65 header tests, with no failures or skips. Both cold module load orders and provider/shared-callback checks pass. Conventions and whitespace checks pass. Historical Windows/CI results keep their original scope; no new Windows or full-suite result is claimed. Please rerun current-head maintainer CI and re-review; merge when satisfied with integration and review checks. |
|
Windows run of the current head
AI-assisted: yes (Claude Code) |
|
@ycbuzi Thanks for testing the exact current head The body keeps this as focused contributor evidence, with no negative-control or full-suite claim. Current-head full maintainer CI remains pending; the existing DLL-unload limitation is unchanged. |
|
Reviewed the overlap bot’s 16 debt-only additions and the five rows for #10856, #11130, #11831, #11872 and #11969. Their captured shared-file edits can be composed with our Published head |
|
Reviewed the new #11664 warning against the captured |
|
Reviewed the 12 debt-file warnings added by the overlap bot at 05:16 UTC, including re-added #11895. Each captured patch changes math implementation/tests and its libc debt. Its debt edit composes with our SIG debt edit over frozen |
|
Reviewed the captured #11947 |
|
Reviewed the captured overlap warnings for #11892 and #12068. Their shared debt and kernel CMake edits compose with our SIG changes over frozen |
|
Reviewed the newly captured overlap warnings for #9019, #11969 and #11625 against this PR's published
These bounded source checks found no required change to this PR or new prerequisite. They include explicit additive mappings where newer CMake/debt context differs; they are not executed integrations or new runtime results. The published head and its existing validation, platform, review and integration qualifications remain unchanged. Reviewed the new #10002 (observed Published Reviewed #10645 (observed Reviewed refreshed #9019 ( Reviewed #10345 ( 2026-10-29 native Linux follow-up: normal-root Release, GCC 16.2.1, AMD Ryzen7 255 exposed by KVM. Execution was pinned to main The paired baseline passed 41/41 selected runtime cases and 65/65 actual public-header checks; the composition passed 43/43 and 65/65 respectively, with zero failures or skips. Both lanes built real This qualifies the selected cases and public-header checks on pinned main 2026-10-30 Linux follow-up on fixed main The baseline passed 53 plain runtime CTests, four with logging enabled, and 65 public-header checks. The composition passed 63 plain runtime CTests, six with logging enabled, and 65 public-header checks, with zero failures, skips or NotRun cases. Please re-review and merge once the existing CI and integration requirements are satisfied; I will address any further findings. |
What
Implement native
libSceLibcInternalsigvec(yhxKO9LYc8A) and preserve libc compatibility. Each patched module owns its public NID and calls one unique private libc conversion core with shared sigaction registration state. Distinct existing host aliases avoid the libc preserve-export-list collision without changing patcher policy or forwarding through the public NID. One canonical shared header supplies the 16-byte vector ABI and typed declarations.Conversion retains the low mask word, flags XOR 2, null combinations, input/output aliasing and forwarded results. Complete previous-vector HostWrite is checked before backend mutation; rejection returns EFAULT without partial output or disposition changes. Missing backend registration throws runtime_error. Independent kernel registration keeps #10749's siginterrupt ownership separate.
Credit BunnyFl's firmware 13.42 report for identifying Internal as the native owner and checking ABI/conversion by executing firmware code under an emulator with recording imports. This was not a physical-console measurement. The source contract follows pinned FreeBSD 11.4 sigcompat.c; inherited signal-backend gaps remain in debt.
The standalone native fixture loads the actual patched modules without guest-runtime startup links, checks module ownership and cold registration, then loads its exception probe. The mixed-ABI source has final no-exceptions/no-asynchronous-unwind options, while utility sources retain native MinGW unwind tables/static runtimes. Windows images are explicitly pinned until process exit while original LoadLibrary references still receive FreeLibrary. Separate child processes retain both cold load orders. Explicit DLL unloading remains outside the fixture; the earlier unload crash's cause is unknown.
Rebased as
581d45c28237onto mainaebeaa53f54d. The final implementation, canonical ABI, positional CMake wrapper and fixtures are preserved. The debt conflict retains upstreamfrexpfand the remaining SIG gaps;sigblock/sigsetmaskremain deferred.Tested
Fresh fixed-main Linux validation on
0362b1521c4eand private SIG+all-six-ICU composition8a5c8c499f0e: baseline passed 53 plain runtime CTests, four with logging enabled and 65 public-header checks; composition passed 63, six and 65 respectively, with zero failures, skips or NotRun cases. Both SIG fixtures passed plain and with logging enabled, including both requested native module-open orders. The published head remains581d45c28237. This qualifies selected cases on that fixed private composition; full CI, later main, Windows/macOS, native signal context and provider unload remain outside this new run.Native Windows: ycbuzi reports current head
581d45c282merged cleanly with mainc547deaacb(reported mergefa54b0eba1), Windows 11 Home 10.0.26200 / WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release,BUILD_TESTING=ON. The seven named targets (libc,libkernel,libSceLibcInternal,nid_patcher,sigvec_modules_exceptions,guest_sigvec_tests,guest_sigvec_modules_tests) built, and bothguest_sigvecandguest_sigvec_modulespassed two repetitions withctest --repeat until-fail:2. This is attributed focused contributor validation, not local reproduction or full maintainer CI. No negative control, full-suite, five-PR-union, macOS or title run was reported.Fresh normal-root Linux Release build at
581d45c28237/ baseaebeaa53f54d: all 68 selected CTests passed with zero failures or skips:guest_sigvec,guest_sigvec_modules,guest_exit_orderand 65 public-header checks. Both Internal-first and libc-first child processes passed module ownership, shared conversion/callback state, exception and guest-output assertions. Independent source, build-input, artifact and runtime review passed.Conventions and whitespace checks passed. Static integration with current main
801d5b9bc110is clean; that later-main union was not run.Earlier maintainer Linux/Windows CI applies to the ordered five-PR union (#10953, #11137, #10995, #11165 and #11219) containing
e0382c66298eon mainb986c11a344e. Earlier native Windows contributor results apply tob397eedc42with main2cce8bc51d: Windows 11, WinLibs GCC 15.2.0 r7, Release, both SIG fixtures passed three repetitions. These results remain historical. Current-head full maintainer CI remains pending; focused Windows contributor validation is recorded above. No title, physical PS5 or macOS run is claimed.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)