Skip to content

fix(libc): keep frexp from raising FE_INVALID for a signaling NaN - #4946

Closed
ErenYeager1836 wants to merge 1 commit into
boykopovar:mainfrom
ErenYeager1836:carry/libc-frexp-signaling-nan
Closed

ErenYeager1836 wants to merge 1 commit into
boykopovar:mainfrom
ErenYeager1836:carry/libc-frexp-signaling-nan

Conversation

@ErenYeager1836

@ErenYeager1836 ErenYeager1836 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

Under IEEE 754 and FreeBSD's s_frexp.c and s_frexpf.c (libm), frexp and frexpf store 0 in *exp for zero, infinity, and NaN and return the argument unchanged without raising the FE_INVALID floating-point exception for a signaling NaN.

In core/libs/prx/libc/src/Math.cpp, frexp and frexpf forwarded directly to host std::frexp. On hosts where std::frexp evaluates floating-point arguments via FPU operations, passing a signaling NaN can raise FE_INVALID, and *exp is left unspecified by C17 7.12.6.4 for infinity and NaN.

This change:

  • Uses bitwise checks (std::memcpy into integer words) to identify zero, infinity, and NaN without tripping host floating-point exception traps on signaling NaNs.
  • Stores 0 in *exponent and returns the argument unchanged for these special values as FreeBSD libm does.
  • Adds test coverage for special values, zeros, infinities, quiet NaNs, and signaling NaNs in GuestMath.cpp (CheckFrexpSpecialValues).
  • Documents technical debt in docs/dev/debt/libc.md.

Tested

  • Windows 11, GuestMath.cpp unit tests.
  • Verified python tools/check_conventions.py --base upstream/main passes with no violations.

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
  • Unblocks: none
  • AI-assisted: yes

@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows test of this PR.

Environment: Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, BUILD_TESTING=ON, -j2, libusb-cmake from a local checkout. Built from main ea2bbefb2 with this PR and six other open PRs (#4958, #4956, #4954, #4953, #4946, #4942, #4927) merged together; the merges had no conflict.

Result: guest_math (1 of 1; the test file is shared with #4942, both merged together) pass.

Not checked: whether the test fails without the change in the library code; the PR was not tested alone, only in the combined merge (the tests of the other PRs in the batch also pass); no title run; Linux not tested.

AI-assisted (Claude Code).

@nicolasfalesy

nicolasfalesy commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Linux test (Arch, GCC 16.2.1, glibc, Release), this PR merged onto main 7e181fc together with #4963, #4953, #4946 and #4942 (no conflicts): guest_math passes. With this PR's Math.cpp change reverted it fails on Linux too, so the check guards the change on both systems.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@ErenYeager1836
ErenYeager1836 force-pushed the carry/libc-frexp-signaling-nan branch from 919963b to f24f7ee Compare October 13, 2026 12:10
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@ErenYeager1836
ErenYeager1836 force-pushed the carry/libc-frexp-signaling-nan branch from f24f7ee to c33caa0 Compare October 13, 2026 18:40
@github-actions

github-actions Bot commented Oct 13, 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
#5495 docs/dev/debt/libc.md L36-37

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title is known to reach this. With the queue at 700+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line.

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.

5 participants