Skip to content

fix(libc): keep buffered input when a seek target is rejected - #15236

Open
AlexWhitehouse wants to merge 1 commit into
boykopovar:mainfrom
AlexWhitehouse:fix/libc-windows-failed-seek-keeps-buffer
Open

AlexWhitehouse wants to merge 1 commit into
boykopovar:mainfrom
AlexWhitehouse:fix/libc-windows-failed-seek-keeps-buffer

Conversation

@AlexWhitehouse

Copy link
Copy Markdown
Contributor

What

fseeko (core/libs/prx/libc/src/FileAndHeap.cpp; fseek and fsetpos call it) handed every seek straight to the host C library. When the target was invalid, both hosts changed the stream before failing:

  • Windows: UCRT's _fseeki64 drops the read buffer before its lseek to a negative target fails with EINVAL, so the stream continued at the descriptor's read-ahead offset. After reading 4 of 8 buffered bytes, a rejected fsetpos to -1 left ftello at 8 and the next read at end of file. docs/dev/debt/libc.md recorded this.
  • Linux: glibc keeps the buffer, but drops pending ungetc data on the failure path and returns EINVAL instead of EOVERFLOW for a target past OFF_MAX.

FreeBSD 11.4's _fseeko checks the target before touching the stream, and returns -1 with the buffer, pushback and EOF state unchanged when the check fails.

This PR adds a ResolveSeekTarget helper that does the same before either host library sees the call:

  • SEEK_CUR: the base is the stream's ftello position. A failure there is returned as is, and a negative base is ESPIPE.
  • SEEK_END: the base is the file size from fstat, but only for a regular file whose stream is not in write mode (UCRT's write flag on Windows, __fwriting on glibc). Buffered output would make the logical end larger, and finding it would need a flush. On UCRT, a flush on a read stream discards the buffer, which is the bug.
  • Checks: a target past OFF_MAX is EOVERFLOW (84); a negative target is EINVAL (22). A valid target is passed on as SEEK_SET.

The Windows pipe check and the closed-stream and origin checks still come first. The success path is unchanged.

The debt entry now leads with what remains:

  • SEEK_END on a stream in write mode, on a non-regular file, or on macOS still goes to the host library;
  • a host seek that fails on a valid target is not handled.

Tested

  • file_position (core/libs/tests/FilePosition.cpp) gains two cases. Every existing assertion is kept.
    • Rejected seeks: read 4 bytes of an 8-byte file, then try fsetpos to -1, fseeko(-1, SEEK_SET), fseek(-5, SEEK_CUR), fseeko(-9, SEEK_END) and fseeko(INT64_MAX, SEEK_CUR). Each must fail with EINVAL (or EOVERFLOW for the last), leave ftello at 4, and leave EOF and error unchanged. A pushed-back byte must survive a rejected SEEK_CUR and a rejected SEEK_SET, and the next reads must return the original bytes.
    • SEEK_END with pending output: still goes to the host and lands where expected.
    • The test has no platform conditionals.
  • Linux (Ubuntu 26.04, GCC 15):
    • with the test on main's FileAndHeap.cpp, file_position fails at line 70 (EINVAL instead of EOVERFLOW);
    • with this PR it passes;
    • the libc stream group (file_position, file_streams, file_stream_status, guest_standard_streams*, guest_stdio_variadic, guest_wide_stream_scan, guest_stream_purge, guest_fopen_errors) passes 12/12.
  • Windows 11 (MinGW GCC 15.2.0):
    • with only FileAndHeap.cpp from main (9090972), file_position fails at line 71: ftello is 8, not 4, after the rejected fsetpos;
    • with this PR (4295667) it passes;
    • the same group plus guest_fopen_long_host_path and guest_stream_buffer_host_write passes 13/13, with guest_fopen_errors skipped for lack of symlink rights;
    • no compiler warnings.
  • Mutation check: removing the negative-target rejection makes file_position fail at line 86 on Linux, because glibc drops the pushed-back byte. On Windows, removing the SEEK_SET check from the first, Windows-only version of the change failed at the ftello check after the rejected fsetpos.
  • python3 tools/check_conventions.py --base origin/main: all conventions checks passed.
  • python3 tools/pr_overlap.py: no confirmed overlap. It could not compare 27 PRs that already conflict with main, so their changes were checked by hand: the FileAndHeap.cpp hunks of the eight that edit it, and the libc.md changes of all 27.

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

🤖 Generated with Claude Code

Check fseeko targets before the host C library sees them, as FreeBSD
11.4's _fseeko does: a negative SEEK_SET, SEEK_CUR or regular-file
SEEK_END target fails with EINVAL and one past OFF_MAX with EOVERFLOW,
leaving the buffer, pushback and EOF state alone. UCRT dropped the read
buffer before rejecting such a target, so the stream continued at the
descriptor's read-ahead offset; glibc dropped pending ungetc data and
returned EINVAL for an overflowing target.

SEEK_END on a stream in write mode or on a nonregular file still goes
to the host library, since finding its end would need a flush.

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

Kikoarl commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Independent Linux tester run of head 429566793d25 merged onto current main 5cb015caaa (clean merge).

  • Environment: Linux 7.2.11 x86_64, GCC 16.2.1, Ninja, Release, BUILD_TESTING=ON in RAM-disk (tmpfs).
  • Tests: file_position_tests passes 6 of 6 (0.00 s). Related stream suites (guest_file_streams, guest_append_writes) pass clean.
  • Differential Red/Green:
    • On main 5cb015caaa with this PR's test applied: file_position_tests fails with SIGABRT asserting errno == EOVERFLOW (glibc returns EINVAL 22 on overflow past OFF_MAX).
    • On this PR branch: all 6 test cases pass cleanly without stream corruption or ungetc buffer loss.
  • Conventions: python3 tools/check_conventions.py --base origin/main --head 429566793d25 reports clean (0 errors, 0 warnings). git diff --check clean. Zero unauthorized code comments; debt documented in docs/dev/debt/libc.md.

AI-assisted: yes

@bunbeans2-arch

Copy link
Copy Markdown
Contributor

A Windows run with GCC 16: Windows 10.0.22631 (Windows 11), i9-9900K, MSYS2 UCRT64 GCC 16.1.0 (Rev5), Release, this PR (429566793d) merged into main d1ae628bc2.

file_position with this PR's new rejected-seek loop:

  • ❌ Before: same tree with only FileAndHeap.cpp put back to main (rebuilt). It failed 5 of 5 runs with File position check failed at line 71 (abort, exit 0xc0000409). Line 71 is Require(ftello_nid_postfix(reader) == 4); inside the loop over rejected fsetpos / fseeko / fseek calls, so after a rejected seek the old code no longer reports the read position 4.
  • ✅ After: with this PR it passed 5 of 5.
  • main alone (its own file_position) passed 3 of 3.

The log doesn't say which of the five seek forms hit line 71 first, and the test stops there 👍

AI-assisted: yes (Claude Code)

@AlexWhitehouse

Copy link
Copy Markdown
Contributor Author

Thanks for the run. On UCRT the first form, the rejected fsetpos to -1, is the one that stops at line 71: _fseeki64 drops the read buffer before its lseek fails, so ftello reports the descriptor offset 8 instead of 4, which matches the Windows result in the PR description.

@JJenkx

JJenkx commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Windows check of head 429566793d, merged onto main b11c7bf84f (cbe4f21993, clean merge).

Setup: Windows 11 Enterprise 22621, WinLibs GCC 15.2 UCRT, Release. The hosted Windows check does not run this test on pull requests.

AI-assisted: yes.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants