Repository navigation
feat(libc): implement feupdateenv - #9232
Conversation
|
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.
|
|
Reviewed the overlap with #9296 at head |
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main a6646f8 and FreeBSD amd64 fenv.c / x86 fenv.h @d106002967:
- The sequence matches FreeBSD's
feupdateenv: savefnstsw|stmxcsr,fesetenv(fldenv+ldmxcsr), thenferaiseexcept(fnstenv/OR/fldenv, MXCSR OR,fwait). An unmasked pending x87 exception traps at the samefwait, and masked ones only set flags in both units, as on FreeBSD. - It uses main's guest
GuestFloatingPointEnvironment(28-byte x87 env + MXCSR at 0x1c) with rawfnstenv/fldenv, so MinGW's hostfenv_tandFE_DFL_ENVsentinel never come into play. Good. - The
fxsaveMXCSR_MASK read (dword 7, 0xFFBF fallback) is the right offset. - NID
YXQ4gXamCrYhashes fromfeupdateenv, and the branch still merges cleanly into current main. - Nit, optional: the debt entry might sit better next to the existing
fegetenv/fe* entries at the top of "Unknown function info" than as a separate paragraph above### Functional. - FYI: the
linux/windowslibs CI jobs were skipped on this PR, so the POSIXSIGFPEside of the new trap tests hasn't run anywhere yet.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
Refreshed onto main d6e7ab4 in 5dfd505. The new conflict was only the expected adjacent debt insertion from merged #9296: both entries are preserved, and I placed feupdateenv next to fegetenv as suggested. The original runtime implementation and GuestUpdateEnv test source are retained. Rebuilt affected Windows libraries and environment/math/loader targets; all 23 selected tests pass, including the 18 pending-exception source/flag cases. Current conventions, whitespace and registration pass (1178 executables, none unregistered). The first build command referenced a nonexistent math target; the corrected invocation built the actual targets before these results. No Linux/macOS/console retest, and the POSIX SIGFPE branch remains unrun locally. Thank you for the source review; the description preserves those validation limits. @harisibrar1 |
What
libSceLibcInternalimports offeupdateenv(YXQ4gXamCrY) have no implementation in the shared libc provider. Restore the supplied 32-byte x87/MXCSR environment, then raise the union of the previously pending flags in both units and executefwait, following FreeBSD 11.4 amd64 fenv.c. This preserves the saved rounding, precision, masks, tags and instruction state while delivering enabled x87 exceptions during the call.The name/NID is corroborated by the public 3.20 inventory and symbol database; the signature and layout follow the public SDK fenv.h. Console behavior and module/library versions remain unmeasured and are recorded in debt. Null inputs and host-unsupported MXCSR bits throw before changing state. No host default-environment sentinel is substituted for the still-missing
__fe_dfl_envobject.This implementation builds independently of the other open fenv contributions.
Tested
Windows x86-64, GCC 15.2 Release. No games or GPU workloads were run.
fegetenvround trips and two concurrent host threads.libsbuild and final CTest selection: 22/22 pass, including 360 patched libraries loaded, 0 failed. A separate single-libc host fixture also passes the main test and all 18 traps through canonical NIDs. Its first attempt mixed patched/unpatched libc instances and terminated on the null-input exception; correcting the fixture's imports to use the patched personality and canonical dependency exports resolves that harness failure without a library change.Linux/macOS and console execution are unavailable here. The POSIX trap source checks
SIGFPEwithout inspecting the specific signal-context flag; it was not run here. Compilation and export presence are not claimed as console behavior proof.Current-main review follow-up
Merged main
d6e7ab42dfe191818721c6c5ad9aee4070b434aawithout rewriting published history; head5dfd50502a067ef8b4c141bb030fc78a35803789. Resolved only the adjacent libc debt conflict with merged #9296, preserving its snprintf/vsnprintf entry and this PR's feupdateenv note. Moved the latter next to the existing fegetenv entries as suggested in review. The original feupdateenv runtime/test implementation is retained.Rebuilt affected Windows libraries and the environment/math/loader targets: 23/23 selected tests pass, including all 18 pending-exception source/flag cases. Current conventions, whitespace and registration checks were repeated. No fresh Linux/macOS/console execution; in particular, the POSIX SIGFPE branch remains unrun locally. The initial build invocation named a nonexistent math-bits target and stopped before compilation; corrected to the actual guest_math/guest_math_exact targets for the passing build above.
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)