Skip to content

fix(libkernel): preserve directory identity across Windows renames - #6228

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Ondra97:codex/fix-windows-directory-renames
Oct 18, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Ondra97:codex/fix-windows-directory-renames

Conversation

@Ondra97

@Ondra97 Ondra97 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

I reproduced a wrong-file read on main d92a8dad390af1807cc91622b924252141816a5a: open a directory, duplicate its descriptor, close the original descriptor, rename the directory, then create a replacement at its old path. openat(dirfd, "marker", ...) reads the replacement's content and getdents(dirfd, ...) lists the replacement's entries. Windows directory descriptors held a NUL descriptor and the original pathname. POSIX openat resolves relative to the directory associated with the descriptor.

Directory descriptors now hold a real Windows directory handle with read/write/delete sharing. The CRT owns and duplicates that handle through the existing descriptor lifecycle, following the _open_osfhandle ownership contract. Cursor snapshots remain shared across aliases. Metadata uses the handle, and path-based operations and snapshot reloads query its current name through the existing native-path helper. Directory classification no longer depends on a pathname lookup succeeding.

The shared path helper normalizes Windows DOS/UNC prefixes for filesystem consumers and preserves native lookup errors. fchdir also verifies an equivalent root ancestor when Windows returns different pathname casing, then retains the virtual root's spelling for subsequent relative paths and getcwd.

Rebased onto main 11acfdd84e801a2eff6e76555df80a2fd1a68f95. The only manual conflict was the adjacent technical-debt entries: main's sceKernelWriteThrottlingStatus null-pointer EFAULT description is preserved together with the directory-identity changes. All 10 files retain the original 184 added and 33 removed lines; the source and regression-test changes are unchanged.

Tested

Validation at head 739e451332fbe284b5395993f9189354774be326 on main 11acfdd84e801a2eff6e76555df80a2fd1a68f95.

Windows (native host): Windows 11 Pro 10.0.26300, AMD Ryzen 9 7900X, 32 GB RAM, NVIDIA GeForce RTX 4070 Ti SUPER; WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release, bundled dependencies, ccache. Graphics: not exercised.

  • Full default application, library and test-executable build passed, including NID patching (cmake --build build_tools/b6228-739e --parallel 4).
  • 50/50 selected filesystem/runtime CTest groups passed, zero failures/skips; slowest 2.399 s.

Linux (VirtualBox): Ubuntu 24.04.5 LTS, kernel 7.0.0-38-generic, VirtualBox 7.2.20 r175154 on the Windows host above, 4 vCPU, 8 GiB VM RAM; GCC 13.3.0, Ninja, Release, bundled dependencies, ext4. Graphics: not exercised.

  • Full application/library targets (relinker libs) and every selected test executable built successfully with two build jobs, including compilation of the graphics components.
  • 42/42 selected filesystem/runtime CTest groups passed serially, zero failures/skips; slowest 0.134 s.
  • All 16,074 source/submodule files matched the exact Git blobs before building and after testing. The previous snapshot was verified before updating it; changed source files received current timestamps for the incremental build.

The same filter was used on both platforms; platform-specific tests account for the counts. Windows used --parallel 2; Linux (VirtualBox) used the serial command below:

ctest --test-dir build --no-tests=error --timeout 30 --output-on-failure --parallel 1 -R '^(guest_kernel_sys|guest_file_vector|guest_directory.*|guest_fdopendir|guest_working_directory|guest_dup|guest_kernel_fcntl|guest_fstatfs|guest_fstat_fault|guest_file_timestamps|guest_filesystem|guest_posix_filesystem|guest_kernel_rename|guest_aio|guest_sendfile|guest_shm_open|guest_realpath|guest_path_case|guest_file_lock_.*|guest_savedata_.*|download_data_mount_.*|patched_libraries|relinked_module.*|guest_(kernel_)?app_archive_.*|guest_append_writes|guest_positioned_close|guest_truncate|guest_fopen_errors|guest_fopen_long_host_path|guest_record_lock_owners)$'

The selection covers the directory identity regression, alias lifetime/cursor sharing, enumeration/rewind, openat/mkdirat/unlinkat, fchdir, directory read errors, metadata after removal, vector and append I/O, close/truncate, and application archives. Conventions, whitespace and test-registration checks passed.

Static review covered the original patch, upstream delta ca82d7d882b0ac17d955f6e107fe2eb0cae244af..11acfdd84e801a2eff6e76555df80a2fd1a68f95, build/test inputs, and the complete rebased patch. Independent line comparison confirmed preservation of both the original changes and the upstream changes in the overlapping sources.

Overlap review: #6302, #6525 and #6897 are complementary changes with textual conflicts; there is no functional dependency. A future combination must retain both the directory check and transfer lock in NativeFtruncate, both independent test groups, and #6897's invalid-descriptor guard without restoring the pathname-based stat fallback. Combined runtime validation was not performed.

Linux graphics/Vulkan tests: NOT RUN — intentionally excluded from VirtualBox validation. Native/bare-metal Linux was not tested. Native Linux GPU validation: not tested. Retail titles, SMB/UNC execution and removed-directory enumeration were not tested. Path-based *at operations and enumeration reloads remain non-atomic with a concurrent rename, as recorded in technical debt. The Windows ancestor-rename restriction reported in the PR cross-check remains unchanged. No title compatibility or performance claim.

Windows symbolic-link success branches in guest_kernel_sys report links unavailable; those branches are not claimed as covered.

Original negative controls before rebase

At main d92a8dad390af1807cc91622b924252141816a5a, the public API probe read the replacement marker and listed the replacement entries after rename. With original fix 64eae5ab55f7a9fa0fdd4f3d777de009dd48a370, it retained the original marker and entries. The expanded guest_kernel_sys failed on main at the replacement-content assertion. Restoring only main's General.cpp also made the fixture fail at fchdir with guest errno 45. These are historical controls, separate from the current-head validation above.

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 (filesystem runtime)
  • AI-assisted: yes (Codex)

@github-actions

github-actions Bot commented Oct 15, 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 6525 core/libs/tests/GuestKernelSys.cpp L112-113
PR 6897 core/libs/prx/libkernel/File/src/NativeStat.cpp L46
PR 7332 docs/dev/debt/libkernel.md L125

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/prx/libc/src/General.cpp
core/libs/prx/libkernel/File/src/NativeStat.cpp
core/libs/prx/libkernel/File/src/Open.cpp
core/libs/prx/libkernel/File/src/Stdio.cpp
docs/dev/debt/libkernel.md
PR 6044 core/libs/tests/GuestKernelSys.cpp
PR 6226 core/libs/prx/libc/src/General.cpp
PR 6446 docs/dev/debt/libkernel.md
PR 6510 docs/dev/debt/libkernel.md
PR 6519 docs/dev/debt/libkernel.md
PR 6534 docs/dev/debt/libkernel.md
PR 6879 core/libs/prx/libc/src/General.cpp
core/libs/prx/libkernel/File/src/NativeStat.cpp
core/libs/prx/libkernel/File/src/Stdio.cpp
PR 6908 docs/dev/debt/libkernel.md
PR 6936 core/libs/prx/libkernel/File/src/Open.cpp
core/libs/prx/libkernel/File/src/Stdio.cpp
docs/dev/debt/libkernel.md
PR 6977 core/libs/prx/libkernel/File/src/Stdio.cpp
docs/dev/debt/libkernel.md
PR 7039 core/libs/tests/GuestKernelSys.cpp
docs/dev/debt/libkernel.md
PR 7049 docs/dev/debt/libkernel.md
PR 7088 core/libs/prx/libkernel/File/include/File.hpp
docs/dev/debt/libkernel.md
PR 7105 docs/dev/debt/libkernel.md
PR 7149 docs/dev/debt/libkernel.md
PR 7192 docs/dev/debt/libkernel.md
PR 7201 core/libs/prx/libc/src/General.cpp
core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp
core/libs/prx/libkernel/File/src/NativeStat.cpp
core/libs/prx/libkernel/File/src/Open.cpp
core/libs/prx/libkernel/File/src/Stdio.cpp
PR 7299 core/libs/prx/libkernel/File/src/Open.cpp
core/libs/prx/libkernel/File/src/Stdio.cpp
docs/dev/debt/libkernel.md
PR 7447 core/libs/prx/libkernel/File/src/Stdio.cpp
docs/dev/debt/libkernel.md
PR 7509 docs/dev/debt/libkernel.md
PR 7541 docs/dev/debt/libkernel.md
PR 7543 docs/dev/debt/libkernel.md
PR 7554 docs/dev/debt/libkernel.md

@screamm

screamm commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Cross-check on Windows: #6228 head 64eae5ab5 on main d92a8dad3 (fast-forward, no conflict).

Host: Windows 11 Pro 26220, Ryzen 7 5700X, RTX 4070 (driver 596.21), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release.

  • Build: full build and libs both succeed, with no new warnings. The one warning in a touched file, General.cpp:426 (u8path is deprecated), is main's unchanged line 410.

  • Tests, --repeat until-fail:3: guest_kernel_sys and 48 more file-system tests pass 3/3, 147 of 147 runs:

    • the guest_directory* and guest_file_lock_* tests;
    • guest_fdopendir, guest_working_directory, guest_realpath, guest_path_case, guest_filesystem, guest_posix_filesystem and guest_kernel_rename;
    • guest_dup, guest_kernel_fcntl, guest_fstatfs, guest_aio, guest_sendfile and guest_shm_open;
    • the guest_savedata_* and download_data_mount_* tests.

    guest_kernel_sys prints directory marker after rename/replacement: o, expected o. Symbolic links are unavailable on this host (Developer Mode off), so those branches did not run.

  • Negative control: the same test binary run with main's libraries aborts at the replacement-content check:

    directory marker after rename/replacement: r, expected o
    Kernel sys check failed at line 171 (guest errno 2)
    FATAL: abort() (SIGABRT)
    
  • Suite, ctest -j8 --timeout 120 -E '^agc_driver_|^video_out_flip': 814 of 814 pass, including the 2 skipped. check_conventions.py --base origin/main and git diff --check are clean.

Titles. Catalog releases, relinked once with main's relinker, so only the libraries differ. Each ran 60 s with the display on, app0/eboot.bin staged and HOME unset.

Title main this PR
Prospero Explorer (PPSA99109) running at 60 s, 448 frames running at 60 s, 506 frames
ccleste (PPSA59322) running at 60 s, 5507 frames running at 60 s, 5688 frames
PS5MC (PPSA99420), 2 pairs in alternating order running at 60 s, 414 and 583 frames running at 60 s, 740 and 427 frames

Prospero Explorer opens every mount as a directory descriptor at startup, with the same lines from both builds:

[src/Open.cpp:215 sceKernelOpen] path=/app0 flags=0x20000 nativeFlags=0x8000 mode=00
[src/Open.cpp:215 sceKernelOpen] path=/savedata0 flags=0x20000 nativeFlags=0x8000 mode=00
[src/Open.cpp:215 sceKernelOpen] path=/ flags=0x20000 nativeFlags=0x8000 mode=00

The logged open/stat lines are the same for each title on both builds, and no error appears on this PR alone:

  • ccleste opens its assets through /app0/assets/hui/../celeste/....
  • In PS5MC, kodi.log has the same 14 error lines and 59 Found addon lines, and both builds log the same 222 SQLite version-probe cannot open file lines.
  • No run has a stop line. Frame counts follow host load: other titles were rendering on the same GPU.

One behaviour change, found with a small guest-API probe (sceKernelOpen with O_DIRECTORY, then rename and rmdir):

Case main this PR
open A/B as a directory, then rename A to A2 rename returns 0, but getdents on the descriptor lists only . .. rename returns 0x80020005 (EIO); getdents lists . .. f
open the regular file H/I/f, then rename H 0x80020005 0x80020005
rmdir an open directory and mkdir it again; rename an open directory; rename its child or sibling all succeed all succeed

With the real handle, Windows refuses to rename an ancestor of an open directory, as it already does for an open file. FreeBSD would allow the rename. This could be one sentence in the Windows directory descriptors debt bullet.

Verdict: confirmed. The stale-path read is fixed, the test fails on main and passes here, and there is no regression in the tests, the suite or the three file-I/O titles. The only new behaviour is that renaming an ancestor of an open directory descriptor now fails with EIO, as it already does for files.

AI-assisted (Claude Code).

This was referenced Oct 15, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 15, 2026
@Ondra97
Ondra97 force-pushed the codex/fix-windows-directory-renames branch from 64eae5a to 29fc849 Compare October 15, 2026 22:06
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 15, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 16, 2026
@Ondra97
Ondra97 force-pushed the codex/fix-windows-directory-renames branch from 29fc849 to 02371ec Compare October 17, 2026 10:14
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@screamm

screamm commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Rechecked on head 02371ec4a (rebased), scratch-merged onto main c9e6c45de (clean, no conflict).

Host: Windows 11 Pro 26220, Ryzen 7 5700X, RTX 4070 (driver 596.21), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release.

What changed since my run on 64eae5ab5. Comparing the PR's diff against its base (d92a8dad3..64eae5ab5 vs ca82d7d88..02371ec4a): the same 10 files, 184 insertions and 33 deletions (was 183 and 33). The source follows the rebase (Classify keeps the new IsArchivedFile branch, the ftruncate/TransferIovecs context), and the test gained one line, Require(sceKernelLseek(target, 0, 0) == 0);, before the read-error assertions. The two debt bullets the PR edits are unchanged. I did not see any new comment or review. My earlier remark is still open: renaming an ancestor of an open directory descriptor fails with EIO on Windows (probe below), and the Windows directory descriptors bullet does not mention it.

  • Build: full build and libs succeed (the old MinGW alignas(65536) failure is gone on current main). The one warning in a touched file, u8path deprecated in General.cpp, exists on main too.
  • Tests, --repeat until-fail:3: 59 tests (guest_kernel_sys and the filesystem selection from the PR body, plus the other file-system tests): 177 of 177 runs pass. guest_kernel_sys prints directory marker after rename/replacement: o, expected o; symbolic links are unavailable on this host (Developer Mode off), so those branches did not run.
  • Negative control: with the eight source files taken from main and the PR's test kept, guest_kernel_sys aborts at the replacement-content check; with the PR's sources it passes again.
    directory marker after rename/replacement: r, expected o
    Kernel sys check failed at line 171 (guest errno 2)
    FATAL: abort() (SIGABRT)
    
  • Suite, ctest -j8 --timeout 120 -E '^agc_driver_|^video_out_flip': 883 tests; 13 timed out in the first pass while other builds were running (the conventions_function_bodies* shards, check_main_sync, configure_seed, debt_split, hardware_oracle_compare, conventions_bom_shebang, tls_function_coverage*, guest_file_lock_process). All pass on serial reruns (conventions_function_bodies, at its 30 s limit, on the second rerun); 2 skipped as before. check_conventions.py --base origin/main and git diff origin/main --check are clean.

Probe with the guest API (sceKernelOpen with O_DIRECTORY, then rename and rmdir), same result as on 64eae5ab5:

Case main this PR
open A/B as a directory, rename A to A2 rename returns 0, getdents on the descriptor lists only . .. rename returns 0x80020005 (EIO), A stays, getdents lists . .. f
open the regular file H/I/f, rename H 0x80020005 0x80020005
rmdir an open directory and mkdir it again; remove a tree around an open subdirectory all succeed all succeed

Overlap.

Titles. Catalog releases relinked once, so only the libraries differ; "main" is the same tree with only this PR's source files reverted. Each ran 60 s with the display on; other builds were running on the host, so frame counts are only indicative.

I first looked for titles that rename a directory. sceKernelRename has call sites in the executables of PPSA99002, PPSA99082, PPSA99109, PPSA99110 and PPSA99600, fchdir only in PPSA99764, and libc rename in many. I logged rename and directory-open calls with a local diagnostic (removed afterwards) on 9 titles (RetroArch PPSA99169 60 s, the others 45 s). The only renames are log files: PPSA99600 renames /download0/debug.log.tmp to debug.log 188 times, PPSA99002 and PPSA99008 rotate their logs to .prev names. No title renamed a directory, so none reaches the path this PR fixes. Titles do open and list directories with O_DIRECTORY through the new handle (RetroArch, Prospero Explorer, OpenNOW).

Title main this PR
RetroArch PPSA99169, 2 pairs running at 60 s, 3152 and 2124 frames running at 60 s, 2517 and 4505 frames
PPSA99600 (renames its debug log) running at 60 s, 1067 frames running at 60 s, 1113 frames
Prospero Explorer PPSA99109 running at 60 s, 413 frames running at 60 s, 535 frames

No run has a stop line. For RetroArch the logs have the same 971 O_DIRECTORY opens and 1047 normalized file-system lines on both sides, with the same 2 error-like lines; PPSA99600 and Prospero Explorer have the same file-system lines on both sides and no error-like line.

Not checked: Linux, symbolic-link branches, UNC paths, removed-directory enumeration, GPU tests.

Verdict: confirmed again. The test fails on main's sources and passes here, nothing regresses in the suite or the four titles, and no local title exercises the fixed path.

Written with Claude Code assistance (validation by a subagent, reviewed in this session).

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@Ondra97
Ondra97 force-pushed the codex/fix-windows-directory-renames branch from 02371ec to 739e451 Compare October 17, 2026 21:42
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@adriadam10 adriadam10 self-assigned this Oct 18, 2026
@adriadam10
adriadam10 merged commit ccf0f58 into boykopovar:main Oct 18, 2026
16 checks passed
@Ondra97
Ondra97 deleted the codex/fix-windows-directory-renames branch October 20, 2026 14:49
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.

3 participants