Repository navigation
fix(libkernel): return EBADF from fstat on a descriptor that is not open on Windows - #6897
Conversation
|
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.
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.
|
karimibrahim07
left a comment
There was a problem hiding this comment.
Windows check of head 6a402a145 merged onto main e3a8e4430 (clean merge). Windows 11 Pro (26300), Ryzen 5 3600, WinLibs GCC 15.2.0 r7 (UCRT), Ninja Release.
guest_filesystem,guest_dup,guest_fstatfs,guest_file_timestamps,guest_file_vectorand the sixguest_file_lock_*tests pass,--repeat until-fail:3.- Negative control: with
main'sNativeStat.cppand the new test kept,guest_filesystemfails at line 628,Require(!g_invalidParameter). The threesceKernelFstatreturn-value checks just above it pass onmaintoo: with the test's recording handler the UCRT returns and_fstat64fails withEBADFanyway. So the handler check is what makes this test fail onmain. That matches the description: only the fatal handler that the crash report installs turns this into the Dying Light 2 stop.
Code: WindowsDescriptorHandle_nid_no_patch is already how DescriptorControl.cpp checks descriptors (6 uses), so fstat now validates the same way as fcntl. The check comes after the directory-descriptor lookup, so directory descriptors are unaffected.
Overlap: the description says #6228 "edits the line above" in DoFstat. It deletes that line (the DirectoryDescriptorPath early return), so git reports a conflict between the two PRs in NativeStat.cpp. The resolution is to keep this PR's check and drop the directory line. One thing to check when that happens: after #6228, directory descriptors reach this check, so WindowsDescriptorHandle_nid_no_patch must return a valid handle for them, or fstat on a directory would become EBADF. I couldn't test the two together, because #6228 currently also conflicts with main in Stdio.cpp.
AI-assisted: yes. I ran the build, the tests and the negative control above.
…pen on Windows sceKernelFstat and fstat passed any descriptor to the UCRT's _fstat64, which raises the invalid-parameter handler for one that is not open. The crash report installs that handler as fatal, so the calling thread died where the console returns EBADF. Dying Light 2 (PPSA02262) hits it on a loading thread right after sign-in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6a402a1 to
4fe32a4
Compare
|
Rebased onto current |
What
On Windows,
sceKernelFstatandfstatpass the descriptor to the UCRT's_fstat64. For a descriptor that is not open, the UCRT raises its invalid-parameter handler. The crash report installs that handler as fatal, so the calling thread dies withFATAL: UCRT invalid parameter, where the console returnsEBADF.DoFstatnow looks the descriptor up withWindowsDescriptorHandle_nid_no_patch(which already silences the handler for that lookup) and fails withEBADFwhen it is not open, before calling_fstat64.sceKernelFstatthen returnsSCE_KERNEL_ERROR_EBADF(0x80020009) andfstatreturns -1 with errno 9, like the other descriptor calls the test already checks (fchmod,futimes,fsync,fdatasync).Dying Light 2 (PPSA02262) hits this on a loading thread right after its sign-in step:
Tested
Windows 11, RTX 3070, GCC 15.2.0 UCRT, Release, on main
66e290f1c.guest_filesystem: new checks thatsceKernelFstaton a closed file descriptor, a closed socket and0x7fffffffreturns0x80020009without raising the invalid-parameter handler (counted with a recording handler, asguest_dupdoes forclose). On main the handler is raised (the check fails at that line); with this change it passes.agc_driver_gpu_indirect_arguments_misaligned_writer_aheaddoes not compile with MinGW (alignment 65536 exceeds object file maximum 8192), andguest_kqueueexits with0xc0000409.tools/check_conventions.pypasses.Checklist
main; no other open PR implements the same functions (fix(libkernel): preserve directory identity across Windows renames #6228 edits the line above inDoFstat, a different change)NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository🤖 Generated with Claude Code