Skip to content

fix(libc): return native fopen errors - #4822

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:fix/fopen-native-open-errors-4814
Oct 14, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:fix/fopen-native-open-errors-4814

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

fopen_nid_postfix now returns nullptr and preserves the native errno whenever the host cannot open the requested stream. Previously, failures other than ENOENT or exclusive-create EEXIST could turn into an uncaught exception after an extension lookup. This makes the Windows access-denied result for opening an existing directory follow fopen's failure contract.

Adds a regression that opens a directory in a mode that fails on each host (rb on Windows, wb on POSIX) and checks that the native error is retained. This does not claim that Windows directory streams behave like FreeBSD directory streams; the limitation is recorded in docs/dev/debt/libc.md.

Tested

  • Debian 12 x86-64, GCC 12.2, Release, BUILD_TESTING=ON: file_streams, guest_fiopen, guest_standard_streams, guest_directory, and guest_filesystem pass (5/5).
  • tools/check_conventions.py --base upstream/main and git diff --check upstream/main...HEAD pass.
  • No Windows build or title run was available locally; the Windows-specific rb directory case is included for hosted Windows CI.

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: PPSA99902 Sonic Unleashed Recompiled (PPSA99902): initial gameplay on fork and remaining work #4443
  • AI-assisted: yes

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Additional validation: MinGW-w64 GCC 12 -Wall -Wextra -Werror -fsyntax-only compilation passes for both FileAndHeap.cpp and the updated FileStreams.cpp. This is a Windows-target syntax check, not a runtime test; the hosted workflow currently builds only the relinker target.

@Amoenus

Amoenus commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Windows runtime validation of exact head 77ede919c460fbe1acbfe2bf4d927fb3db0a6416: all five groups ×20 pass (100 executions, no skips) — file_streams, guest_fiopen, guest_standard_streams, guest_directory, guest_filesystem. Observed individual durations: 0.01–0.35s. Windows 11 Enterprise 26200, WinLibs GCC15.2.0r7, Ninja Release / testing ON, bundled dependencies. The five native targets, conventions against pinned main 43e3a70f8, and diff checks pass.

For a Windows failing-main control, I kept your regression fixture unchanged and rebuilt only FileAndHeap.cpp from main 43e3a70f8143f014f621c48abbdb137fffd7346e. file_streams aborts with fopen_nid_postfix: open failed ... Permission denied, CTest exit 0xC0000409. Restoring your exact implementation gives the repeated passes above. The source and runtime are restored to your head; tracked source is clean. This verifies the Windows directory rb failure path retaining EACCES alongside existing positive stream controls.

ctest --test-dir <build> --parallel 1 --repeat until-fail:20 --timeout 30 --output-on-failure -R "^(file_streams|guest_fiopen|guest_standard_streams|guest_directory|guest_filesystem)$"

Full Windows build and patched system libraries of the same unchanged head now pass, along with conventions and test registration. Full NVIDIA suite: 1011 passed / 0 failed / 8 skipped of 1019, maximum observed test 22.808s (ctest --parallel 2 --timeout 30). RTX5080 / driver610.62; AnyPS5 shader cache disabled, driver-cache coldness unverified. Skips: agc_driver_stencil_ref_export, guest_video_out_high_frame_rate, video_out_flip_lifetime, video_out_flip_reopen, video_out_flip_controls, video_out_flip_overlay, video_out_flip_unopened, guest_video_out_wqhd_detection_no_vulkan. Conditional title comparison now completed: Sonic Unleashed Recompiled, PPSA99902 / 01.003.000, same fixed f609 conversion and frozen 6cc runtime. Fresh control aborts at the existing Inspire directory / fopen permission-denied exception after 10.126s. Replacing only libc.prx with the integration build of your exact FileAndHeap.cpp (retaining the game's private prerequisite exports) changes the observed abort to poll: descriptor 3 is not a socket / descriptor5 on another thread after 9.981s. All other123 runtime-library hashes and executable hash match. Both runs exit0xC0000409 with no frames or timeout; originals, runtime, converted guest and user data are restored/hash-verified. This removes the observed fopen exception from the replay; it does not establish rendering, gameplay or unchanged whole-main startup. The next poll guard is already tracked in #3521, with overlapping open #3644/#3649; a subsequent scalar observer confirms read pipes, and the #3644 kernel-only comparison now renders startup and reaches the existing private MSAA guard after789.240s, with originals/runtime/user data restored. This is still a conditional two-fix stack, not whole-main startup or gameplay. Title evidence and preserved-assets checks are maintained in #4814 and Game bring-up #4443. The template's Unblocks line can use PPSA99902 #4443 to link the required bring-up discussion, keeping #4814 as the issue reference. AI-assisted: Codex; author responsible.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Please add a docs/dev/debt/libc.md entry: on Windows fopen of a directory in a read mode fails with EACCES, where FreeBSD opens it. The description says this isn't claimed to match; the debt file should say it too, since that is the call PPSA99902 makes.

Unblocks should name the bring-up discussion (PPSA99902 #4443). Then this can go in.

@MarcZX10
MarcZX10 force-pushed the fix/fopen-native-open-errors-4814 branch from 77ede91 to d1a8c32 Compare October 13, 2026 03:12
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Addressed the review points: docs/dev/debt/libc.md now records that Windows fopen of a directory in a read mode fails with EACCES, while FreeBSD opens it; the PR checklist now links Unblocks: PPSA99902 #4443. I also rebased the PR onto current main. The updated head passes tools/check_conventions.py --base upstream/main and git diff --check; the previously reported Windows and Linux validation remains on the code unchanged by this follow-up.

@nicolasfalesy

Copy link
Copy Markdown
Contributor

Linux test (Arch, GCC 16.2.1, glibc, Release), this PR merged onto main 7e181fc together with #4927, #4903, #4877, #4822 and #4823 (no conflicts): file_streams passes. With this PR's source change reverted (test kept), file_streams fails on Linux, so the check guards the change on Linux too.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@MarcZX10
MarcZX10 force-pushed the fix/fopen-native-open-errors-4814 branch from d1a8c32 to ed7dfe1 Compare October 13, 2026 07:44
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Rebased again onto current main after the debt-file conflict returned; the updated debt entry retains the current unwind note and records the Windows directory-read EACCES behavior. The checklist still links PPSA99902 #4443, and the rebased patch passes git diff --check. Thanks for the Linux control run: it shows file_streams fails when this PR’s source change is reverted while its regression test is kept, and passes with the implementation included. That is useful Linux-side regression evidence; the Windows runtime and title results above remain attributed to their reported heads/stacks.

@MarcZX10
MarcZX10 force-pushed the fix/fopen-native-open-errors-4814 branch from ed7dfe1 to a475226 Compare October 13, 2026 07:51
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@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
#5033 docs/dev/debt/libc.md L47

@oneandonlydean
oneandonlydean merged commit 91e5f60 into boykopovar:main Oct 14, 2026
14 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