Skip to content

fix(libc): preserve errno across fmodf domain errors - #9643

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
Noxy-HNR:fix/libc-float-domain-errors
Oct 23, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
Noxy-HNR:fix/libc-float-domain-errors

Conversation

@Noxy-HNR

@Noxy-HNR Noxy-HNR commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What

Preserve errno around fmodf, as the existing double-precision fmod wrapper does. The original Windows library changes sentinel errno 73 to EDOM 33 for fmodf(1, 0), leaking the host CRT's policy into this model.

FreeBSD 11.4's C implementation returns the invalid-operation NaN without an errno store. Its selected amd64 assembly implementation likewise has no errno store. The wrapper continues to use the host remainder result and flags; console errno, flags and NaN payloads remain unmeasured and are recorded in debt.

Tested

Windows 11 26H2 (build 10.0.26300), WinLibs GCC 15.2.0 r7, full Release build, head ee057ea on main d6e7ab4.

  • guest_math and guest_floating_point pass, 2/2.
  • The regression covers both zero divisors, infinite operands, quiet and signaling NaNs in either operand, ordinary signed remainders, signed zero, subnormal operands and results, errno preservation, FE_INVALID and preservation of an existing exception, and restores the prior floating-point environment and errno. Against main's fmodf it fails with errno 73 changed to EDOM 33.
  • fix(libc): compute asinf with FreeBSD msun's algorithm, as titles' libc.prx does #9771 also edits core/libs/prx/libc/src/Math.cpp next to fmodf. Either landing order gives one conflict hunk there, resolved by keeping both functions; the tests and docs/dev/debt/libc.md merge cleanly. That combination built and passed guest_math and guest_floating_point, 2/2, and the #9771 author confirmed either order is fine.
  • tools/check_conventions.py --base origin/main and tools/check_test_registration.py pass. tools/pr_overlap.py and merge tests against the 103 open pull requests that touch these files find no conflict other than fix(libc): compute asinf with FreeBSD msun's algorithm, as titles' libc.prx does #9771.
  • Linux, PS5 hardware and title behavior of this revision are untested here.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One 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 file
  • No notes, investigation .md files 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

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

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.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 9771 core/libs/prx/libc/src/Math.cpp L293
PR 10129 docs/dev/debt/libc.md L59-60
PR 10134 docs/dev/debt/libc.md L59-60

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.

Pull request Shared files not compared
PR 3965 docs/dev/debt/libc.md
PR 5021 docs/dev/debt/libc.md
PR 5303 docs/dev/debt/libc.md
PR 6044 core/libs/tests/GuestMath.cpp
PR 6158 core/libs/prx/libc/src/Math.cpp
core/libs/tests/GuestMath.cpp
PR 6469 docs/dev/debt/libc.md
PR 7081 docs/dev/debt/libc.md
PR 7201 core/libs/prx/libc/src/Math.cpp
PR 7648 docs/dev/debt/libc.md
PR 7715 docs/dev/debt/libc.md
PR 8150 docs/dev/debt/libc.md
PR 8306 docs/dev/debt/libc.md
PR 8658 core/libs/prx/libc/src/Math.cpp
core/libs/tests/GuestMath.cpp
PR 8965 docs/dev/debt/libc.md
PR 9133 docs/dev/debt/libc.md
PR 9408 docs/dev/debt/libc.md
PR 9447 docs/dev/debt/libc.md
PR 9526 docs/dev/debt/libc.md
PR 9527 docs/dev/debt/libc.md
PR 9552 docs/dev/debt/libc.md
PR 9669 docs/dev/debt/libc.md
PR 9929 docs/dev/debt/libc.md

@rooklz

rooklz commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Data on the console behaviour this models, from the acosf of the libc.prx bundled with PPSA28319 (catalog zip sha256 6426468091b4…, the same binary the sinhf, asinl and atanhf debt entries cite), export QI-x0SL8jhw at 0x40980 of the unwrapped module:

  • For |x| <= 1 it is FreeBSD msun's e_acosf.c instruction for instruction: the same branch points (0x3f800000, 0x3effffff, 0x32800000), pS0..qS1 (0x3e2aaa75, 0xbd2f13ba, 0xbc0dd36b, 0xbf34e5ae), pio2_hi 0x3fc90fda, pio2_lo 0x33a22168 read from .data (msun's volatile), and the 0xfffff000 mask on sqrtf(z).
  • When hx & 0x7fffffff is above 0x3f800000, which covers |x| > 1, ±inf and every NaN, it calls a helper at 0x11740 with edi = 1 (FE_INVALID) before returning (x - x) / (x - x). The helper stores 0x21 (EDOM) through libkernel's __error (the import slot at 0x10f8b8 is 9BcDykPmo1I), calls the module's feraiseexcept (0x11a70: fnstenv/fldenv and stmxcsr/ldmxcsr with the flags or-ed in, then fwait), and stores EDOM again because bit 0 is set (ERANGE, 0x22, for the overflow and underflow bits).

So on the console acosf(2.0f) sets errno to EDOM and raises FE_INVALID, and a quiet NaN argument takes the same branch, so it does too. That differs from "keeps errno" and "leaves a quiet NaN alone" in this PR's debt line, and from the errno == 0 check #3688 added from FreeBSD's source, where e_acosf.c itself doesn't touch errno but the console's build of it does. This is a reading of the binary, not a measurement on a console.

I'm not touching those paths. Separately I'm porting the |x| <= 1 part, since the host libraries differ there: over the 2,130,706,434 floats in [-1, 1], msun's acosf differs from glibc 2.39's on 1,046,180 and from Apple's x86-64 libm on 17,127,560 (acosf(0.5f) is 0x3f860a91 on macOS, 0x3f860a92 in msun). That changes the lines below the ones you edit, so the two should merge cleanly.

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux run, since the description lists Linux behavior as untested: the updated guest_math is red on main and green with the PR on glibc.

Machine: Linux, Ubuntu 26.04, GCC 15.2.0 (glibc host libm), x86-64, i7-8700. Head bd769c3c3 merged with git merge --no-commit onto current main d6eafa54d. Full build, BUILD_TESTING=ON, ANYPS5_RELINKER_ONLY=OFF, Ninja Release; the whole build passes.

  • PR test with main's Math.cpp: guest_math aborts on the first new case: Guest acosf failed for 3f800001: got ffc00000, expected ffc00000, invalid 0, errno 73, the same missing invalid flag and untouched sentinel errno described for Windows.
  • PR: guest_math and guest_floating_point pass (2/2).

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@MarcZX10

MarcZX10 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

The Linux run independently confirms the test goes red on main and green with this PR for the missing FE_INVALID flag. However, the console-binary analysis posted above for PPSA28319 reports acosf domain errors also set errno=EDOM, including the quiet-NaN branch, while the current code restores errno and the debt entry says it is preserved. Because that evidence is a direct disassembly report, please reconcile the implementation, tests and debt text with it before marking the draft ready; if you have contrary evidence, include the exact module/version and call path. The separate fmodf errno-preservation fix appears internally consistent. Current-main conventions and whitespace pass. Hosted relinker/registration checks pass; full product jobs are skipped.

@rooklz

rooklz commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

A data point from @JJenkx on #9668 that narrows what I wrote above: with __error supplied to a harness, PPSA13934's libc.prx did not set errno for acosf(2.0f). That copy is a different build (FMA-contracted in the |x| <= 1 part), so the EDOM store I described is confirmed for PPSA28319's build only, and the error path can differ between the builds titles ship.

@Noxy-HNR
Noxy-HNR force-pushed the fix/libc-float-domain-errors branch from bd769c3 to 43c4203 Compare October 22, 2026 12:44
@Noxy-HNR Noxy-HNR changed the title fix(libc): preserve float domain flags and errno fix(libc): preserve errno across fmodf domain errors Oct 22, 2026
@MarcZX10

MarcZX10 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

I rechecked the updated head 43c4203. The prior review concern about acosf no longer applies to this patch: its implementation change is now limited to preserving errno around fmodf, and the test/debt additions cover that behavior. I withdraw the earlier request to reconcile acosf behavior for this version of the PR.

On this Linux host, I compiled and ran a UBSan-enabled focused harness using the new wrapper and the PR's fmodf vectors; result values, FE_INVALID, sentinel errno, and preservation of an existing floating-point flag all passed. This was a standalone harness, not the project's guest_math executable. Current hosted conventions, eligibility, labels, report, relinker Linux/macOS, register-context, write-watch and ASan checks pass; full Linux/Windows product jobs are skipped by path filters and Windows relinker is still pending. No new code issue found in the updated scope.

@MarcZX10

Copy link
Copy Markdown
Contributor

While reviewing #9771, I compared the current heads. Its asinf insertion and this PR's fmodf change produce an adjacent-hunk conflict in Math.cpp, but the two edits compose by retaining both functions. CONTRIBUTING.md asks authors to agree merge order for overlapping PRs and record a dependency when one must land first. Could you coordinate with the #9771 author whether either order is acceptable, and note that agreement on the PR checklist? No functional conflict is apparent from the pairwise merge.

@JJenkx

JJenkx commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

@Noxy-HNR I maintain #9771 (asinf). Either merge order works from our side: if #9643 lands first, I will rebase #9771 retaining your fmodf change and our asinf implementation/tests, then rerun guest_math. If #9771 lands first, the adjacent-hunk resolution should retain both functions. There is no functional dependency from #9771 on #9643. Please confirm whether either order also works for you so we can record the agreement on #9771.

@Noxy-HNR
Noxy-HNR force-pushed the fix/libc-float-domain-errors branch from 43c4203 to 7b839b0 Compare October 22, 2026 17:58
@Noxy-HNR

Copy link
Copy Markdown
Contributor Author

Rebased onto main b7d1d47. guest_math and guest_floating_point pass, 2/2, on Windows 11 with GCC 15.2.0; conventions pass. The stale "full suite still running" and #5239 draft notes are gone, and Tested is updated.

#9771 edits the line next to fmodf in Math.cpp, so either landing order gives one conflict hunk, resolved by keeping both functions; the tests and debt file merge cleanly. I built that combination and guest_math and guest_floating_point pass, 2/2, so either order works.

@Noxy-HNR
Noxy-HNR force-pushed the fix/libc-float-domain-errors branch from 7b839b0 to ee057ea Compare October 22, 2026 20:11
@Noxy-HNR
Noxy-HNR marked this pull request as ready for review October 22, 2026 20:20
@MarcZX10

Copy link
Copy Markdown
Contributor

I’ve noticed that asking you to hold this PR was unnecessary. It’s fine to keep submitting PRs; maintainers will handle them in due course.

@oneandonlydean oneandonlydean self-assigned this Oct 23, 2026
@oneandonlydean
oneandonlydean merged commit 8dcbc4e into boykopovar:main Oct 23, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants