Skip to content

fix(libc): report fopen_s and freopen_s argument errors to the constraint handler - #12597

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
JJenkx:windows/libc-fopen-s-constraints
Nov 1, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
JJenkx:windows/libc-fopen-s-constraints

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

What

fopen_s and freopen_s now report argument errors to the constraint handler, as titles' libc.prx does. The handler gets EINVAL and a null pointer, with these messages:

argument error message (both functions)
null FILE ** "fopen_s: bad FILE ** argument"
null filename (fopen_s), null mode, or null stream (freopen_s) "fopen_s: bad argument"

freopen_s really does report the fopen_s messages. On main both returned EINVAL without calling the handler. The return values and the stored stream (left alone for a null FILE **, set to null otherwise) were already right and are unchanged. Other errors, such as an invalid mode or a missing file, are not changed here.

Depends on #9935. On main the default constraint handler aborts, so until #9935 merges, a title that hits one of these violations without installing a handler would abort. Merges cleanly with #9935, #10755 and #11831.

Tested

Windows 11, WinLibs GCC 15.2.0 UCRT r7, Release, main 319621ba2e.

  • Native comparison: a scratch loader calls the titles' functions by NID next to this build's, with the same recording handler on both sides. It runs 12 cases: each null-argument case of fopen_s and freopen_s (including all null), plus gets_s with size 0, size 2^61 and a null buffer. Each case compares the return value, the stored stream, and the handler's calls, error, pointer and message.
    • main: the 9 fopen_s/freopen_s cases differ, and the 3 gets_s cases already match.
    • This PR: 0 of 12 differ.
    • PPSA07923 and PPSA05916 give the same results.
    • Paths that open a file aren't compared, because they need kernel imports the loader doesn't provide.
  • Regression: guest_fiopen and guest_fopen_errors install a recording handler around these cases and check each message. With the previous FileAndHeap.cpp both fail.
  • Test set: guest_fiopen, guest_fopen_errors, guest_safe_errors, guest_bounds_checking and guest_crt pass 3/3. Conventions pass.

Current qualification — 2026-10-30

Integrated upstream main 08a10d225306ad102b57408e2ef7566bcfb829b2 at 1768c1821b2921745f051fb97b999bb63b97f38f, retaining the actual previous head and main as parents. Manual conflict resolution changes only debt documentation; production and test files equal the automatic merge output. Both parents' added test lines and main gitlinks are preserved.

Linux CPU: 9 passes (guest_fiopen, guest_fopen_errors, guest_crt); public_headers 1 passes; no skips. MinGW source syntax and conventions pass.

No new Windows/macOS runtime, native SDK, game, GPU or performance claim. The Windows user pause remains in force.

Evidence: immutable source and qualification receipts, SHA-256 61f5500e7526ea4e107159927de78c37db7e0ac1cb7ebd8625b22ab0ad5d9bd7 (download verified).

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: fix(libc): write the constraint message and return from the default handler #9935
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

…aint handler

The libc.prx that titles ship (PPSA07923, PPSA05916) calls the
constraint handler with EINVAL and a null pointer when fopen_s or
freopen_s gets a null FILE ** ("fopen_s: bad FILE ** argument") or a
null filename, mode or stream ("fopen_s: bad argument"); freopen_s
reports the fopen_s messages too. main returned EINVAL without the
call. Return values and the stored stream are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oneandonlydean oneandonlydean self-assigned this Oct 28, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@JJenkx the code matches your description and the tests check each message; one leftover to fix:

docs/dev/debt/libc.md still says the opposite of what this PR does. The freopen_s entry (line 231) reads "... a null result, mode or stream returns EINVAL (22) ... and the constraint handler is not invoked, as in fopen_s." After this PR the handler is invoked for a null FILE ** and for a null mode or stream, and fopen_s does the same for a null FILE **, filename or mode. Please update both entries (fopen_s, line 214, and freopen_s) to state the new rule, as you did for strerror_s, wcsrtombs_s and the others in this series: the handler gets EINVAL, a null pointer and "fopen_s: bad FILE ** argument" or "fopen_s: bad argument" (the titles use the fopen_s text for freopen_s too), from the libc.prx PPSA07923 and PPSA05916 ship, with the paths that open a file not compared. The null filename case of freopen_s still reports ENOTSUP without the handler, which the entry already says and the test checks.

@adriadam10 adriadam10 self-assigned this Oct 28, 2026
@github-actions

github-actions Bot commented Oct 28, 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 11172 docs/dev/debt/libc.md L239-240
PR 11625 docs/dev/debt/libc.md L238-239
PR 12920 docs/dev/debt/libc.md L257

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 docs/dev/debt/libc.md
PR 6565 docs/dev/debt/libc.md
PR 7386 docs/dev/debt/libc.md
PR 7447 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 7715 docs/dev/debt/libc.md
PR 8150 docs/dev/debt/libc.md
PR 8965 docs/dev/debt/libc.md
PR 9340 docs/dev/debt/libc.md
PR 9371 docs/dev/debt/libc.md
PR 9447 docs/dev/debt/libc.md
PR 9526 core/libs/prx/libc/src/FileAndHeap.cpp
core/libs/tests/GuestFiopen.cpp
docs/dev/debt/libc.md
PR 9863 docs/dev/debt/libc.md
PR 9929 docs/dev/debt/libc.md
PR 10324 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 10405 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 10729 docs/dev/debt/libc.md
PR 10953 docs/dev/debt/libc.md
PR 10989 docs/dev/debt/libc.md
PR 11262 docs/dev/debt/libc.md
PR 11266 docs/dev/debt/libc.md
PR 11661 docs/dev/debt/libc.md
PR 11689 docs/dev/debt/libc.md
PR 11698 docs/dev/debt/libc.md
PR 11831 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 12560 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 12586 docs/dev/debt/libc.md
PR 12682 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 12688 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 12700 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 12965 core/libs/prx/libc/src/FileAndHeap.cpp
docs/dev/debt/libc.md
PR 13442 docs/dev/debt/libc.md
PR 13499 docs/dev/debt/libc.md
PR 13678 docs/dev/debt/libc.md
PR 13793 docs/dev/debt/libc.md
PR 13809 docs/dev/debt/libc.md
PR 14212 docs/dev/debt/libc.md
PR 14235 docs/dev/debt/libc.md

Both debt entries still said the constraint handler is not invoked.
They now give the measured messages and note that the null-name
freopen_s case keeps ENOTSUP without a handler call. Pointed out by
oneandonlydean in review.

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

JJenkx commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@oneandonlydean thanks, fixed in 9eb070d687 (docs-only, on c1cc55c47e). Both entries now state the new rule:

  • fopen_s: a null FILE ** reports "fopen_s: bad FILE ** argument", and a null filename or mode "fopen_s: bad argument", each with EINVAL and a null pointer. The measurement came from the libc.prx of PPSA07923 and PPSA05916, and the paths that open a file aren't compared.
  • freopen_s: reports the same fopen_s texts for a null result, mode or stream. The null-name case keeps ENOTSUP with no handler call, as the test checks.

Conventions pass.

AI-assisted: yes (Claude Code).

@github-actions github-actions Bot added libc docs needs rebase Conflicts with main, rebase needed labels Oct 29, 2026
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 30, 2026
@JJenkx

JJenkx commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Integrated upstream main 08a10d225306ad102b57408e2ef7566bcfb829b2 at 1768c1821b2921745f051fb97b999bb63b97f38f, retaining the actual previous head and main as parents. Manual conflict resolution changes only debt documentation; production and test files equal the automatic merge output. Both parents' added test lines and main gitlinks are preserved.

Linux CPU: 9 passes (guest_fiopen, guest_fopen_errors, guest_crt); public_headers 1 passes; no skips. MinGW source syntax and conventions pass.

No new Windows/macOS runtime, native SDK, game, GPU or performance claim. The Windows user pause remains in force.

Evidence: immutable source and qualification receipts, SHA-256 61f5500e7526ea4e107159927de78c37db7e0ac1cb7ebd8625b22ab0ad5d9bd7 (download verified).

AI-assisted: yes.

@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 (1768c1821b) merged into main fae9e48.

  • With this PR: guest_fiopen passed 5 of 5.
  • Same tree with only FileAndHeap.cpp put back to main (this PR's tests kept): guest_fiopen aborted at Fiopen check failed at line 85, the new checkedStream == nullptr && Reported("fopen_s: bad argument") check, in 5 of 5 runs.
  • main fae9e48 alone (its own tests and source): guest_fiopen passed 3 of 3.
  • guest_fopen_errors skipped on every side here, so this run says nothing about it.

AI-assisted: yes (Claude Code)

@oneandonlydean
oneandonlydean merged commit 0b2c52e into boykopovar:main Nov 1, 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.

4 participants