Repository navigation
Conversation
…andler Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
I reviewed the implementation and regression path. The handler now mirrors the I independently ran |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. I merged current |
|
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 reviewed head The stated dependency on #9087 is still open; both update the |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head f96b6d2fb8e39db1f0fb8d9c5f3777aee65833bc against current main 895f47f4eeaf56048b9a7080b7c01a9a47cf0ecf. The default handler writes its supplied message and newline to guest stderr and returns; this is the behavior attributed to the three shipped libc.prx disassemblies, and the debt entry separately retains the measured libSceLibcInternal.sprx process-termination difference. The regression redirects guest stderr, checks two exact messages, and verifies both violating calls return EINVAL. The author reports five focused/neighboring Windows tests passing three repetitions after merging main; I did not rerun the C++ suite locally. I independently ran current-main git diff --check and conventions; both pass. Hosted eligibility, conventions, intake, labels, relinker, ASan, register-context and write-watch checks pass; full Linux/Windows product builds are skipped by repository policy. The declared #9087 debt-text integration is appropriate and no intervening current-main commit changes the reviewed source, test or debt paths. I found no remaining issue in the reviewed change.
|
@Pouare514 @JJenkx: #7081 and #9688 change the same lines as this PR in |
|
@boykopovar #10706 replaces the local |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@B-Howell the overlap check lists #11621 here: it adds entries next to the |
|
@orezraey a note for the merge order: #12574 makes The two conflict only on the AI-assisted: yes (Claude Code). |
|
@orezraey a heads-up from #12589, which makes Whichever of the two lands second can update that one line. #12589 notes it, and with that change a local merge of #9935 and #12589 passes AI-assisted: yes (Claude Code). |
|
@JJenkx either order works. If #12589 lands first, I'll merge |
|
@JJenkx #12598 conflicts with this PR only in |
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — new head, looks good
The current head retains the reviewed default-handler behavior and test, and the rebase passes the available static and hosted checks.
What I ran
Reviewed PR head 7aa063d91cb079e5d82cdb64aff5731288b0406f against base f80ce6cec1cad29aa45bd0bf6d8062a714c18e03; refreshed current main is f46f453a56f91d77e29ce0cbd7fe5bfb2fb69834. The merge preview with current main is clean (tree a678797797647eee21884965d95daac02477dea6). Environment: Linux 6.1.0-54-amd64 x86_64; no local compiler, build, or tests run.
git diff --check upstream/main...refs/review/pr-9935-current
python3 tools/check_conventions.py --base upstream/main
git merge-tree --write-tree upstream/main refs/review/pr-9935-current
gh pr checks 9935 --repo boykopovar/AnyPS5Results
| Check | Result |
|---|---|
| Diff whitespace check | Pass; exit 0, no output. |
| Repository conventions | Pass. |
Merge preview with current main |
Clean. |
| Hosted relinker, ASan, register-context, write-watch, conventions, eligibility, labels, and report checks | Pass on this exact head. |
| Hosted Linux and Windows product jobs | Skipped by the workflow. |
Focused guest_safe_errors and neighboring tests |
The author reports three passing runs on Windows and WSL2 on this exact head; not rerun locally. |
Unedited output from local and hosted checks
$ git diff --check upstream/main...refs/review/pr-9935-current
(exit 0; no output)
$ python3 tools/check_conventions.py --base upstream/main
All conventions checks passed.
$ git merge-tree --write-tree upstream/main refs/review/pr-9935-current
a678797797647eee21884965d95daac02477dea6
$ gh pr checks 9935 --repo boykopovar/AnyPS5
conventions pass 17s https://github.com/boykopovar/AnyPS5/actions/runs/40971070959/job/123787653774
eligibility pass 5s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122005990939
labels pass 10s https://github.com/boykopovar/AnyPS5/actions/runs/40971071060/job/123787654343
register-context (macos-15-intel, Debug) pass 53s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006026989
register-context (macos-15-intel, Release) pass 1m7s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006027030
register-context (ubuntu-latest, Debug) pass 31s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006026976
register-context (ubuntu-latest, Release) pass 28s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006027024
relinker (macos-latest, clang++) pass 2m54s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006027082
relinker (macos-latest, clang++, x86_64) pass 5m20s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006027040
relinker (ubuntu-latest, g++) pass 3m41s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006027013
relinker (windows-latest, g++) pass 7m57s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006026952
relinker-asan pass 2m34s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006026978
write-watch (macos-15-intel) pass 49s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006026977
linux skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006028604
windows skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006028418
report pass 38s https://github.com/boykopovar/AnyPS5/actions/runs/40448684047/job/122005990560
write-watch (macos-15) pass 37s https://github.com/boykopovar/AnyPS5/actions/runs/40448684045/job/122006026999
The focused test results are author-reported in the PR description; they are not local runs.
Negative control
| Assertion | Change to code under test | Did the new test fail? |
|---|---|---|
| The default handler returns instead of aborting and writes its message and newline | The author reports running the added test against main without this PR. |
Yes; it ended with exit code 0xc0000409 at the first violation. |
Findings
Blocking
None found in the current head. It writes the supplied message and newline to guest stderr, then returns to the caller; the test exercises two violations and confirms both return EINVAL and produce the expected output.
Non-blocking
The PR records merge-order dependencies for adjacent libc debt and test changes. Preserve that order if one of those dependent changes lands first.
Sources
Not verified
I did not build or run tests locally, independently reproduce the author's Windows/WSL2 tests, or verify behavior on a physical console. The hosted Linux and Windows product jobs were skipped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
boykopovar#12589 passes "strerror_s: bad argument" to the constraint handler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…traint-handler-returns
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This change reached |
|
Thank you for this. The change is already on main through #10270 (merging this head with main gives main's tree unchanged), so we will close this one as already merged. |
What
The default runtime-constraint handler of
libccalledstd::abort()without writing anything. It now writes the message and a newline to the gueststderrand returns, so an_sfunction called with no handler installed returns its error to the title instead of ending it.This follows the default handler of the
libc.prxtitles ship insce_module/, which this module stands for. JJenkx read it from the disassembly of the copies in PPSA13934, PPSA07923 and PPSA05916 (#9087 comment):set_constraint_handler_sstoreshandler ? handler : default, and the default callsfputs(message, stderr),fputc('\n', stderr)and returns, whileabort_handler_smakes the same two calls and then callsabort. Thewcsrtombs_sdebt entry already describes that handler the same way. The console'slibSceLibcInternal.sprx(firmware 13.60, measured for #9087) has a different default that ends the process after writing; that module is not this one, and the debt entry says so.libSceAmpr's re-export shares the header, so its default handler changes the same way.Tested
main(f80ce6cec1, with docs(libc): record feholdexcept, fegetexcept, setlocale and strcasecmp as measured on a PS5 #11121):7aa063d91c. The only conflict wasdocs/dev/debt/libc.md: kept main'slocaleconventry (and its removal of thesetlocaleentry) and this PR'sset_constraint_handler_swording.guest_safe_errorspasses three runs in a row on Windows (MinGW) and on Linux (WSL Ubuntu, GCC);check_conventionspasses.main(f26434afa3, with refactor(libs): add full export headers for libs #10706):0bae438695. The test now takes its declarations from the libc headers;SonyCrt.hppincludesApi.hppinstead of declaringfputs/fputcagain.guest_safe_errors,guest_crt,guest_bounds_checking,guest_qsortandguest_wide_stringspass three runs in a row on Windows (MinGW) and on Linux (WSL Ubuntu, GCC);public_headersaudit andcheck_conventionspass.main(a2c36d4ed6, with fix(libc): report strerror_s violations as titles' libc.prx does #12589):9260172a4c.guest_safe_errorsnow expects the default handler to writestrerror_s: bad argument\ntwice, the message fix(libc): report strerror_s violations as titles' libc.prx does #12589 passes.guest_safe_errors,guest_crtandguest_bounds_checkingpass three runs in a row on Windows (MinGW, GCC 15.2) and on Linux (WSL Ubuntu, GCC);check_conventionspasses.Windows 11, MinGW (winlibs).
guest_safe_errorsnow installs the default handler withset_constraint_handler_s(NULL), redirects the gueststderrto a file and checks that twostrerror_sviolations returnEINVAL(22) and writestrerror_s: bad argument\ntwice. Onmainthe test ends with0xc0000409at the first violation; with the change it passes.guest_safe_errors,guest_crt,guest_bounds_checking,guest_qsortandguest_wide_stringspass three runs in a row.tools/check_conventions.pypasses.main(ba8dcc6ae8, with test(libc): register the macOS write watch fault tests once #13220, which removes the duplicate write-watch test names that failed the macOS jobs):d7b7f9ad59.guest_safe_errorspasses three runs in a row on Windows (MinGW, GCC 15.2).tools/check_conventions.pypasses.main(98346b748c, with fix(libc): sort like the console's qsort and qsort_s #9688'sqsort_sentry):91d71873be. The only conflict wasdocs/dev/debt/libc.md: kept main'sqsort_sentry and this PR'sset_constraint_handler_swording.guest_safe_errorspasses three runs in a row on Windows (MinGW, GCC 15.2) and on Linux (WSL, GCC 16.2).tools/check_conventions.pypasses.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)set_constraint_handler_sdebt entry and edits the same lines; merge order only; fix(libc): sort like the console's qsort and qsort_s #9688 has merged); fix(libc): follow snprintf_s and sscanf_s constraints as measured on a PS5 #9087 lands after this PR and will be refreshed then; refactor(libs): add full export headers for libs #10706 removes the test declarations this PR adds toGuestSafeErrors.cpp(merge order only); feat(libc): implement lfind and lsearch #11621 (B-Howell) addslibc.mdentries next to the one this PR edits (merge order only); fix(libc): check qsort_s arguments as titles' libc.prx does #12598 (JJenkx) rewrites theqsort_sentry next to theset_constraint_handler_sentry this PR edits (merge order only)