Repository navigation
fix(relinker): reject collisions between generated outputs - #4802
Conversation
…oykopovar#4501) (#439) * Port upstream boykopovar#4340 (GNU hash tables) by @ArjenS-LA Ported from boykopovar/AnyPS5 PR boykopovar#4340 (author @ArjenS-LA), applied as a patch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Port upstream boykopovar#4802 (output collisions) by @ArjenS-LA Ported from boykopovar/AnyPS5 PR boykopovar#4802 (author @ArjenS-LA), applied as a patch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Port upstream boykopovar#4501 (SELF skip warning) by @devavijitsingh Ported from boykopovar/AnyPS5 PR boykopovar#4501 (author @devavijitsingh), applied as a patch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Adapt boykopovar#4802 port: hoist registry paths so the collision check runs before registries are written Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Windows test: head AI-assisted (Claude Code). |
|
@harry-z-kang: #4916 introduces I checked #4802 at I propose #4802 first, then including This is source inspection and a tree-only merge preview; I have not built or run the combined tree. AI-assisted: OpenAI Codex. |
CnRJay
left a comment
There was a problem hiding this comment.
Rejecting outputs that resolve to the same canonical path, including case variants on a case-insensitive directory, is useful: --executable and --module-dir can produce those names. I'm less sure about the hard-link part, which accounts for most of the Windows code.
- The scenario is rare. It is a hard link that someone created between two future output names before converting. It is worth guarding against, but it costs a lot here: during the preflight the converter creates
.anyps5-output-identity-*directories and hard links in the user's output directories, and it refuses read-only outputs with a new error. - Windows has a direct identity check.
GetFileInformationByHandleon a handle opened withFILE_READ_ATTRIBUTES(andFILE_FLAG_BACKUP_SEMANTICS) givesdwVolumeSerialNumberplusnFileIndexHigh/nFileIndexLow, which identify the file regardless of the libstdc++ permission emulation that breaksequivalent. It needs no write access, so read-only outputs need no special case, and it creates nothing on disk. - The case-sensitivity probe could likewise be replaced by comparing the identity of the existing parent directory, or skipped when the names differ only in case and the target doesn't exist yet. Treating that as a collision is the conservative choice and matches the default Windows behaviour.
- #5018 overlaps. It writes outputs on Linux and macOS to a temporary file and renames it over the target. After that, a hard-linked output is replaced name by name, and the overwrite described here no longer happens on those hosts.
As ArjenS-LA notes, #4916's .needed.json companions also need to go into outputPaths, whichever lands first.
|
Rebase handoff — rebased onto current Three follow-throughs beyond the raw conflict markers: main's Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: Head is |
c6feebf to
6937530
Compare
|
Rebased and updated at @blawesom, thanks for the handoff. I retained its integration with @CnRJay, I checked the design points against current source:
Validation on Windows x64 / supported WinLibs GCC 15.2.0: both before/after regressions reproduced the intended failures; all 57 hard-link pairs plus direct-path and identity controls pass after the fix. Release relinker build passes; the selected CTest suite has 145 passes, one explicit long-path skip, and no failures. Nine unrelated Git-commit-producing tooling tests were excluded under my local commit limit. Conventions passes on the exact new commit. Native Linux/macOS, full-library, title, and GPU execution were not run locally for this revision. The refreshed audit covers 433 open PRs and all 27 shared-file candidates at the exact heads in the updated description. Seven pairs are clean; twenty remain inconclusive because the other PR already conflicts with main. No new pairwise conflict was reported. AI-assisted: OpenAI Codex. |
|
macOS data point, plus one gap on case-insensitive APFS. Apple M2 Max, macOS 27.2 (26B5091g), AppleClang, Ninja Release, relinker-only. Head
Gap: the case-variant comparison in
So the PR catches the existing-file case on macOS (via Written with AI assistance (Claude Code). I ran the builds and tests above on my machine. |
|
You're right, I missed that rule. CONTRIBUTING says "The relinker uses only the C++20 standard library", so |
|
Checked the collision preflight against main: with Follow-ups outside this PR: a fresh case-only collision on case-insensitive macOS volumes (reported above) and |
What
Reject generated outputs that resolve to the same destination before writing executables, guest modules, registries, desktop files, icons, or Windows import diagnostics. Main can report success after one output overwrites another: for example, hard-link the executable and generated module destinations before conversion.
The preflight compares canonical paths and existing file identities. It includes every
--executableoutput and every executable/module registry. A secondary executable namedwindows-diagnostics-importswith a primary output namedoutput.txtalso produces the diagnostics path; this now fails before either output is written.On supported Windows MinGW,
filesystem::equivalentmisses hard links with different extension-derived synthetic permissions. The standard-library workaround compares temporary hard links with identical names in reserved sibling directories, then removes them before writing. Read-only identities are rejected before link creation. Existing case variants use actual identity; absent ASCII case variants use a parent-directory case-sensitivity probe on Windows. This does not provide transactional safety against concurrent filesystem mutations.Branch:
fix/relinker-output-collisions; fixed commit:69375305f500004f5648e683eb012323e6e9ad6b; baseline/current main checked:d8888df9b280a23077ff47d412bd7668c8c8b656.Tested
Windows x86-64, build 26200; supported WinLibs MinGW-w64 GCC 15.2.0 (
x86_64-ucrt-posix-seh, r7), Python 3.13.7, CMake 4.3.1, Ninja 1.13.2. Release relinker-only builds; actual filesystem conversion fixtures, no title or GPU testing.d8888df9b280a23077ff47d412bd7668c8c8b656: executable/module aliasing incorrectly returns 0. The identical test passes with the fix.1d9394dc3739d6195951cd71e9ed6496446848danormalized onto the same main: primary/secondary executable aliasing incorrectly returns 0. Its binary was preserved before the fix. The identical test passes with all executable outputs registered.input_magic,output_collisions, andsecondary_output_collisions. Selected suite: 145 passed, 1 skipped (long_paths, Windows long paths disabled), 0 failed, 27.49 seconds. Nine unrelated tooling tests were excluded because they create disposable Git commits under the local daily commit limit; the exclusion expression below lists them.git diff --checkand committed-objecttools/check_conventions.py --base origin/mainpassed. Local results describe the exact fixed commit above; upstream CI is reported separately.Commands with WinLibs
binon PATH and a working Python interpreter:Integration
Refreshed all 433 open PRs across five pages with complete changed-file lists, and inspected all 27 shared-file implementations. Tree-only merge previews normalize exact heads onto main
d8888df9b280a23077ff47d412bd7668c8c8b656, then compare with that explicit merge base. No temporary commits were created. No duplicate output-collision implementation was found. Seven pairs merge cleanly; twenty are inconclusive because the other PR already conflicts with main. No new pairwise conflict was reported. Exact checked heads and pre-existing main-conflict paths are recorded below.#4916 at
22b190ae081c5e01119a1f6b3559b64754b18df9merges cleanly but introduces a semantic integration requirement: append itsneededPathstooutputPathsbefore the collision loop, and extend collision coverage to.needed.jsoncompanions when the second PR lands. Its feature is not included in this PR. #5018 at26c62cc63d404d81392f9f4ab004d3a6cd29fe2eis complementary: Windows, its fallback writes, and desktop/icon writes still need collision protection.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)Exact overlap audit
67452b252f041456d3b607c3cdce02d98b41bef01f466a8f09e73e8dbbc4fd1d89aea152ce10127c1a384f3769510cb8543b6536165748e5d4f5071fdba9140ff98325ff177f67e0d262957a80bdf2fa7b77941beb4d08d589d3543a23cad9107f6f35f022705f1f080cffa143b49967f16559d6116692b9a93a16168870d40080b4924191201702676ccadf8f9544978c3081b76ee96547358d4eeb5e12984850db14a2e895492f68ec9fd3d97a2bd599439ddf7010bb3101adfdfe021a1181836e63581db7e410e5ba22ce3338e25b57bd874a670ded9a333324de3625741d1cf6f05897aea07658c9f4f7296493f4c25a5783e51db770884e90c416c2614f2ef58791766e0a908207bf6ab66e36dfa14e32524f89d18b007d30827d368e4c0e447d2b004a87f6add78dd967bcf02e9e3c9056bc493bf23d2df17b984139b7b22bd245f1f729afb9ac0fb566c8d4ff75f67e040f7e237338d539283973584628167b7a6ac40dbad83b44515937fe992c1dd19d4c440e54950b38b222b190ae081c5e01119a1f6b3559b64754b18df94fc8ef602bc15fdf19cc1d97ff76b6b51b8ddd005f564173209aec77c82051ada9399066941d1eb89954e58af95a3ae9bc315f56550adbd4ae883e7ac37a4b8b6b85d94cb9bcc6c75b8b86905ebf3c290093322ba1810dbdf06f5115c0b75ac4fab8d4c0a8ed810d0785d9acd6930d09b1fe0448e4876a668b400afc5d5635e5af803c10b5725d7f4405ede7Pre-existing conflicts with main for #3891, #4156, #4396, #4397, #4422, #4423, #4424, #4520, #4521, #4522, #4523, #4566, #4599, #4718, #4740, #4749, #4760, #4813:
CMakeLists.txt,core/libs/prx/libSceAgcDriver/PostCompile.cmake,core/libs/prx/libSceFiber/Export.cpp,core/libs/prx/libSceVideoOut/CMakeLists.txt,core/libs/prx/libSceVideoOut/src/VideoOutDriver.cpp,core/libs/prx/libc/src/GuestAllocations.cpp,core/libs/prx/libkernel/AppMetadata/src/ParamJsonParser.cpp,core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp,core/libs/prx/libkernel/System/src/SignalSets.cpp,core/libs/tests/GuestMemory.cpp,core/libs/tests/GuestSignalSets.cpp,docs/dev/debt/libkernel.md,docs/dev/debt/relinker.md,docs/dev/debt/shader.md.Pre-existing conflicts with main for #4593:
docs/user/USAGE.md.Pre-existing conflicts with main for #5021:
core/libs/prx/libSceFiber/Export.cpp,core/libs/prx/libSceLibcInternal/CMakeLists.txt,core/libs/prx/libSceVideoOut/src/VideoOutDriver.cpp,core/libs/prx/libkernel/AppMetadata/src/ParamJsonParser.cpp,core/libs/prx/libkernel/System/src/SignalSets.cpp,core/libs/tests/GuestMemory.cpp,core/libs/tests/GuestSignalSets.cpp,docs/dev/debt/libkernel.md.#5018 at
26c62cc63d404d81392f9f4ab004d3a6cd29fe2ewas separately normalized and compared with the same main base: clean tree-only pair preview. No combined build is claimed.