Skip to content

fix(libc): rename over a dangling symbolic link on macOS - #9911

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
tkcm01:fix/libc-rename-dangling-symlink-macos
Oct 24, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
tkcm01:fix/libc-rename-dangling-symlink-macos

Conversation

@tkcm01

@tkcm01 tkcm01 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What

RenamePath_nid_no_patch compares source and destination with fs::equivalent and clears ENOENT from it, so a destination that does not resolve counts as "not the same file". libc++ reports errc::not_supported instead of ENOENT when either path does not resolve, so on macOS rename of a file over a dangling symbolic link returned EIO (5) instead of replacing the link. guest_filesystem stops there (GuestFilesystem.cpp:871 on main: rename_nid_postfix(replacement, dangling), "Filesystem check failed … (guest errno 5, host errno 5)").

A path that does not resolve cannot be the same file as the other, so not_supported is cleared the same way, on macOS only. Linux and Windows are unchanged.

The same guard is part of the macOS ports in #7201 (@marion909) and #8658 (@iamthehimansh), both credited as co-authors; this splits it out as a small change, as for the rest of the macOS series on #1.

Tested

macOS 27.0.1, Apple M4, Apple clang 21, x86-64 build under Rosetta 2, no Vulkan loader. The libraries do not link on macOS from main alone, so the run is on mugurc's ci/macos-series-2 (53db0ba, main 3fd1cc3) with #7524, the code commits of #7937 and the libSceSsl change from #7332, as described on #1.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

🤖 Generated with Claude Code

RenamePath compares source and destination with fs::equivalent and
treats ENOENT from it as "not the same file". libc++ reports
errc::not_supported instead when either path does not resolve, so on
macOS renaming a file over a dangling symbolic link failed with EIO
(guest_filesystem). A path that does not resolve cannot be the same file
as the other, so not_supported is cleared the same way on macOS. The same
guard is part of boykopovar#7201 and boykopovar#8658.

Co-Authored-By: marion909 <103169397+marion909@users.noreply.github.com>
Co-Authored-By: iamthehimansh <67262056+iamthehimansh@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tkcm01 tkcm01 mentioned this pull request Oct 22, 2026
@MarcZX10

Copy link
Copy Markdown
Contributor

The Windows relinker CI run on this head built successfully but CTest reported 234/235 passing; only conventions_function_bodies_shard_1 timed out at the 120-second limit. Could you rerun that shard (or the relinker CTest suite) and post the result? The other tests passed, so this may be load-sensitive, but the current hosted run is still red and needs a confirming result. AI-assisted review.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

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/Rename.cpp
PR 7201 core/libs/prx/libc/src/Rename.cpp
PR 9693 core/libs/prx/libc/src/Rename.cpp

@MarcZX10

Copy link
Copy Markdown
Contributor

The macOS change is guarded at the equivalent error mapping, leaving Linux and Windows paths unchanged; the source check and diff whitespace check are clean on head 03d5065995d8cb1938dca4a493a13e5297fbf10e. The author reports the focused macOS filesystem regression passing and no new failures in the broad run. One hosted check remains red: the Windows relinker CTest run built successfully but ended 234/235, with only conventions_function_bodies_shard_1 timing out at its 30-second per-test limit (the workflow command has an outer 120-second timeout). A prior comment already asks for a rerun; please post its result when available. No conclusion about product builds follows, since those jobs were skipped.

@tkcm01

tkcm01 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. I can't rerun jobs here, so I reran the same Build workflow on my fork for the unchanged head 03d5065995: relinker (windows-latest, g++) passed 235/235, with conventions_function_bodies_shard_1 passing in 7.58 s (job). The earlier 30 s timeout looks load-related; this PR only touches Rename.cpp.

AI-assisted: yes

@tkcm01

tkcm01 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@MarcZX10 rerun of the relinker suite on head 03d5065995 (merged onto current main), GitHub Actions on my fork, same Build workflow (full=false):

  • relinker (windows-latest, g++): 235/235 passed; conventions_function_bodies_shard_1 passed in 7.3 s.
  • relinker (ubuntu-latest, g++): 240/240. relinker (macos-latest, clang++) and the x86_64 variant: passed, the shard in 3.1 s and 1.9 s.

run. So the earlier 120 s timeout looks load-related; this change doesn't touch the relinker or the conventions tooling.

AI-assisted: yes

@harisibrar1 harisibrar1 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.

Checked against main 3875f8c, FreeBSD rename(2) and libc++ filesystem/operations.cpp (__equivalent):

  • FreeBSD's rename doesn't follow the destination's final component, so replacing a dangling link is the expected result.
  • libc++ equivalent reports errc::not_supported whenever either path's stat doesn't give an existing file, which explains the EIO on main. Clearing it is safe here because destinationExists already comes from symlink_status, and any real failure surfaces from the actual rename.
  • Optional nit: this is libc++ behaviour rather than Apple's, so #ifdef _LIBCPP_VERSION would describe the guard more precisely. Fine as is for the macOS series.
  • The branch still merges cleanly with current main.

Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.

@maximilianfv

Copy link
Copy Markdown
Contributor

Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)

Applied at 03d506599 to mugurc's macOS series (27e968974) with other open macOS fixes, then toggled only this PR. Test object rebuilt at -O0 -g, breakpoint on abort:

guest_filesystem stops at
Without this PR GuestFilesystem.cpp:847 on the series: rename_nid_postfix(replacement, dangling) == 0, the check this PR describes
With this PR That check passes; it now stops later, at :1262 (:1294 on main): sceKernelGetdents on a plain file does not return 0x80020016

The later stop is a separate gap that this PR does not claim to fix. I found no open PR for it.

AI-assisted: yes (Claude Code applied the PR, ran the tests and wrote this report).

@tkcm01

tkcm01 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @maximilianfv. That later stop is in the series, not main: the macOS sceKernelGetdirentries in File/src/Stdio.cpp on ci/macos-series-2 opens openat(fd, ".", O_DIRECTORY), which fails with ENOTDIR for a regular file, so sceKernelGetdents returns 0x80020014 instead of EINVAL (0x80020016). I reported it to @mugurc on #1, since it needs a change in the series rather than a PR against main. With both fixed, guest_filesystem passes on the M4.

@harisibrar1 good point on _LIBCPP_VERSION. I'd keep __APPLE__ for now to match the other macOS guards from #7201/#8658, but I'll switch it if a maintainer prefers.

AI-assisted: yes

@oneandonlydean oneandonlydean self-assigned this Oct 23, 2026
@oneandonlydean
oneandonlydean merged commit e512cd5 into boykopovar:main Oct 24, 2026
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants