Repository navigation
fix(libkernel): report a 64 KiB st_blksize for directories - #12922
Conversation
A PS4 Pro (firmware 10.71) reports st_blksize 0x10000 from stat and fstat for /app0, its subdirectories and the system library directory. Titles size one getdents buffer from it: CUSA18659's CryPak reads st_blksize + 264 bytes once, so the 512 reported here cut its shader directory listing short. Directories, app0.zar ones included, now report 0x10000; /data directories (0x8000 on the console) and files are recorded as debt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
|
|
Windows run of this head (
AI-assisted: yes (Claude Code). |
rooklz
left a comment
rooklz
left a comment
There was a problem hiding this comment.
Host-side check of head da7581eec4 on main bb04a4f. I did not run anything on a console.
What I ran (macOS arm64, Apple clang, ASan+UBSan)
check_conventions.py --base upstream/main: passed.git diff --check: clean.git merge-treeagainst bb04a4f: clean.pr_overlap.py --branch: 1451 open PRs, no textual conflicts. The only ones it could not compare are old PRs that already conflict with main and touchlibkernel.md/GuestFilesystem.cpp/GuestKernelAppArchive.cpp. #11625 touchesArchivedFile.cppbut notFillNode's last line.clang++ -fsyntax-onlyonNativeStat.cppandArchivedFile.cpp: clean.- I built a standalone harness:
NativeStat.cppfrom the PR, unchanged except the macOSst_atimespecfield rename on the host struct reads, plus stubs for the archived and directory-descriptor helpers. It has 12 rows, each against the host's ownstat:- These must be 0x10000: directory through
TryFillFileStat,FillLinkStaton a directory,FillFileStatFromDescriptoron anO_DIRECTORYfd,FillFileStat(int), andstatthrough a symlink to a directory. - These must keep the host value:
lstatof a symlink to a directory, a regular file by path, through a symlink, and by fd, a pipe, a unix socket, and/dev/null. - Result: the PR head passes 12/12. With main's
NativeStat.cpp, exactly the 5 directory rows fail.
- These must be 0x10000: directory through
Mutants of the CopyNativeStat line
| Mutant | Harness | PR's own test rows |
|---|---|---|
| M1: always 0x10000 | killed (6 rows) | survives: only directories are asserted |
M2: st_mode & 0040000 bit test (also matches sockets) |
killed (socket row only) | survives |
| M3: every non-regular file | killed (symlink lstat, /dev/null, socket) |
survives |
Optional rows to close the gap:
- A regular-file row next to the new ones kills M1. For example,
sceKernelStat(presentName.c_str(), &fileStatus) == 0 && fileStatus.st_blksize != 0x10000. On Windows the code fixes this at 512. On APFS and ext4 the host reports 4096. - M2 and M3 leave regular files alone, so only a non-regular, non-directory descriptor kills them. In my harness the socketpair fstat (8192 on macOS) kills both;
/dev/nulland anlstatof a symlink also kill M3. The existingGuestSocketPair.cpp:203row checks onlyst_blksize != 0. Both kinds of row are optional.
Notes (not blocking)
- On macOS,
/dev/nullreportsst_blksize65536 from the host, so a "not 0x10000" check is only valid for regular files, not devices. - The mode test is applied after the copy, so
lstatof a symlink to a directory correctly keeps the host value (S_IFLNK). The 0140000 socket paths (Udp.cpp:943) do not go throughCopyNativeStat. - The debt entry already records the evidence limits: the measurement is from a PS4 Pro on firmware 10.71, not a PS5, and
/datareports 0x8000 there.
|
@rooklz Thanks for the harness and the mutants. 8604e22 adds the regular-file row next to the directory ones: @JJenkx Thanks for the Windows run and for tracking the line-1552 failure to #12820; it is not hit on this machine's build (26200). AI-assisted: yes (Claude Code) |
As rooklz suggested in review: a row for a regular file next to the directory rows, so a change that reports 0x10000 for every stat fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Static review plus an independent Linux run (CachyOS, kernel 7.2.10, GCC 16.2.1, Ninja 1.13.2, Release, What I checked
Focused test run
Not an AGC/shader/VideoOut change — no paired validation-layer run required. |
rooklz
left a comment
rooklz
left a comment
There was a problem hiding this comment.
Re-review of head 8604e227c9.
git range-diff:da7581eec4is unchanged. The new commit adds one regular-file row, the one I suggested:sceKernelStat(presentName) ... st_blksize != 0x10000at GuestFilesystem.cpp:1432.present.txtis created at L1037 and nothing removes it before this row (the only unlink near it targetsviaMissingand asserts thatpresentstill exists).- Mutant M1 (always 0x10000) is now killed by the PR's own rows. A regular file gets 512 from the Windows branch, NativeStat.cpp:102, and the host value on POSIX (4096 for a fresh file on APFS here; ext4/xfs/btrfs/tmpfs are also 4096). M2 and M3 (socket / other non-regular) still survive. I had marked them optional, and as you note the socket paths do not go through
CopyNativeStat. That leaves/dev/nulland an lstat of a symlink, which remain optional. - One small caveat, not blocking:
!= 0x10000depends on the host. A POSIX filesystem whose preferred I/O size really is 64 KiB (some network or ZFS setups) would make the row fail even though the code is correct. CI and dev hosts use 4096/512, so this is fine in practice. - Checks:
git diff --checkclean;git merge-treevs upstream mainbb04a4f1eaclean;check_conventions.py --base upstream/mainpasses; CI windows + conventions pass, other lanes skipped (path-filtered).
LGTM.
AI-assisted: yes
|
A Windows run with GCC 16: Windows 11 Pro 10.0.22631, i9-9900K, MSYS2 UCRT64 GCC 16.1.0, Release,
AI-assisted: yes (Claude Code) |
|
At |
CopyNativeStat keeps the host block size for regular files on POSIX, so a 64 KiB host (ZFS, some network mounts) makes the st_blksize != 0x10000 assertion fail with correct production code. Compare against the host's actual st_blksize (POSIX) or the 512 Windows convention, which still catches an unconditional directory-size assignment spilling onto files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean good catch — fixed at The regular-file row now reads the host's own Latest AI-assisted: yes (Claude Code) |
What
statandfstatreportedst_blksize512 for directories on Windows. A PS4 Pro (firmware 10.71) reports 0x10000 for/app0,/app0/sce_sys,/app0/sce_moduleand the system library directory, through bothsceKernelStatandsceKernelFstatof a directory opened withO_DIRECTORY. Titles size their directory reads from it: Crysis Remastered (CUSA18659)'s CryPak allocatesst_blksize + 264bytes and callsgetdentsonce, so with 512 it listed only 33 of the 54 entries inengine/shaders, missed the shader flag files listed after them, and drew characters' skin and eyes black because those shader lookups then missed its cache.Directories now report 0x10000,
app0.zardirectories included. The console reports 0x8000 for/datadirectories, which the host paths do not tell apart, and/app0files report 0x10000 where files here keep the host's value; both are in the newlibkernel.mdentry.Console rows (probe buffer prefilled with 0xAA, read at FreeBSD 9 offsets):
Tested
Windows 11, WinLibs GCC 15.2, NVIDIA GeForce RTX 2080, on
mainbb04a4f:guest_filesystemchecksst_blksizeof a directory throughsceKernelFstatandsceKernelStat, andguest_kernel_app_archive_archivechecks anapp0.zardirectory; with the previousNativeStat.cppandArchivedFile.cppboth fail, with this change all fourguest_filesystem/guest_kernel_app_archive_*tests pass 3 of 3.python tools/check_conventions.py --base origin/mainpasses. Crysis Remastered (CUSA18659) lists all 54 entries and draws faces with this change on a local branch; not run on currentmain, where the title does not boot yet.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)