Repository navigation
fix(tests): compare the enabled exception query with the environment the host keeps - #10626
maximilianfv wants to merge 1 commit into
Conversation
…the host keeps CheckEnabledExceptions writes x87 and MXCSR states with some SSE exceptions unmasked, calls fegetexcept, and expects the environment afterwards to equal the state it wrote. Under Rosetta 2 MXCSR keeps its six exception masks set, as CheckTraps already allows for, so whenever the test unmasks an SSE exception the state read back differs from the one written, and the check failed although fegetexcept changed nothing. Compare with the environment read back right after the write instead; on other hosts the two are the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head 7ce21df27a02852f76eb728b14ea0cc104fe2b95. Reading the effective x87/MXCSR state immediately after WriteEnvironment makes the preservation assertion portable to hosts that refuse to unmask SSE exceptions; the independent fegetexcept result assertion still checks x87 mask decoding and errno preservation. The test restores the seeded state before asserting, as before.
git diff --check upstream/main...HEAD and python3 tools/check_conventions.py --base upstream/main --head HEAD pass. Hosted eligibility, conventions, intake, and labels pass; product jobs were skipped. The Rosetta probe and test result in the PR body are author-reported; I did not independently reproduce them. I found no blocking issue in this test-only fix.
|
Follow-up to the new overlap notice for #8863: I fetched both current PR heads and checked their merge. The changes are in separate portions of |
|
Windows run of |
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed 7ce21df27a02852f76eb728b14ea0cc104fe2b95 against author merge base aeeaefad98613c63a49ea2daae4d4558304b14cd.
Approve. No actionable findings in the environment-preservation test correction.
The test now records ReadEnvironment immediately after WriteEnvironment, then compares the query's result with that actual host-retained state. This avoids treating host normalization as a query mutation while retaining the independently computed x87 mask expectation, errno check and restoration of seeded state. Reviewed the complete patch and Read/Write/query helpers.
Validation: head/baseline syntax and conventions/whitespace/registration passed. Focused native programs using each revision's exact CheckEnabledExceptions and environment helpers plus the pinned simple query functions passed all 64×64 mask combinations (4,096 each), linked to the existing libc for errno. Both pass on this CPU, so this is compatibility coverage rather than a reproduced normalization regression. The complete suite could not link to the older DLL, and full pinned FloatingPoint.cpp builds hit GCC Windows SEH assembler errors in other functions. No full-suite or cross-host runtime pass is claimed; hosted native jobs were skipped.
What
CheckEnabledExceptionsincore/libs/tests/GuestFloatingPoint.cpp(added in #9216, "feat(libc): implement fegetexcept") writes x87 and MXCSR states with some SSE exceptions unmasked, callsfegetexcept, and required the environment afterwards to equal the state it wrote. Under Rosetta 2 MXCSR keeps its six exception masks set, so whenever the test unmasks an SSE exception the environment read back differs from the state written, andguest_floating_pointaborted withenabled exception query preserves control modes and flagsalthoughfegetexceptchanged nothing.The test now reads the environment back right after the write and compares the environment after the call with that. On a host that keeps MXCSR as written, the read-back equals the state written, so the check is the same as before there. The
fegetexceptresult check (enabled exceptions use the x87 masks independently of SSE) is unchanged and passes.Host behaviour, from a standalone probe (x86_64 under Rosetta 2): MXCSR reads
0x1f80; after writing it with the six exception masks (0x3f << 7) cleared, which is0, it reads back0x1f80, so the masks stay set. The Rosetta behaviour is already recorded indocs/dev/debt/libc.md, so the debt file is unchanged.Precedent: mugurc's #8287 adapted two other checks in this file (
CheckFlags,CheckTraps) to the same Rosetta behaviour. Its first commit (65ba3a6) reads the environment back after the write inCheckFlags, which is what this PR does inCheckEnabledExceptions; its second (100148f) hasCheckTrapsexpect the SSE masks the host keeps throughHostKeepsSseTraps(). This PR uses the read-back because the check is thatfegetexceptchanges nothing, which comparing with the read-back states directly.CheckEnabledExceptionswas merged after #8287, so #8287 does not cover it. The closed whole-port PR #7332 by ycarogabriel-lgtm had the sameCheckFlagsread-back a few hours earlier (commit 3bb3929, "test(libc): compare effective floating point masks on Rosetta", 2026-10-20); neither touchesCheckEnabledExceptions. The whole-port PRs #7201 and #8658, and the PRs split from #8658 (#10362, #10419), do not touch this test. No open PR changesCheckEnabledExceptions.Tested
Apple M1 Pro, macOS 27.0.1, x86_64 build under Rosetta 2.
mainalone does not build on macOS, so this was tested in a tree made of mugurc's macOS series (27e9689) merged withmainat 1e20650, plus other unopened macOS fixes, toggling only this change.guest_floating_point:floating-point environment: enabled exception query preserves control modes and flags.Not tested on Linux or Windows. This is a test-only change; on a host that keeps MXCSR as written, the comparison is the same as before.
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