Skip to content

fix(libc): wrap narrow integers in sscanf_s as titles' libc.prx does - #12507

Open
JJenkx wants to merge 1 commit into
boykopovar:mainfrom
JJenkx:windows/libc-scanf-s-narrow-wrap
Open

JJenkx wants to merge 1 commit into
boykopovar:mainfrom
JJenkx:windows/libc-scanf-s-narrow-wrap

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

What

sscanf_s (and the other _s scanners that go through ScanSource: vsscanf_s, fscanf_s, vfscanf_s, scanf_s, vscanf_s) handed a narrow integer conversion to the host at the destination's width. On an out-of-range value the host saturates and sets ERANGE. Titles' own libc.prx (PPSA07923 and PPSA05916 agree) instead reads the field as a 64-bit value and keeps its low bits, leaving errno alone:

input format titles before this PR
61439ebdf %x 0x1439ebdf, errno unchanged 0xffffffff, ERANGE
9c747bca9c %x 0x747bca9c 0xffffffff, ERANGE
-eb371ade9 %hx 0x5217 0xffff, ERANGE
867259ccd %hhX 0xcd 0xff, ERANGE

Plain sscanf on Windows already does this: ScanWindowsArguments scans narrow integers as ll and truncates. ScanSource now does the same for %d, %i, %o, %u, %x and %X with no length, h or hh: it scans into a 64-bit temporary and stores the destination's low bytes. Past 64 bits the host still saturates with ERANGE, as the titles do (%d of 99999999999999999999 sets ERANGE in both).

Tested

Windows 11 Enterprise 10.0.22621, Ryzen 9 5950X, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release, BUILD_TESTING=ON, on main 6793a72475.

  • Against titles' own libc.prx: I loaded PPSA07923's and PPSA05916's libc.prx natively and called their sscanf_s (24m4Z4bUaoY) side by side with this build. It covered 200,000 random numeric conversions: d i u o x X f e g a, every length, random widths, and inputs drawn from digits, hex letters, signs, ., x, e, p, inf/nan letters and spaces. I compared the result, the stored bytes, %n and errno.
    • narrow-integer differences: main 21 → 0, on both titles;
    • what's left is unrelated to this change and listed in the debt file or measured separately:
      • 0x with no hex digit (78, where the titles fail the conversion);
      • out-of-range %f/hex %a errno (37, where the titles leave errno);
      • one long double NaN's upper bytes.
  • Tests: guest_formatting (three new rows), guest_stdio_variadic, guest_scan_count, guest_wide_stream_scan and guest_wide_formatting pass in each of 3 runs.
  • Negative control: with main's Formatting.cpp, guest_formatting fails.

Not tested on Linux or macOS. ScanSource is the same code there, so Linux sscanf_s now also scans these conversions as 64-bit and truncates. I haven't measured what glibc did with them before.

Overlap: tools/pr_overlap.py finds no shared lines: the 14 pull requests touching these files merge cleanly with this one. #5021, #7386, #10324 and #11332 conflict with main and couldn't be compared.

Independent Windows validation reported by bunbeans2-arch for topic 0b3eb0b3e53a8faa1f01fd01a6de4061247ab308 merged into main 409fe64199: Windows 11 Pro 22631, i9-9900K, MSYS2 UCRT64 GCC 16.1, Release. guest_formatting passes 5/5; retaining this PR's tests while restoring Formatting.cpp to main fails 5/5; plain main with its own source/tests passes 3/3. This is the contributor's run, not a new local Windows execution.

Independent reports on this head (their runs, summarised here):

  • @bunbeans2-arch (report): Windows 11 Pro, MSYS2 UCRT64 GCC 16.1.0, head 0b3eb0b3e5 merged into main 409fe64199: guest_formatting passes 5/5; with Formatting.cpp from main it fails 5/5.

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 (Claude Code)

🤖 Generated with Claude Code

The sscanf_s of titles' libc.prx (PPSA07923 and PPSA05916 agree) reads a
%d, %i, %o, %u, %x or %X with no length, h or hh as a 64-bit value and
keeps its low bits: %x of 61439ebdf stores 0x1439ebdf and leaves errno
alone. ScanSource handed the directive to the host at the destination's
width, which saturated to 0xffffffff and set ERANGE. It now scans those
conversions into a 64-bit temporary and stores the low bytes, as the
plain Windows sscanf path already does. Out of 64-bit range the host
still saturates with ERANGE, as the titles do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@MarcZX10 MarcZX10 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.

Reviewed exact head 0b3eb0b3e53a8faa1f01fd01a6de4061247ab308. For unsuppressed integer conversions with no length, h, or hh, the change scans through a 64-bit temporary and copies only the destination-width low bytes, avoiding the host-width saturation observed for narrower destinations. The tests cover %x, %hx, %hhX, and preservation of the preexisting errno value; the PR also supplies paired title-libc comparisons across 200,000 conversions. Windows, conventions, policy, Ubuntu relinker/register-context and ASan checks pass. Linux is skipped and macOS jobs are pending; the PR description clearly records that host limitation and the remaining >64-bit overflow behavior.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

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 5021 core/libs/tests/Formatting.cpp
PR 7386 core/libs/prx/libc/src/Formatting.cpp
core/libs/tests/Formatting.cpp
PR 9340 core/libs/prx/libc/src/Formatting.cpp
core/libs/tests/Formatting.cpp
PR 11332 core/libs/prx/libc/src/Formatting.cpp
core/libs/tests/Formatting.cpp
PR 11724 core/libs/prx/libc/src/Formatting.cpp
PR 11726 core/libs/prx/libc/src/Formatting.cpp
core/libs/tests/Formatting.cpp
PR 13486 core/libs/prx/libc/src/Formatting.cpp
core/libs/tests/Formatting.cpp

@JJenkx

JJenkx commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

Linux validation completed for exact clean head 0b3eb0b3e53a8faa1f01fd01a6de4061247ab308 (no source changes or rebase).

Arch Linux 7.2.9-arch1-1, c++ (GCC) 16.2.1 20260810, glibc 2.44. Built the actual patched core/libs/libs/libc.prx and verified the sscanf_s NID export. guest_formatting, guest_stdio_variadic, guest_scan_count, guest_wide_stream_scan, and guest_wide_formatting each passed three runs: 15/15 executions, no skips. Conventions and whitespace checks passed.

A separate plain-glibc probe already truncates the three narrow-integer examples and preserves the existing errno. This establishes Linux compatibility; it does not reproduce the Windows host saturation bug. I did not rerun the Windows author's 200,000 title-libc comparisons.

Build/test logs, JUnit, exact-source and patched-library hashes, and host probe (SHA-256 3067e6c1661abeeb82da1d8e403b3f2489b4f2da2e2855b0567540467595f8e3).

@bunbeans2-arch

Copy link
Copy Markdown
Contributor

A Windows run with GCC 16: Windows 11 Pro 10.0.22631, i9-9900K, MSYS2 UCRT64 GCC 16.1.0, Release, this PR (0b3eb0b3e5) merged into main 409fe64.

  • With this PR: guest_formatting passed 5 of 5.
  • Same tree with only Formatting.cpp put back to main (this PR's tests kept): it failed with Formatting check failed, in 5 of 5 runs.
  • main 409fe64 alone (its own tests and source): the same tests passed 3 of 3, so the failure above comes from the checks this PR adds.

AI-assisted: yes (Claude Code)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants