Repository navigation
feat(libc): implement feraiseexcept - #8903
Conversation
|
I ran an additional native Linux check on exact head |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the new export and regression coverage. The implementation updates x87 and MXCSR flags independently, rejects unsupported bits before changing state, preserves errno/control fields, and uses fwait for enabled x87 exceptions. I independently ran the PR test on Linux with GCC 12.2 through a temporary host harness; the state, errno, thread, and six trap cases passed. Console behavior remains unmeasured and is clearly recorded in debt. The platform jobs still running are not covered by this approval.
|
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.
|
|
The implementation matches FreeBSD's amd64 feraiseexcept (fesetexceptflag on x87 status and MXCSR, then __fwait()), so valid masks behave the same way. |
|
Native Linux full-project validation on exact head Environment: Debian 13.7 x86-64, kernel 7.2.9-x64v3-xanmod1, Ryzen 5 7640HS, GCC/libstdc++ 15.2.0, CMake 3.31.6, Release, bundled dependencies, Repro / diagnosis:
ready.arrive_and_wait();
*__error_nid_postfix()=static_cast<int>(100+flag);
Require(raiseFlags(static_cast<int>(flag))==0,"thread return");
const int raisedError=*__error_nid_postfix();
ready.arrive_and_wait();
// Keep the existing environment checks here, then:
Require(raisedError==static_cast<int>(100+flag),"thread-local errno");Additional passing evidence:
Build/repro commands: CCACHE_DIR=/tmp/anyps5-ccache cmake --build build --target libc guest_raise_except_tests guest_floating_point_tests nid_patcher --parallel 2
ctest --test-dir build --output-on-failure --parallel 2 --repeat until-fail:3 -R '^(guest_raise_except(_trap_[0-5])?|guest_floating_point)$'All diagnostic variants stayed under |
|
Addressed both review points in 35c508e. The thread test now sets its distinct errno sentinel after barrier one and captures it immediately after The debt entry now explicitly states that FreeBSD does not mask or validate the requested exception bits, whereas this implementation rejects unknown bits before modifying state. Console behavior for those bits remains unknown. Conventions and whitespace checks pass. The PR description separates the original full-library validation from these focused review checks. |
|
@UnhealthyKraken #10688 landed and conflicted with this in |
|
@oneandonlydean Recorded your history-preserving main merge |
What
Implement the missing
feraiseexceptexport (NIfFNcyeCTo) by accumulating requested flags in both x87 and MXCSR, preserving other state and errno, and delivering enabled x87 exceptions withfwait. All six defined exception bits are supported; unverified bits throw before changing state. This deliberately does not call_Feraise, whose errno and range/inexact behavior differs.The signature is from the public SDK fenv.h. The public 3.20 table and symbol database corroborate the NID. For the six supported bits, behavior follows FreeBSD amd64 fenv.c; console equivalence and module/library versions remain unmeasured and are recorded in debt.
Unknown bits are explicitly rejected before changing state, unlike the FreeBSD reference, which does not mask or validate the requested bits. This rejection is an unsupported-path policy; PS5 behavior for those bits is unknown.
Tested
Windows x86-64, GCC15.2 Release, isolated checkout based on
1ff5c8ac5c3050d71360957c5ff1be38aaac7114. Regression first: seven new tests fail baseline's missing export while the existing floating-point test passes. With the same final executables and only the unpatched libc swapped, baseline is 1 pass/7 fail; candidate is 8/8 pass.The main case checks 196608 flag/control combinations: all 64 requested and prior flag sets, independent x87/SSE flags, 16 rounding combinations and three x87 precision modes. It checks errno, other control/status/tag/pointer fields, rejected masks, differing exception masks, and thread isolation. Six separate cases check enabled x87 traps; Windows validates the saved unmasked cause as well as the exception code. A direct native x87 load of the smallest subnormal and the implementation both produce Windows
0xc0000090on this host, so the denormal case accepts that code only when the saved active flag is DE, not invalid.At the original submitted head
2d22f777, the full Windowslibsbuild succeeded; validation was 9/9 pass, including all 354 patched libraries loading with zero failures. The expected NID is verified in patched shared libc, which supplies the interface for libSceLibcInternal under the existing provider convention. Loading verifies linkage only. No Linux/macOS or console execution by this contributor. The POSIX trap handler confirms SIGFPE but does not inspect its saved cause. No games run.Conventions, whitespace and registration checks pass (1148 targets, none unregistered). Independent read-only review found no actionable defect and did not rerun tests. The overlap report's 14 base-conflicted shared-file warnings were reviewed manually: no competing export. #6044 reorganizes tests; #6879 changes host math callers rather than adding this export. The original overlap run had no compared-file conflict or duplicate export. The review-correction scan also has none; its 14 current base-conflicted shared-file warnings were inspected at their fetched heads and concern unrelated stack/runtime, formatting, environment, archive, directory and test-reorganization work. #6044 remains a test-directory migration consideration when rebased. The debt paragraph was relocated away from #8397/#8287 documentation changes.
Review correction: the thread test now sets its errno sentinel after the first host barrier and captures errno immediately after the export, before the second barrier. The floating-point isolation checks and distinct thread sentinels are retained; no runtime code changed. Filip722's Linux review reports 22/30 original runs passing, barrier-induced errno changes, and 30/30 passing with this test correction; those are attributed reviewer results, not execution on this Windows host.
Locally, a deterministic temporary probe makes both barriers set errno to 789: the original test fails and the repaired test passes against the same preserved DLL. A negative control that changes errno inside the called export still fails the repaired assertion. The rebuilt eight Windows tests pass, the main test passes 30 consecutive runs, and identical final executables pass 8/8 against both preserved baseline and candidate DLLs (byte-identical SHA256
bf4a81f2a272152f4886821d3c432b6ce14b98302037c5439a91297888ad8a75). These review checks rebuild the affected targets, not the entire library collection. Conventions and whitespace checks pass; independent read-only review found no actionable issue and did not rerun tests.Checklist
Based on current
main; no other open PR implements the same functionsOne 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 fileNo notes, investigation
.mdfiles 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
Fixes: none
Unblocks: none
GPUs tested: n/a
AI-assisted: yes