Skip to content

fix(kernel): open game files through the extended-length path on Windows when the path is MAX_PATH or longer - #9947

Open
klosixgaming-arch wants to merge 1 commit into
boykopovar:mainfrom
klosixgaming-arch:fix/kernel-open-long-path
Open

klosixgaming-arch wants to merge 1 commit into
boykopovar:mainfrom
klosixgaming-arch:fix/kernel-open-long-path

Conversation

@klosixgaming-arch

@klosixgaming-arch klosixgaming-arch commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What

On Windows, CreateFileW and the streams built on it refuse an ordinary path of 260 characters (MAX_PATH) or more. #12560 added the shared NativeVerbatimPath_nid_no_patch and already uses it in open and the directory open. This PR builds on that helper for the places that still used the plain path:

  • the APR reader: the file stream, the size lookup (the FindFirstFileExW listing of the parent and the is_regular_file fallback) and the end-of-file check
  • directory descriptors: Load lists a directory through FindFirstFileExW with a verbatim search pattern on Windows and stats the directory, its parent and each entry through the verbatim path. InsideApp0 opens the /app0 root that way, and OpenDirectoryDescriptor checks for a directory that way.
  • sceKernelOpen: the O_NOFOLLOW symlink check and the directory fallback after a failed open

Samurai Maiden (PPSA04132, 01.060.000) has asset bundles with long names under Media/StreamingAssets/aa/PS5/. Under a 116-character runtime folder, every open whose resolved path reached 260 characters failed, and the title stayed dimmed on its logo screen.

Tested

Windows 11 (10.0.26200), WinLibs GCC 15.2.0, this head on main 4f41f4c375:

  • pass: the nine guest_apr_read_* cases including the new guest_apr_read_longeof; guest_filesystem; guest_windows_directory_link_types (with LongNoFollow); guest_directory_entries, _buffer, _rewind and _close; agc_shader_disk_cache_long_paths
  • not conclusive: long_paths failed while removing its temporary folder (WinError 32, file in use). Antivirus on this machine was blocking ctest.exe at the time, so I couldn't rerun it, guest_directory, guest_filesystem_error or agc_pipeline_journal_long_paths.
  • Open.cpp, Apr.cpp and DirectoryDescriptor.cpp compile on Linux (g++ -fsyntax-only). No full Linux build.
  • Samurai Maiden was tested on the earlier head of this PR, which reached the Load Game menu and a story scene. Not retested on this head.

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; no silent stubs
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: PPSA04132
  • GPUs tested: n/a
  • AI-assisted: yes. Written with AI assistance and reviewed by me.

@github-actions github-actions Bot added the unblocks title Names a title ID on the Unblocks line label Oct 22, 2026
@MarcZX10

Copy link
Copy Markdown
Contributor

I reviewed head 7925fd2dd74be8ebd577a3647cee570b5e6c7ac8; diff-check and conventions pass, and the change preserves existing extended/device paths while applying the \\?\ or \\?\UNC\ prefix after absolute normalization. The reported title run is useful end-to-end evidence for both Open.cpp and the APR reader. Could you add a small Windows-only unit test for the path helper’s drive, UNC, already-extended and device-path cases? Those transformations can be checked without creating a 260-character directory, and would cover the new shared helper directly. Product build jobs were skipped; I did not run the C++ build locally.

@github-actions

github-actions Bot commented Oct 22, 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 12139 core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp L206-209
PR 15040 docs/dev/debt/libkernel.md L165-166

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 4563 core/libs/prx/libkernel/CMakeLists.txt
PR 4630 docs/dev/debt/libkernel.md
PR 5021 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/Open.cpp
docs/dev/debt/libkernel.md
PR 6740 docs/dev/debt/libkernel.md
PR 6936 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/Open.cpp
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/Open.cpp
docs/dev/debt/libkernel.md
PR 7135 docs/dev/debt/libkernel.md
PR 7149 docs/dev/debt/libkernel.md
PR 7192 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 7447 docs/dev/debt/libkernel.md
PR 7464 docs/dev/debt/libkernel.md
PR 7804 docs/dev/debt/libkernel.md
PR 9019 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/Open.cpp
PR 9156 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9199 docs/dev/debt/libkernel.md
PR 9293 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9297 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9302 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9371 core/libs/tests/GuestFilesystem.cpp
PR 9383 core/libs/prx/libkernel/CMakeLists.txt
PR 9389 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9557 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9566 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9843 docs/dev/debt/libkernel.md
PR 9938 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 10025 core/libs/prx/libkernel/Apr/src/Apr.cpp
docs/dev/debt/libkernel.md
PR 10319 core/libs/prx/libkernel/File/src/Open.cpp
core/libs/tests/GuestFilesystem.cpp
docs/dev/debt/libkernel.md
PR 10442 docs/dev/debt/libkernel.md
PR 11831 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/Open.cpp
PR 11896 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 11939 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 12104 docs/dev/debt/libkernel.md
PR 12544 core/libs/prx/libkernel/CMakeLists.txt
PR 12560 core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp
core/libs/prx/libkernel/File/src/Open.cpp
core/libs/tests/GuestFilesystem.cpp
docs/dev/debt/libkernel.md
PR 12682 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp
core/libs/prx/libkernel/File/src/Open.cpp
docs/dev/debt/libkernel.md
PR 12688 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp
core/libs/prx/libkernel/File/src/Open.cpp
docs/dev/debt/libkernel.md
PR 12700 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp
core/libs/prx/libkernel/File/src/Open.cpp
docs/dev/debt/libkernel.md
PR 12965 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp
core/libs/prx/libkernel/File/src/Open.cpp
docs/dev/debt/libkernel.md
PR 12970 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 13560 docs/dev/debt/libkernel.md
PR 14103 docs/dev/debt/libkernel.md
PR 14128 docs/dev/debt/libkernel.md
PR 14212 docs/dev/debt/libkernel.md
PR 14264 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 14299 core/libs/prx/libkernel/CMakeLists.txt
PR 14310 docs/dev/debt/libkernel.md
PR 14832 docs/dev/debt/libkernel.md
PR 14854 docs/dev/debt/libkernel.md
PR 14886 docs/dev/debt/libkernel.md
PR 14894 docs/dev/debt/libkernel.md

@klosixgaming-arch
klosixgaming-arch force-pushed the fix/kernel-open-long-path branch from 7925fd2 to 97a7c25 Compare October 22, 2026 23:42
@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Thanks. Added core/libs/tests/WindowsExtendedPath.cpp (Windows only, windows_extended_path in ctest) covering a short path left unchanged, drive, forward slashes, dot segments, UNC, already-extended, device, a path that shrinks below 260 after normalization, and a relative path.

The test found a real bug in the helper: on this toolchain (WinLibs GCC 15.2.0, libstdc++) std::filesystem::absolute does not treat \\server\share\... as a UNC root, so a long UNC path came out as \\?\C:\server\share\.... UNC paths are now handled before absolute() and give \\?\UNC\server\share\.... I also removed a dangling reference to a temporary's native() that I had in the draft of that change.

The branch is force-pushed as two commits: the fix and the test. The test passes here; I ran it on Windows only. I re-ran Samurai Maiden on the final build: no bundle open fails (only /app0/Media/UnitySubsystems, which the dump does not have) and the title runs past the menu as before.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@MarcZX10

Copy link
Copy Markdown
Contributor

The UNC fix and new Windows-only path tests cover the reported cases well. One remaining helper edge case: the first path.native().size() < MAX_PATH return also bypasses absolute() for short relative paths. A relative path shorter than 260 characters can still resolve beyond 260 when the current directory is long; CreateFileW keeps relative paths subject to MAX_PATH, and the extended prefix cannot be applied to a relative path. Could the helper resolve relative paths before making the length decision, and add a Windows test with a short relative path under a long current directory? Microsoft documents that relative paths remain MAX_PATH-limited: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation.


inline std::filesystem::path WindowsExtendedPath(const std::filesystem::path& path) {
#ifdef _WIN32
constexpr std::size_t maxPath = 260;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The early return still leaves the short-relative-path case from my previous comment unchanged: relative\\file returns before absolute() is called, even if the current directory makes its resolved path reach MAX_PATH. WindowsExtendedPath.cpp currently tests a relative input whose own length exceeds 260, so it does not exercise that case. Please resolve relative paths before this length decision (or otherwise ensure the resolved path is checked), and add a short relative input under a long current directory as a regression test.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exact-head implementation still returns short relative paths before resolving them. A short relative input can resolve past MAX_PATH under a long current directory, while the added test only uses a relative string already longer than the limit. Please resolve/check the relative path before this early return and add the short-relative-under-long-current-directory regression case. Exact-head diff-check and conventions pass; hosted eligibility, conventions, labels and report pass, while product/relinker jobs are skipped. I have not run the Windows-only CTest locally.

@MarcZX10

Copy link
Copy Markdown
Contributor

Additional check on exact head 97a7c258: I cross-compiled the new WindowsExtendedPath.cpp and its exact helper header with MinGW-w64 GCC 12.2 using C++20, -Wall -Wextra -Werror, and linked the Windows x64 executable successfully. This is compile/link coverage only; this host has no Windows runtime, so the assertions were not executed. The short-relative-path behavior described in my requested-change review remains unaddressed.

@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Agreed, and fixed in a third commit (the branch is pushed, no force needed this time). The helper now resolves a relative path before comparing its length; an absolute path under 260 characters still returns at once, so the common case does no extra work. A path that absolute() cannot resolve is returned unchanged.

New case in windows_extended_path: a 35-character relative name under a 240-character current directory (the test creates it under the temp folder, changes into it, and removes it afterwards) must come back as \\?\<current directory>\<name>; a short relative name under a short current directory is checked to stay unchanged. The test passes on Windows 11 with WinLibs GCC 15.2.0. I did not run it without the helper change, and not on Linux, where the helper returns the path unchanged.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head b1ab76973a82cfe88193fae36a62f23e4e4289f4. The third commit resolves relative paths before checking the normalized absolute length, preserves the fast path for short absolute paths, and adds the requested short-relative-name under long-current-directory case with cleanup. The reported Windows 11 run passes the updated helper test and repeats the title reproduction. I independently compiled and linked the exact Windows test and helper with MinGW-w64 GCC 12.2 (-std=c++20 -Wall -Wextra -Werror); this host has no Windows runtime, so I did not execute the binary. Exact-head diff-check and conventions pass. Product/relinker CI remains skipped by repository path policy.

Comment thread core/libs/tests/WindowsExtendedPath.cpp Outdated
const auto original = std::filesystem::current_path();
const std::wstring file = L"file_" + std::wstring(30, L'x');
if (original.native().size() + file.size() + 1 < 260) Require(Extend(file) == file);
const auto root = std::filesystem::temp_directory_path() / L"aps5_extended_path_test";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test uses a fixed path under the system temp directory and later calls remove_all(root). If aps5_extended_path_test already exists, the test can delete unrelated user data; two concurrent runs can also interfere with one another. Please create a unique, exclusively-created temporary directory for each run and remove only that directory.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One issue remains in the new regression test: it creates and recursively removes a fixed directory under the system temp path (aps5_extended_path_test). That can delete a pre-existing directory belonging to the user or interfere with a concurrent test run. Please use an exclusively created unique temp directory and clean up only that directory. The short-relative-path fix itself is present; exact-head diff-check/conventions pass, and the Windows test/helper cross-compile and link cleanly (runtime assertions were not run here).

@MarcZX10

Copy link
Copy Markdown
Contributor

For a repository-local pattern, GuestFopenLongHostPath.cpp derives its temp root from steady_clock; for this test, also create that candidate with create_directory and only install cleanup after exclusive creation succeeds. That avoids deleting a colliding pre-existing path.

@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Fixed in a fourth commit (pushed, no force). The relative-path case now creates its directory with std::filesystem::create_directory under a name built from steady_clock plus an attempt number, retries up to 16 times if that name already exists, and throws if it cannot create one. Only a directory it created is removed, and the current directory is restored first, on every exit (a small RAII class, so a failed check cleans up too). It no longer touches a fixed aps5_extended_path_test path.

windows_extended_path passes on Windows 11 with WinLibs GCC 15.2.0. Not run on Linux (the helper returns the path unchanged there).

AI-assisted: yes. Written with AI assistance and reviewed by me.

@adriadam10 adriadam10 self-assigned this Oct 23, 2026
@adriadam10
adriadam10 force-pushed the fix/kernel-open-long-path branch from 1282b3a to eff20c8 Compare October 23, 2026 23:32
@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Re-checked on current main (e2b9700a98) on Windows 11 with an RTX 5090, because Samurai Maiden (PPSA04132) is back to freezing at the start on main.

  • On main: a 112 s run opens its first long-named Unity bundles under /app0/Media/StreamingAssets/aa/PS5/, logs nothing after 3.3 s, and never reaches the menu.
  • With this PR's head (eff20c8d8d) merged onto that main: the same run opens 1447 files, keeps compiling shaders and logging past 49 s, and does not stall at the start.

I only checked the log for the stall, not the picture. The earlier run on this PR reached the menu.

The re-review of the last commit (unique temp directory in the test) is still open if anyone has a moment.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 24, 2026
@adriadam10
adriadam10 dismissed MarcZX10’s stale review October 24, 2026 10:20

Dismissing a stale review: the requested changes were addressed after it (new head and/or updated description with the asked-for runs) and the merge queue blocks on it. Please request changes again if anything is still open.

@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (cf528e78c6). One conflict, in docs/dev/debt/libkernel.md: main added a runtime module initialization entry next to this PR's long-path entry, and I kept both. The code is unchanged; the five commits are the same.

I built the rebased head and windows_extended_path passes (Windows 11, WinLibs GCC 15.2.0). That is the only test I ran on it. The last commit (unique temporary directory in the test) is still waiting on a review.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@klosixgaming-arch
klosixgaming-arch force-pushed the fix/kernel-open-long-path branch from eff20c8 to 4cda47b Compare October 25, 2026 00:26
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@wachi35676

Copy link
Copy Markdown
Contributor

Windows run of head 4cda47b6632f merged onto main aeff26bd9. Windows 11 10.0.26300, WinLibs GCC 15.2.0, Release.

  • windows_extended_path, guest_filesystem and the seven guest_apr_read_* tests: 9/9 pass, 2 runs.
  • Guest-level check: I added sceKernelOpen calls to main's TestLongHostPathStat in GuestFilesystem.cpp, which already aliases a >284-character directory:
/long-stat-host/... main this PR
open O_RDONLY existing, read 8 bytes 0x80020002 0, data correct
open O_WRONLY|O_CREAT new, write 0x80020002 0
open O_DIRECTORY + getdents 0x80020002 0x8002000d
mkdir / rename / unlink / rmdir 0x80020002 0x80020002

So the file open fix works.

Three notes:

  1. Long directories now fail with EACCES instead of ENOENT. With the \\?\ form, CreateFileW on a directory fails with ACCESS_DENIED. The fallback in OpenNative that would open it as a directory (std::filesystem::is_directory(directory, error), Open.cpp:242) still uses the plain path, so it returns false and EACCES goes back to the title. A short directory with the same flags opens fine. Using the extended path in that fallback, and in OpenDirectoryDescriptor, should make it open.
  2. The debt line is out of date on stat. Since ee5ff02 and 28eaa5b, main stats long paths through NativeMetadataPath in the same Open.cpp, and TestLongHostPathStat covers it. That function does the same \\?\ / \\?\UNC\ conversion as the new helper. Sharing one helper would avoid two copies.
  3. Sharing the helper would also fix NativeMetadataPath. It calls std::filesystem::absolute() first. With WinLibs GCC 15.2.0, absolute(L"//server/share/dir/file.bin") gives D:\server\share\dir\file.bin (is_absolute() is 0), so a long UNC path goes wrong in stat. This is the issue you already handle in the new helper.

AI-assisted: yes (Claude Code); results above were run on this machine.

@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Thanks for running this against main and for the guest-level comparison, that is a more direct check than my unit test.

I agree with all three points and pushed a follow-up commit (aa978a8):

  1. The directory fallback in OpenNative and OpenDirectoryDescriptor now use the extended path, so a long directory opens instead of failing with EACCES. The directory listing iterator uses it too.
  2. NativeMetadataPath now calls a shared File::WindowsNormalizedPath, which WindowsExtendedPath also uses. A UNC path is converted before absolute() is called, so a long UNC stat no longer goes through the drive-relative rewrite on WinLibs GCC.
  3. The stat part of the debt line is gone. It now says that mkdir, rename, unlink and rmdir still use the plain path.

I added your cases to the tests: opening and reading a long file and opening a long directory with O_DIRECTORY in guest_filesystem, and short, long and //server/share UNC paths in windows_extended_path. On Windows 11 with WinLibs GCC, windows_extended_path, guest_filesystem and the seven guest_apr_read_* tests pass (9 of 9). I did not run getdents on the long directory, only the open.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@wachi35676

Copy link
Copy Markdown
Contributor

Re-checked head aa978a81e6 merged onto main b90d009d9f (clean). Windows 11 10.0.26300 with WinLibs GCC 15.2.0, and WSL Ubuntu 24.04 with GCC 13.3, both Release.

  • The tests pass: windows_extended_path, guest_filesystem and the seven guest_apr_read_*, 9/9 on Windows, 2 runs, and 6/6 on Linux.
  • With Open.cpp, DirectoryDescriptor.cpp and Apr.cpp from main, guest_filesystem fails at the new sceKernelOpen check.
  • My three earlier points are fixed. With the >284-character directory from TestLongHostPathStat, an O_DIRECTORY open now returns 0 (it was 0x8002000d). O_WRONLY on it gives EISDIR, and O_DIRECTORY on a file gives ENOTDIR. Through \\localhost\C$, a long UNC path can be stat'ed, opened and read. A short UNC directory, which main's NativeMetadataPath fails to stat (0x80020002), now stats.

One new problem: listing a long directory returns the current directory's entries. sceKernelGetdents on that open descriptor returns apr_read_direct, apr_read_eof, …, CMakeFiles, cmake_install.cmake. Those are the files in ctest's working directory. The directory itself holds only payload.bin and created.bin. The long UNC directory gives the same wrong list. A short UNC directory lists correctly.

The path Load passes is correct (\\?\C:\Users\...\anyps5-stat-long-...\dddd...). The problem is libstdc++. With WinLibs GCC 15.2.0, std::filesystem::directory_iterator on a 313-character \\?\D:\... directory returns no error but lists the current directory. I checked this with a standalone program run from two different directories. FindFirstFileW(path + L"\*") on the same path works. Replacing the iterator in Load with FindFirstFileExW / FindNextFileW on WindowsExtendedPath(*path) + L"\*" lists created.bin payload.bin for the long local and the long UNC directory, and guest_filesystem still passes. The new test only opens and closes the directory, so it doesn't catch this. A getdents check that looks for payload.bin would.

AI-assisted: yes (Claude Code); results above were run on this machine.

@Gruzzberry

Copy link
Copy Markdown
Contributor

Gruzzberry here: I am opening a small PR that changes the file number (d_fileno) getdirentries reports in DirectoryDescriptor.cpp, next to lines your PR edits. Proposal: yours goes first and mine rebases. Tell me if you would rather go the other way.

@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Thanks, good catch, and the standalone repro made it easy to confirm. I pushed a follow-up commit (22a7ae7):

  • Load in DirectoryDescriptor.cpp no longer uses std::filesystem::directory_iterator. It enumerates with FindFirstFileExW / FindNextFileW on WindowsExtendedPath(path / L"*"), skips . and .. (which are added separately as before), and takes the directory flag from dwFileAttributes. Names go through the same path(...).string() conversion as before.
  • TestLongHostPathStat now reads the long directory with sceKernelGetdents and requires exactly ., .. and payload.bin, so a listing of the working directory fails the test. I did not run it against the old iterator, so I can only say it passes with the new code.

I merged the branch onto current main (14c99b5, clean merge) and built it on Windows 11 with WinLibs GCC 15.2.0, Release. Run 3 times with --repeat until-fail:3:

  • windows_extended_path, guest_filesystem, guest_apr_read_*: pass.
  • Directory and file tests around the change: guest_directory_buffer, guest_directory_rewind, guest_directory_entries, guest_directory_close, guest_positioned_close, guest_kernel_rename, guest_truncate, guest_truncate_extend, guest_dup, guest_append_*, guest_fstatfs, guest_fstat_fault, guest_file_vector*, guest_file_timestamps, guest_kernel_fcntl, guest_rename_dot, guest_path_name_length, download_data_mount_*, guest_kernel_app_archive_*: pass.
  • Not run: guest_truncate_race, guest_kernel_fcntl_sync and guest_directory matched my filter but I had not built their executables, and guest_directory_link_type and guest_path_errors would not build as targets in this configuration. I have not looked into why.

I did not test the long UNC listing (\\localhost\C$) or Linux, which you covered.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks for this. The current head doesn't build on Linux:

core/libs/prx/libkernel/File/src/Open.cpp:224:33: error: 'WindowsExtendedPath' is not a member of 'File'
  224 |     const auto openPath = File::WindowsExtendedPath(native);

In Open.cpp the new #include "prx/libkernel/File/include/WindowsExtendedPath.hpp" (line 37) sits inside the #ifdef _WIN32 block that starts at line 31, but the call at line 224 is outside any #ifdef. The header itself is portable (WindowsExtendedPath returns the path unchanged when _WIN32 isn't defined), so moving that include above the #ifdef _WIN32 block should fix the Linux build. Could you move it and push?

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 2, 2026
@klosixgaming-arch
klosixgaming-arch force-pushed the fix/kernel-open-long-path branch from 94db793 to 4eac542 Compare November 2, 2026 09:25
@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

@oneandonlydean thanks, fixed in 4eac542. The WindowsExtendedPath.hpp include now sits with the other prx/libkernel/File includes, above the #ifdef _WIN32 block. The other files that call it (Apr.cpp, DirectoryDescriptor.cpp and the test) already include it unconditionally.

I checked Open.cpp on Linux with g++ -std=c++2b -fsyntax-only (g++ 11.4) and got no errors. The old include placement reproduces your error at line 224. I haven't done a full Linux build.

I also rebased the branch onto main 88e804b to clear the conflict. The only conflict was in docs/dev/debt/libkernel.md: I kept #9882's rewritten sceKernelMprotect entries and put the Windows long paths entry where main had the blank line, as agreed with orezraey.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Nov 2, 2026
@klosixgaming-arch
klosixgaming-arch force-pushed the fix/kernel-open-long-path branch from 4eac542 to a5be6b2 Compare November 2, 2026 16:35
@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (head a5be6b2). The only conflict was in core/libs/prx/libkernel/CMakeLists.txt: main added the guest_direct_memory_for_app test at the same spot as windows_extended_path, so both are kept. Open.cpp still compiles on Linux (g++ -fsyntax-only).

AI-assisted: yes. Written with AI assistance and reviewed by me.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 2, 2026

@BunnyFl BunnyFl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XairRrR's two points look addressed in 67c3f5e, and here's the Windows run the Linux reviews can't give. The O_NOFOLLOW and O_CREAT|O_EXCL link checks in Open.cpp (is_symlink, GetFileAttributesW, ReadJunction) now use the extended openPath. The APR end-of-file size check in Apr.cpp uses the extended hostPath, with the new longeof case. The UNC branch of WindowsExtendedPath keeps the \server\share root and gives \?\UNC\server\share\... for a long path.

Tested on Windows 11, WinLibs GCC 15.2.0, with this head merged onto current main: 15 of 15 pass, nothing skipped. They are guest_filesystem, guest_directory, guest_directory_entries, the nine guest_apr_read_* cases including guest_apr_read_longeof, long_paths, agc_pipeline_journal_long_paths and agc_shader_disk_cache_long_paths. With main's Apr.cpp, DirectoryDescriptor.cpp, Open.cpp and libkernel CMakeLists.txt (XairRrR's negative-control set), guest_filesystem aborts (0xc0000409).

@XairRrR XairRrR left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked — approved

Both blocking findings from my last review are fixed. Commit 67c3f5e7b9 puts the sceKernelOpen link checks and the APR end-of-file file_size call on the extended path and adds LongNoFollow and guest_apr_read_longeof. Commit a5be6b2720 moves the helper include out of the #ifdef _WIN32 block, so the Linux build works again. The two failures the bot reported come from main, not from this PR.

What changed since my last review

I last reviewed fa00e458e6. Since then:

  • 67c3f5e7b9: OpenNative now computes openPath once and uses it for the O_DIRECTORY status check, the O_CREAT|O_EXCL reparse-point check and the O_NOFOLLOW checks. _readFile uses one hostPath for the stream and for file_size. _fileSize extends the parent directory before listing it and extends the path in its is_regular_file fallback. The unreachable #ifdef in Load is gone. UNC normalization now keeps \\server\share as the root.
  • New tests: LongNoFollow in GuestWindowsDirectoryLinkTypes.cpp, guest_apr_read_longeof, and a \\server\share\..\file.bin case in windows_extended_path.
  • a5be6b2720: the WindowsExtendedPath.hpp include in Open.cpp sits above the #ifdef _WIN32 block. This fixes the Linux build error reported by oneandonlydean.
  • Rebases: the branch was rebased onto newer main. The range-diff shows only context changes in the earlier commits.
  • Other reviews: BunnyFl approved and reported a Windows run: 15/15 pass, and with main's production files guest_filesystem aborts.

What I ran

Head a5be6b2720c0af77b3324523f07af18b8c870793, main 5cb015caaaaf83f8ee431414ee6a457b5b307d49, merged tree 766baa0fe992b51a75c8c6b93fc766ee0297cf94 (Linux, GCC 14.2.0, Release, Ninja).

git diff --check 5cb015caaaaf83f8ee431414ee6a457b5b307d49...a5be6b2720c0af77b3324523f07af18b8c870793
python3 tools/check_conventions.py --base 5cb015caaaaf83f8ee431414ee6a457b5b307d49
ctest --test-dir build --output-on-failure --repeat until-fail:3 \
  -R '^(agc_driver_image_sample_lod_clamp_shader_lane|unwrap_self_UnwrapTest_test_new_output_inherits_the_destination_directory_group|guest_filesystem|guest_apr_read_.*)$'
# attempted negative control: Apr.cpp, DirectoryDescriptor.cpp, Open.cpp, libkernel CMakeLists.txt from main
git show 5cb015caaa:<path> > <path>
ninja -C build -j32 -l32 libkernel agc_driver_image_sample_lod_clamp_tests guest_filesystem_tests guest_apr_read_tests
git show 766baa0fe9:<path> > <path>
git status --porcelain
git log --oneline 096c45570f..5cb015caaa -- core/libs/prx/libSceAgcDriver core/shader tools/unwrap_self.py tools/tests/test_unwrap_self.py
Check Result
git diff --check clean
check_conventions.py All conventions checks passed.
Build (script) configure, build and libs OK
guest_filesystem, the 7 Linux guest_apr_read_* cases pass, 3 repeats each
agc_driver_image_sample_lod_clamp_shader_lane fails every time; comes from main (see below)
unwrap_self_..._destination_directory_group fails (os.chown → EINVAL in the sandbox); comes from main (see below)
windows_extended_path, guest_windows_directory_link_types, guest_apr_read_longeof not built here (Windows-only)

The two new failures come from main:

  • The failure baseline is from 096c45570f, which is older than main. Since then main merged 4e2a028e43 fix(agc): bind a T# minimum LOD through an S# that holds the LOD at 0 (#12980) and new unwrap test cases (#14558). Those are the two failing tests.
  • On Linux the PR changes nothing at run time: WindowsExtendedPath returns its input unchanged when _WIN32 is not defined. Every other changed line in Apr.cpp, Open.cpp and DirectoryDescriptor.cpp is inside _WIN32 code or goes through that helper.
  • The unwrap test is a Python test of tools/, which this PR does not touch.
  • I tried to confirm the AGC failure by rebuilding with main's production files, but could not (see Not verified).
AGC failure (same as build.json)
AGC graphics: image_sample_lz through the S# that holds the LOD at 0 with T# MIN_LOD 768 ... thread 0 component 1 is 0.000000, expected 0.062500

Previous findings

  • Blocking 1, sceKernelOpen link checks on the plain path: Resolved. core/libs/prx/libkernel/File/src/Open.cpp:235-255 uses openPath for status, both GetFileAttributesW calls, is_symlink and ReadJunction. The LongNoFollow case expects 0x8002001F for a junction past MAX_PATH. I checked this in source. The Windows run and negative control are BunnyFl's and the author's reports.
  • Blocking 2, APR end-of-file file_size on the plain path: Resolved. core/libs/prx/libkernel/Apr/src/Apr.cpp:165-188 uses hostPath for both the stream and file_size. guest_apr_read_longeof reads 200 bytes from a 100-byte long-path file and checks the zero fill and the guard bytes. Its Windows result is author-reported.
  • Non-blocking 1, APR size lookup and stat: Resolved. Apr.cpp:79 and Apr.cpp:106 extend the path. FillFileStat goes through NativeMetadataPath → WindowsNormalizedPath (Open.cpp:120-121), and the debt entry names the size lookup and the end-of-file check.
  • Non-blocking 2, dead #ifdef in Load: Resolved. The non-Windows loop now has only the DirectoryEntryType line.
  • Non-blocking 3, UNC .. under the share: Resolved. WindowsExtendedPath.hpp:20-28 normalizes only the part after \\server\share, and windows_extended_path has the \\server\share\..\file.bin case.

New findings

Blocking

None.

Non-blocking

  1. APR directory listing at the MAX_PATH boundary (core/libs/prx/libkernel/Apr/src/Apr.cpp:79).
    • The listing extends path.parent_path() first and appends \* afterwards. This is the boundary case mashingaan found in ListDirectoryEntries, which the PR already fixed there.
    • For a parent of 258 or 259 characters, WindowsExtendedPath leaves the parent unchanged and the search pattern reaches 260. FindFirstFileExW then fails, and an empty size map is cached for that directory.
    • Results stay correct, because each file then falls back to is_regular_file + FillFileStat on the extended path. But every file in such a directory pays a separate stat.
    • Suggested fix: File::WindowsExtendedPath(path.parent_path() / L"*"), as DirectoryDescriptor.cpp:145 does.

Not verified

  • Linux negative control: I did not repeat it. On Linux the helper returns the path unchanged, so a Linux run cannot fail. My previous review showed this.
  • AGC failure check: I tried to confirm it with main's production files, but the sandbox stopped the build: CMake's regeneration fails with Operation not permitted in SDL2's configure_file. That attribution rests on the main history and on the Linux no-op argument above. The worktree is back at the merged tree (git status --porcelain is empty), but the build tree's build.ninja is now marked for regeneration.
  • Windows: nothing was compiled or run on Windows here. The Windows tests and the Windows negative controls are from BunnyFl's and the author's runs.
  • Title run: the PPSA04132 reproduction was not run (Windows-only, and this review does not launch titles).

@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ AMD GPU test: passed

PR head a5be6b2 merged onto main a4eca2a · tested 2026-11-03 00:17 UTC · 30 min

GPU AMD Radeon RX 6300 (RADV NAVI24), driver Mesa 25.2.8-0ubuntu0.24.04.4, Vulkan 1.4.318
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-42-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 3,481 passed · 13 failed (0 new, 12 also fail on main, 1 unstable on main) · 59 skipped (ANYPS5_REQUIRE_VULKAN=1)
Test setup CPU-only tests without a GPU; GPU tests (and any CPU-only test that failed or skipped without one) once on the GPU with VK_LAYER_KHRONOS_validation and ANYPS5_NO_SHADER_CACHE=1; main ran the same way
Test selection 3,553 of 3,591 tests run: the ones this change can reach, chosen by tools/select_tests.py from main's build graph
Vulkan validation ✅ no new validation IDs vs main (1311 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.1 s, ⚠️ agc_driver_graphics 79.9 s, ⚠️ agc_driver_graphics_maps_fallback 77.0 s, ⚠️ agc_driver_image_sample_unnormalized 53.5 s, ⚠️ public_headers 43.3 s
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_pipeline_journal: failed in 4 of the last 6 main runs
Failing on main too (12, not counted against this PR)
agc_driver_guest_memory_lifetime_direct
agc_driver_image_table
agc_driver_job_manager_compute_capacity
agc_driver_native_depth_bda_clear
agc_driver_native_depth_buffer_clear
agc_driver_native_depth_clears
agc_driver_native_multisample_depth
agc_driver_submit_progress
agc_idle_publish_wait
agc_submit_progress_no_timeline
guest_mmap_executable
guest_socket_options_multicast
Validation IDs already present on main (4 IDs in 754 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (754 tests): agc_compute_pipeline_replay, agc_driver, agc_driver_address_passes, agc_driver_address_passes_draw_transitions, agc_driver_address_passes_targets_split, agc_driver_alias_copy_label, agc_driver_alpha_to_coverage, agc_driver_array_layer_target, and 746 more
VUID-vkCmdCopyImage-srcImage-01548 (11 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_cpu_write_after_memo, agc_driver_depth_plane_cpu_write_after_page_unwatch, agc_driver_depth_plane_cpu_write_after_partial_unwatch, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_partial_store_unimported, agc_driver_depth_storage_dispatch, and 3 more
VUID-vkCmdCopyImage-srcImage-01551 (11 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_cpu_write_after_memo, agc_driver_depth_plane_cpu_write_after_page_unwatch, agc_driver_depth_plane_cpu_write_after_partial_unwatch, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_partial_store_unimported, agc_driver_depth_storage_dispatch, and 3 more
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (2 tests): agc_driver_prepared_exports, agc_driver_srgb8_color_target
Skipped (59)
agc_driver_bda_reads_shadowed_results: skipped, the device reports watched host memory written when it imports or reads it
agc_driver_bda_reads_shadowed_results_per_build: skipped, the device reports watched host memory written when it imports or reads it
agc_driver_copied_writer_label_order: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_copied_writer_label_order_drain: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_float16_unused_nan_payloads: skipped, the device has no native f16 conversions with RTE rounding and kept f16 denormals
agc_driver_image_table_vouched_lookups: skipped, the device reports 4 of 4 host-imported pages written after a GPU read, so a watched imported table cannot be vouched for
agc_driver_linear_target_direct_write_completion: skipped, the device does not keep host imports write-watched, so no completion stamp over the target is tracked
agc_driver_ngg_vertex_wave_reduction: skipped, s3 describes a 32-lane wave only on 32-lane subgroups, and the device has 64 lanes
agc_driver_scattered_runs: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_sparse_dma_buf_import: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_unit_shadow: skipped, the device does not import watched host memory for unit shadows
agc_driver_unit_shadow_budget: skipped, the device does not import watched host memory for unit shadows
agc_driver_vertex_elected_atomic_max: skipped, the device runs vertex programs in subgroups, so they keep their cross-lane reductions
agc_host_import_sparse: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_import_settle_initial_dcc: skipped, the device has no deferred settle of host imports: imports under 1 MiB wait for one only on Windows
agc_shadowed_key_stores: skipped, the device imports no watched host memory under a unit shadow
agc_small_tiled_shadow_label: skipped, host imports are unavailable: the test block is not imported and watched
agc_storage_clear_hand_over: skipped, the device imports no watched host memory for the clear-key hand-over
agc_unit_shadow: skipped, the device does not import watched host memory for unit shadows
agc_unit_shadow_completion_store: skipped, the device reports watched host memory written when it imports or reads it
guest_http_uri_locale: 
guest_net_ctl: skipped, the host has no IPv4 address
guest_video_out_color_space: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_hdr: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_high_frame_rate: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_output_mode: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_resolution_status: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_wqhd_detection: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
video_out_flip_aftergpu: skipped, no display or Vulkan device: DisplayWindow: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
video_out_flip_backtoback: skipped, no display or Vulkan device: DisplayWindow: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
video_out_flip_controls: skipped, no display or Vulkan device: DisplayWi
Reproduce
git fetch origin a5be6b2720c0af77b3324523f07af18b8c870793 a4eca2a056bf3bbbec98ab42e9d28163e4ea9cb8
git checkout a4eca2a056bf3bbbec98ab42e9d28163e4ea9cb8 && git merge --no-ff a5be6b2720c0af77b3324523f07af18b8c870793
git submodule update --init --recursive
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_ENABLE_SPIRV_TOOLS=ON
cmake --build build && cmake --build build --target libs
VK_DRIVER_FILES=/nonexistent/icd.json ctest --test-dir build -E '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 12 --output-on-failure
ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation \
  ctest --test-dir build -R '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 16 --output-on-failure
# failures are then re-run alone: --timeout 360 --parallel 2; new validation IDs: the test alone, twice, --parallel 1

Automated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.

@github-actions github-actions Bot added the review queue Next in the maintainer review queue: checks pass, no open change request, no conflict label Nov 3, 2026
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed review queue Next in the maintainer review queue: checks pass, no open change request, no conflict labels Nov 3, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Hi klosixgaming-arch,

Thanks for moving the WindowsExtendedPath.hpp include out of the Windows block in a5be6b2. The Linux build is green at this head (the AMD GPU run on Linux built and passed with 0 new failures).

Since then, #12560 (merged today) added a shared helper for the same job: NativeVerbatimPath_nid_no_patch in core/libs/prx/libc/include/WindowsVerbatimPath.hpp:241. main now uses it in open (core/libs/prx/libkernel/File/src/Open.cpp:120, plus the metadata paths from :236) and in the directory open (core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp:231, with WindowsMaxDirectoryPathLength). This PR now conflicts with it in those two files, and File::WindowsNormalizedPath / File::WindowsExtendedPath would be a second helper for the same \\?\ prefixing.

Could you rebase onto current main and build on NativeVerbatimPath_nid_no_patch instead of adding WindowsExtendedPath.hpp? The parts main still lacks are worth keeping:

  • the APR reads and end-of-file check in Apr.cpp;
  • listing long directories with FindFirstFileExW and the extended search pattern;
  • the extended /app0 root;
  • the new tests.

If the helper's rules differ from yours for a case you need (for example its handling of DOS device names and trailing dots or spaces), please say which case, so the difference is settled in one place.

…ctory listings on Windows

Builds on NativeVerbatimPath_nid_no_patch from boykopovar#12560 instead of a second helper: the APR read, size lookup and end-of-file check, the /app0 root, the O_NOFOLLOW symlink check and the directory fallback in open, and directory listings through FindFirstFileExW with a verbatim search pattern.
@klosixgaming-arch
klosixgaming-arch force-pushed the fix/kernel-open-long-path branch from a5be6b2 to 61546e6 Compare November 3, 2026 14:14
@klosixgaming-arch

Copy link
Copy Markdown
Contributor Author

@oneandonlydean done in 61546e6. The branch is now one commit on main 4f41f4c. WindowsExtendedPath.hpp and its test are gone, and everything uses NativeVerbatimPath_nid_no_patch (with the plain path kept if it returns an error). What's left is the part main lacked:

  • Apr.cpp: the stream, the FindFirstFileExW size listing, the is_regular_file fallback and the end-of-file file_size
  • DirectoryDescriptor.cpp: a FindFirstFileExW listing with a verbatim search pattern on Windows, verbatim stats in Load, the /app0 root in InsideApp0, and the directory check in OpenDirectoryDescriptor
  • Open.cpp: two spots that still used the plain path, the O_NOFOLLOW is_symlink check and the directory fallback after a failed open
  • tests: guest_apr_read_longeof, LongNoFollow and the GuestFilesystem cases

I used the helper's default limit (248), as open does. I didn't need a case where its rules differ from mine. Windows results are in the description: the APR, link-type, filesystem and directory-entry tests pass. Antivirus blocked a few reruns here, and those are listed there too.

AI-assisted: yes. Written with AI assistance and reviewed by me.

@XairRrR XairRrR left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked — approved

The branch was rewritten as one commit, 61546e6932, on top of main 4f41f4c375. It removes WindowsExtendedPath.hpp and its test and calls #12560's NativeVerbatimPath_nid_no_patch instead. It keeps the APR, directory-listing, /app0-root and O_NOFOLLOW fixes and their tests. The diff check, the conventions check and the affected Linux tests pass, and I found nothing blocking.

What changed since my last review

I last reviewed a5be6b2720. Since then:

  • Rewrite onto the shared helper. oneandonlydean asked for this after #12560 was merged. File::WindowsExtendedPath / WindowsNormalizedPath and the windows_extended_path test are gone. The debt entry the earlier head added is gone too. main has no long-path entry in docs/dev/debt/libkernel.md, so nothing is left stale.
  • Apr.cpp: a new _hostPath (verbatim path, or the plain path if the helper returns an error) is used for the parent listing, the is_regular_file fallback, the stream and the end-of-file file_size.
  • DirectoryDescriptor.cpp:
    • HostPath does the same job.
    • InsideApp0 opens the root through it.
    • ListDirectoryEntries (FindFirstFileExW) builds the verbatim pattern from the complete directory / L"*".
    • Load stats the directory, its parent and each entry through verbatim paths.
    • OpenDirectoryDescriptor checks is_directory on the verbatim path.
  • Open.cpp: the O_NOFOLLOW is_symlink check uses openNative on Windows. The directory fallback after a failed open goes through NativeMetadataPath.
  • Tests: these are the same as before: guest_apr_read_longeof, the LongApp0Root, LongNoFollow and SearchPatternAtMaxPath cases in GuestWindowsDirectoryLinkTypes.cpp, and the long-path open and getdents checks in GuestFilesystem.cpp. All of them are Windows-only.
  • Windows results in the description are author-reported. On this head:
    • The APR, link-type, filesystem and directory-entry tests pass.
    • long_paths, guest_directory, guest_filesystem_error and agc_pipeline_journal_long_paths were not conclusive or not run. The author says antivirus blocked them.
    • The title was not retested.

What I ran

Head 61546e69320d5ecca1851f3c5a9f4298c590e4d1, main 4f41f4c375f5abb487dbd184fbc150f31994d762, merged tree a25d61afe035f9a511cfb1512ab077e7b3bc37ad (Linux, GCC 14.2.0, Release, Ninja).

git diff --check 4f41f4c375f5abb487dbd184fbc150f31994d762...61546e69320d5ecca1851f3c5a9f4298c590e4d1
python3 tools/check_conventions.py --base 4f41f4c375f5abb487dbd184fbc150f31994d762
ctest --test-dir build -j8 --test-load 32 --output-on-failure -R '^(guest_filesystem|guest_apr_read_.*|guest_directory.*)$'
git status --porcelain
Check Result
git diff --check clean
check_conventions.py All conventions checks passed.
Build (script) configure, build and libs OK
Script test run 0 new failures, 0 timeouts; the 9 known failures also fail on main (baseline is main_sha)
guest_filesystem, 7 Linux guest_apr_read_*, guest_directory, guest_directory_entries, guest_directory_link_types 10/10 pass
Windows-only tests (guest_apr_read_longeof, guest_windows_directory_link_types, the long-path part of guest_filesystem) not built here

Negative control

None on this machine. All new and changed test code is inside #ifdef _WIN32 (TestLongHostPathStat sits in the _WIN32 block of GuestFilesystem.cpp, longeof is only registered on WIN32, and GuestWindowsDirectoryLinkTypes.cpp is Windows-only). On Linux the production change does nothing at run time: NativeVerbatimPath_nid_no_patch returns its input when _WIN32 is not defined (core/libs/prx/libc/include/WindowsVerbatimPath.hpp:257-262), and the rest of the changed code is in _WIN32 branches. So no Linux run can fail with main's production files. The Windows negative controls come from earlier heads: the author's and BunnyFl's runs, against code that was functionally the same. Nobody has repeated them on this head.

Previous findings

  • Non-blocking 1, APR directory listing at the MAX_PATH boundary: Resolved. core/libs/prx/libkernel/Apr/src/Apr.cpp:86 still extends the parent before it appends \*. But the shared helper switches to the verbatim form from 248 characters (WindowsMaxDirectoryPathLength, WindowsVerbatimPath.hpp:12,252). So a parent left unprefixed is at most 247 characters, and its \* pattern stays at or below 249, under 260. The boundary case can no longer happen.

The two blocking findings from my earlier review (the link checks in sceKernelOpen, and the APR end-of-file file_size) were resolved at a5be6b2720. The rewrite keeps both fixes:

  • Open.cpp:243-257 uses openNative for the GetFileAttributesW, is_symlink and ReadJunction checks.
  • Apr.cpp:172 uses one hostPath for the stream and for file_size.

New findings

Blocking

None.

Non-blocking

  1. Two copies of the same wrapper. _hostPath (core/libs/prx/libkernel/Apr/src/Apr.cpp:70-74) and HostPath (core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp:42-50) both do "verbatim path, or the input on error". Open.cpp:235-237 and OpenPlaceholder (DirectoryDescriptor.cpp:271-273) write the same thing inline. This could become one helper next to NativeVerbatimPath_nid_no_patch, in this PR or a follow-up. The #ifdef in HostPath is also unneeded, because the helper already has a non-Windows overload.
  2. The new test cases leak on failure. LongApp0Root, LongNoFollow and SearchPatternAtMaxPath (core/libs/tests/GuestWindowsDirectoryLinkTypes.cpp) and TestLongEof remove their directories only on the success path. LongApp0Root also leaves the /app0 alias in place if Types throws. Each run uses a fresh steady_clock name, so a failed run only leaves directories behind in the build tree. A small RAII guard, like the one the earlier windows_extended_path test had, would clean them up.
  3. Checklist format. Unblocks: PPSA04132 has no Game bring-up discussion number. CONTRIBUTING asks for the form Unblocks: PPSA01325 #3232. Please add the discussion, if one exists.

Not verified

  • Windows: nothing was compiled or run on Windows here. The Windows results on this head are the author's report and are partial, as the description says.
  • Windows negative control: not repeated on this head (see above).
  • Title run: the PPSA04132 reproduction was not run. It is Windows-only, and this review does not launch titles.

@BunnyFl BunnyFl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 61546e6932, the one-commit rewrite onto main 4f41f4c375. My earlier approval was for a5be6b2720. This head is a new implementation, and I read the whole diff again rather than a delta.

What I read

  • The new _hostPath / HostPath helpers in Apr.cpp and DirectoryDescriptor.cpp call NativeVerbatimPath_nid_no_patch (core/libs/prx/libc/include/WindowsVerbatimPath.hpp:241) and keeps the plain path when it returns an error. The helper returns \\?\ and \\.\ paths unchanged (core/libs/prx/libc/include/WindowsVerbatimPath.hpp:247), so ListDirectoryEntries can pass an already verbatim directory back through HostPath without doubling the prefix. Outside Windows it returns the path as given (core/libs/prx/libc/include/WindowsVerbatimPath.hpp:257-262), so the unguarded _hostPath in Apr.cpp is a no-op on Linux and macOS.
  • The verbatim entry paths that Load passes to FillLinkStat / TryFillFileStat are safe: NativeStatPath in NativeStat.cpp leaves \\?\ paths as they are.
  • InsideApp0 only changes how the /app0 root is opened. The component comparison against the guest directory is unchanged.
  • In Open.cpp, the Windows O_NOFOLLOW check now tests openNative, the path the open itself uses. The directory fallback stats through NativeMetadataPath, falling back to the plain path on error, like the metadata paths above it. Non-Windows code is unchanged.
  • The Windows listing in Load throws on any FindFirstFileExW / FindNextFileW error except ERROR_FILE_NOT_FOUND and ERROR_NO_MORE_FILES, like the directory_iterator loop it replaces.

What I ran (Windows 11, WinLibs GCC 15.2.0, Release, this head merged onto 4f41f4c375): 16 tests (guest_apr_read_* including longeof, guest_filesystem, guest_directory, guest_directory_entries, guest_windows_directory_link_types, long_paths, agc_pipeline_journal_long_paths, agc_shader_disk_cache_long_paths): 16/16 pass.

Negative control: the same tests with Apr.cpp, DirectoryDescriptor.cpp and Open.cpp taken from main: 3 of 16 fail, all in the new cases.

  • guest_apr_read_longeof
  • guest_filesystem: "Filesystem check failed at line 392 (guest errno 13, Win32 error 3)", the long /long-stat-host/ directory open.
  • guest_windows_directory_link_types: line 120, the directory open in Types().

One limit of this run: the link-type test printed "Symbolic-link cases require Windows developer mode or symbolic-link privilege; junction cases still run". Its symbolic-link cases did not run here; the junction cases did.

@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ AMD GPU test: passed

PR head 61546e6 merged onto main 4f41f4c · tested 2026-11-03 14:45 UTC · 21 min

GPU AMD Radeon RX 6300 (RADV NAVI24), driver Mesa 25.2.8-0ubuntu0.24.04.4, Vulkan 1.4.318
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-42-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 3,697 passed · 16 failed (0 new, 13 also fail on main, 3 unstable on main) · 60 skipped (ANYPS5_REQUIRE_VULKAN=1)
Test setup CPU-only tests without a GPU; GPU tests (and any CPU-only test that failed or skipped without one) once on the GPU with VK_LAYER_KHRONOS_validation and ANYPS5_NO_SHADER_CACHE=1; main ran the same way
Test selection 3,773 of 3,811 tests run: the ones this change can reach, chosen by tools/select_tests.py from main's build graph
Vulkan validation ✅ no new validation IDs vs main (1334 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.5 s, ⚠️ agc_driver_graphics 117.0 s, ⚠️ agc_driver_graphics_maps_fallback 112.4 s, ⚠️ agc_driver_bda_device 62.3 s, ⚠️ public_headers 52.9 s
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_array_layer_target_cpu: failed in 1 of the last 9 main runs
agc_driver_pipeline_journal: failed in 7 of the last 9 main runs
agc_driver_two_sample_alpha_to_coverage_sample_mask_export: failed in 1 of the last 9 main runs
Failing on main too (13, not counted against this PR)
agc_driver_alpha_to_coverage_sample_mask_export
agc_driver_guest_memory_lifetime_direct
agc_driver_image_table
agc_driver_job_manager_compute_capacity
agc_driver_native_depth_bda_clear
agc_driver_native_depth_buffer_clear
agc_driver_native_depth_clears
agc_driver_native_multisample_depth
agc_driver_submit_progress
agc_submit_progress
agc_submit_progress_no_timeline
guest_mmap_executable
guest_socket_options_multicast
Validation IDs already present on main (4 IDs in 767 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (767 tests): agc_compute_pipeline_replay, agc_driver, agc_driver_address_passes, agc_driver_address_passes_draw_transitions, agc_driver_address_passes_targets_split, agc_driver_alias_copy_label, agc_driver_alpha_to_coverage, agc_driver_alpha_to_coverage_sample_mask_export, and 759 more
VUID-vkCmdCopyImage-srcImage-01548 (12 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_cpu_write_after_memo, agc_driver_depth_plane_cpu_write_after_page_unwatch, agc_driver_depth_plane_cpu_write_after_partial_unwatch, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_partial_store_unimported, agc_driver_depth_storage_dispatch, and 4 more
VUID-vkCmdCopyImage-srcImage-01551 (12 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_buffer_memory, agc_driver_depth_plane_cpu_write_after_memo, agc_driver_depth_plane_cpu_write_after_page_unwatch, agc_driver_depth_plane_cpu_write_after_partial_unwatch, agc_driver_depth_plane_partial_store, agc_driver_depth_plane_partial_store_unimported, agc_driver_depth_storage_dispatch, and 4 more
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (1 test): agc_driver_srgb8_color_target
Skipped (60)
agc_driver_bda_reads_shadowed_results: skipped, the device reports watched host memory written when it imports or reads it
agc_driver_bda_reads_shadowed_results_per_build: skipped, the device reports watched host memory written when it imports or reads it
agc_driver_copied_writer_label_order: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_copied_writer_label_order_drain: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_float16_unused_nan_payloads: skipped, the device has no native f16 conversions with RTE rounding and kept f16 denormals
agc_driver_image_table_vouched_lookups: skipped, the device reports 4 of 4 host-imported pages written after a GPU read, so a watched imported table cannot be vouched for
agc_driver_linear_target_direct_write_completion: skipped, the device does not keep host imports write-watched, so no completion stamp over the target is tracked
agc_driver_ngg_vertex_wave_reduction: skipped, s3 describes a 32-lane wave only on 32-lane subgroups, and the device has 64 lanes
agc_driver_scattered_runs: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_sparse_dma_buf_import: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_unit_shadow: skipped, the device does not import watched host memory for unit shadows
agc_driver_unit_shadow_budget: skipped, the device does not import watched host memory for unit shadows
agc_driver_vertex_elected_atomic_max: skipped, the device runs vertex programs in subgroups, so they keep their cross-lane reductions
agc_host_import_sparse: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_idle_publish_wait: skipped, the device reports 4 of 4 host-imported pages written after a GPU read, so a forced watch of the import reads its submissions as CPU stores
agc_import_settle_initial_dcc: skipped, the device has no deferred settle of host imports: imports under 1 MiB wait for one only on Windows
agc_shadowed_key_stores: skipped, the device imports no watched host memory under a unit shadow
agc_small_tiled_shadow_label: skipped, host imports are unavailable: the test block is not imported and watched
agc_storage_clear_hand_over: skipped, the device imports no watched host memory for the clear-key hand-over
agc_unit_shadow: skipped, the device does not import watched host memory for unit shadows
agc_unit_shadow_completion_store: skipped, the device reports watched host memory written when it imports or reads it
guest_http_uri_locale: 
guest_net_ctl: skipped, the host has no IPv4 address
guest_video_out_color_space: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_hdr: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_high_frame_rate: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_output_mode: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_resolution_status: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_wqhd_detection: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
video_out_flip_aftergpu: skipped, no display or Vulkan device: DisplayWindow: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
video_out_flip_backtoback: skipped, no display or Vulkan device: DisplayWindow: Vulkan support i
Reproduce
git fetch origin 61546e69320d5ecca1851f3c5a9f4298c590e4d1 4f41f4c375f5abb487dbd184fbc150f31994d762
git checkout 4f41f4c375f5abb487dbd184fbc150f31994d762 && git merge --no-ff 61546e69320d5ecca1851f3c5a9f4298c590e4d1
git submodule update --init --recursive
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_ENABLE_SPIRV_TOOLS=ON
cmake --build build && cmake --build build --target libs
VK_DRIVER_FILES=/nonexistent/icd.json ctest --test-dir build -E '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 12 --output-on-failure
ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation \
  ctest --test-dir build -R '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 16 --output-on-failure
# failures are then re-run alone: --timeout 360 --parallel 2; new validation IDs: the test alone, twice, --parallel 1

Automated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.

@github-actions github-actions Bot added review queue Next in the maintainer review queue: checks pass, no open change request, no conflict libkernel build needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed review queue Next in the maintainer review queue: checks pass, no open change request, no conflict labels Nov 3, 2026

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

Labels

build libkernel needs rebase Conflicts with main, rebase needed unblocks title Names a title ID on the Unblocks line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants