Skip to content

fix(libkernel): change O_APPEND on a Windows file through fcntl - #9256

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
Dom3005:fix/windows-fcntl-append
Oct 24, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
Dom3005:fix/windows-fcntl-append

Conversation

@Dom3005

@Dom3005 Dom3005 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

What

On Windows, fcntl(F_SETFL) threw fcntl: changing O_APPEND on a file is not implemented whenever it set or cleared O_APPEND on a regular file. FreeBSD 11.4's kern_fcntl changes O_APPEND like any other F_SETFL flag.

The guest flag already decides whether write goes through WriteAtEnd. A descriptor opened with O_APPEND also carries the CRT's append flag, though (NativeOpen passes _O_APPEND to _open_osfhandle), so after the guest cleared O_APPEND, ::_write would still seek to the end. The CRT flag has to stay set while O_APPEND is on, because fdopen(fd, "w") on such a descriptor writes through the CRT and must keep appending.

When O_APPEND changes on a Windows file, F_SETFL now duplicates the descriptor's handle, opens it with or without _O_APPEND through _open_osfhandle, and _dup2s it onto the same number. This is the same pattern MoveAppendingHandle in libc uses for fdopen in append mode. On FreeBSD O_APPEND belongs to the open file, so this is done for every descriptor that shares the status (the descriptors dup and F_DUPFD create). The duplicated handle keeps the same file object, so file position and flock locks carry over.

The "Changing O_APPEND on a Windows file" clause is removed from the fcntl entry in docs/dev/debt/libkernel.md.

Tested

Windows 11 10.0.26200, WinLibs GCC 15.2.0 r7 UCRT, Release, BUILD_TESTING=ON, on main cecb540.

  • guest_kernel_fcntl now runs the same O_APPEND checks on every host, where it used to expect the throw on Windows: append, clear O_APPEND and write at offset 0, set it again through a F_DUPFD duplicate and check that the original descriptor appends, clear it through the duplicate and check that the original no longer appends, then read the file back (Ybcdeabc).
    • Without the fix, it fails with fcntl: changing O_APPEND on a file is not implemented, fd=3.
    • With the fix, it passes in 3 of 3 runs.
  • ctest -E '^agc_driver': 1230/1230 passed.
  • python tools/check_conventions.py: passed.
  • Not run on Linux. The Linux code is unchanged, and the test's new checks follow POSIX O_APPEND semantics, which the host's own F_SETFL provides.

Overlap: tools/pr_overlap.py shows #8544 and #9109 editing the same paragraph of docs/dev/debt/libkernel.md (#8544 changes the pipe F_GETFL part of the fcntl entry), so whichever merges second needs a one-line rebase. #6044, #6977 and #8520 share files, but their diffs don't touch O_APPEND.

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

@github-actions

github-actions Bot commented Oct 21, 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 10319 docs/dev/debt/libkernel.md L238-239

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 4630 docs/dev/debt/libkernel.md
PR 5021 docs/dev/debt/libkernel.md
PR 6044 core/libs/tests/GuestKernelFcntl.cpp
PR 6149 docs/dev/debt/libkernel.md
PR 6740 docs/dev/debt/libkernel.md
PR 6830 docs/dev/debt/libkernel.md
PR 6936 docs/dev/debt/libkernel.md
PR 7049 docs/dev/debt/libkernel.md
PR 7088 docs/dev/debt/libkernel.md
PR 7105 docs/dev/debt/libkernel.md
PR 7121 core/libs/prx/libkernel/File/src/DescriptorControl.cpp
docs/dev/debt/libkernel.md
PR 7149 docs/dev/debt/libkernel.md
PR 7509 docs/dev/debt/libkernel.md
PR 7511 docs/dev/debt/libkernel.md
PR 7565 docs/dev/debt/libkernel.md
PR 7653 docs/dev/debt/libkernel.md
PR 8010 docs/dev/debt/libkernel.md
PR 8520 core/libs/prx/libkernel/File/src/DescriptorControl.cpp
docs/dev/debt/libkernel.md
PR 8553 docs/dev/debt/libkernel.md
PR 9019 core/libs/prx/libkernel/File/src/DescriptorControl.cpp
PR 9074 docs/dev/debt/libkernel.md
PR 9123 docs/dev/debt/libkernel.md
PR 9133 docs/dev/debt/libkernel.md
PR 9156 docs/dev/debt/libkernel.md
PR 9160 docs/dev/debt/libkernel.md
PR 9199 docs/dev/debt/libkernel.md
PR 9284 docs/dev/debt/libkernel.md
PR 9293 docs/dev/debt/libkernel.md
PR 9297 docs/dev/debt/libkernel.md
PR 9302 docs/dev/debt/libkernel.md
PR 9557 core/libs/prx/libkernel/File/src/DescriptorControl.cpp
docs/dev/debt/libkernel.md
PR 9561 core/libs/prx/libkernel/File/src/DescriptorControl.cpp
core/libs/tests/GuestKernelFcntl.cpp
docs/dev/debt/libkernel.md
PR 9566 core/libs/prx/libkernel/File/src/DescriptorControl.cpp
docs/dev/debt/libkernel.md
PR 9575 docs/dev/debt/libkernel.md
PR 9669 docs/dev/debt/libkernel.md
PR 9693 docs/dev/debt/libkernel.md

@cadenasynyster

Copy link
Copy Markdown
Contributor

Independent Windows run of this PR (head da61bf7).

Environment: Windows 11 10.0.26300, Ryzen 7 9800X3D, WinLibs GCC 15.2.0 r7 (MinGW), Release, BUILD_TESTING=ON.

Results:

  • PR head: guest_kernel_fcntl passes 3/3 (--repeat until-fail:3).
  • Baseline: I kept the PR's test and replaced only core/libs/prx/libkernel/File/src/DescriptorControl.cpp with the merge-base version (cecb540). The test fails with fcntl: changing O_APPEND on a file is not implemented, fd=3, the same as the description. I restored the PR's file afterwards.

Note for anyone repeating the baseline: that failing run aborts after writing abcde and ab, so kernel_fcntl_probe.bin stays behind in build/core/libs/prx/libkernel with 7 bytes. FileFlagsRoundTrip opens it without O_TRUNC, so every later run, on main as well, fails earlier at the FIONREAD check (readable == 3, line 241) until the file is deleted. My first runs of this PR hit that. With the file deleted, main (0c6a5bd) and this PR both pass. This is in the existing test, not in this PR's change.

Not run: Linux/macOS, fdopen(fd, "w") on a descriptor whose O_APPEND was changed (the description mentions it, but the test does not cover it), the full repository CTest, and any title.

AI-assisted: yes

@Dom3005

Dom3005 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the independent run and the baseline. Before this PR, Windows only expected the throw at that point, so the test could not stop partway and leave the probe file behind. Now it can, so ddcb41a removes kernel_fcntl_probe.bin at the start of FileFlagsRoundTrip. I checked it by writing a 7-byte abcdeab probe file into build/core/libs/prx/libkernel: the previous binary fails, and the new one passes --repeat until-fail:3 and removes the file afterwards.

The fdopen(fd, "w") case is still covered only by the reasoning in the description. It goes through libc, so this kernel test does not exercise it.

@DavidSamir

Copy link
Copy Markdown
Contributor

Hi @Dom3005, a merge-order note for two of mine:

AI-assisted: yes

@Dom3005

Dom3005 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Works for me: no dependency either way, and whichever merges second keeps both test changes and every debt sentence in the fcntl row.

AI-assisted: yes

@oneandonlydean oneandonlydean self-assigned this Oct 22, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 23, 2026
Dom3005 and others added 2 commits October 24, 2026 02:28
fcntl(F_SETFL) threw when it changed O_APPEND on a Windows file. The
guest flag already chooses WriteAtEnd, but a descriptor opened with
O_APPEND also carries the CRT's append flag, so _write kept appending
after the guest cleared it. F_SETFL now replaces the CRT descriptor of
every descriptor sharing the open file's status with a duplicate handle
opened with or without _O_APPEND, as libc fdopen does for append mode.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Dom3005
Dom3005 force-pushed the fix/windows-fcntl-append branch from ddcb41a to 69cdebd Compare October 24, 2026 00:28
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 24, 2026
@oneandonlydean
oneandonlydean merged commit 6887189 into boykopovar:main Oct 24, 2026
10 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