Skip to content

fix(relinker): do not take instructions another AMD-only stub moved as a short instruction's followers - #8001

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
mopga:fix/relinker-amd-only-claimed-followers
Oct 20, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
mopga:fix/relinker-amd-only-claimed-followers

Conversation

@mopga

@mopga mopga commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

What

A short AMD-only instruction takes the following instructions into its stub until its site fits a jump. That loop did not check which instructions an earlier 256-bit store island had already claimed, so with --to-rosetta two trampolines could cover the same bytes.

Reproduced with relinker --to-rosetta on current main (je to the RCPPS, jmp over nothing, vmovups [rdi],ymm0, rcpps xmm1,xmm2, four add rax,rcx, padding, ret):

74 06 EB 00 C5 FC 11 07 0F 53 CA 48 01 C8 ×4 90 ×8 C3

The store's forward window is blocked by the branch target and its backward window by the jmp, so it is lowered through an island that claims the adds. RCPPS then took the same adds as its followers: the converter returned trampolines at +8 (6 bytes) and +11 (6 bytes), and the conversion failed with AMD-only site bytes changed before patching (offset 0x1008). With RDPID (F3 0F C7 F8) instead of RCPPS the result is the same.

A claimed follower now ends the attempt: an optional instruction such as RCPPS, RSQRTPS, RDPID or RDTSCP is kept, and any other instruction fails with an error naming the moved follower instead of producing overlapping sites.

Tested

Windows 11, AMD Ryzen 7 7800X3D (Zen 4), MinGW-w64 GCC 15.2.0 (WinLibs r7), relinker-only build of this branch on current main:

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
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

🤖 Generated with Claude Code

…s a short instruction's followers

A short AMD-only instruction takes the following instructions into its stub until its site fits a jump. The loop did not check which instructions an earlier 256-bit store island had already claimed, so with --to-rosetta two trampolines covered the same bytes: for je; jmp; vmovups [rdi],ymm0; rcpps xmm1,xmm2 followed by adds, the RCPPS site at +8 overlapped the island at +11, the patched code crashed, and the real patchers failed with 'AMD-only site bytes changed before patching'. A claimed follower now ends the attempt: an optional instruction such as RCPPS or RDPID is kept, and any other instruction fails with an error naming the moved follower.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wachi35676

Copy link
Copy Markdown
Contributor

Verdict: confirmed on Windows and Linux, and it works together with #7999.

Environment: Windows 11 Home 10.0.26300, Ryzen 5 9600X (Zen 5), WinLibs GCC 15.2.0 r7, relinker-only Release build. This PR was merged onto main 9127ea3, alone and together with #7999.

  • Reproduction through the CLI: both of your sequences as the body of a test_tls_function_coverage.py make_image image, converted with relinker --skip-sce-module --to-rosetta. The RCPPS one is 74 06 eb 00 c5 fc 11 07 0f 53 ca (48 01 c8)x4 (90)x8 c3.
    • main: rc 2, Intel conversion: 0 in place, 3 stubs, 0 kept, FAIL: AMD-only site bytes changed before patching (offset 0x1248), for both RCPPS and RDPID.
    • This PR: rc 0, 2 stubs, 1 kept, for both.
  • Negative control: with only Amd64OnlyConverter.cpp reverted to main, amd64_only_converter_tests fails with A short instruction's site overlaps instructions another stub already moved.
  • ctest:
    • Windows: 212/212. Three tests that timed out under -j4 because of host load, and also do so on main, passed on a serial rerun.
    • WSL Ubuntu 24.04, GCC 13.3: 216/217. The one failure, shader_cache_runtime_sources, fails intermittently on main too.
  • With fix(relinker): move in-place AMD-only replacements into a short instruction's stub #7999 merged as well, the build merges cleanly and passes Windows 212/212 and WSL 217/217. In the combined build, your two cases are kept and the six in-place-follower cases from fix(relinker): move in-place AMD-only replacements into a short instruction's stub #7999 convert. fix(relinker): move in-place AMD-only replacements into a short instruction's stub #7999 changes MatchSequence and this PR adds the claimed check in the follower loop of _convertSegment. An in-place follower that fix(relinker): move in-place AMD-only replacements into a short instruction's stub #7999 moves into the stub goes into taken/consumed, not claimed, so the two changes don't interact.
  • Homebrew: PS5 OpenGL Showcase 1.0.2 (PPSA99005), PPSA47402, PS5LM Plus 0.8.4 (PPSA99581), and Prospero Win v0.2.0 (PPSA99995) with its 16 Wine modules. I relinked them with --windows --to-intel, --macos --to-rosetta and Linux --to-intel. These conversions produce real stubs: 1/5/22/0 under --to-intel and 4495/16745/51131/192 under --to-rosetta, for PPSA47402/PPSA99005/PPSA99581/PPSA99995 respectively, with 0 kept. All 12 output sets are byte-identical to main, both with this PR alone and combined with fix(relinker): move in-place AMD-only replacements into a short instruction's stub #7999. None of these stubs hit the claimed-follower case.

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

@github-actions

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 8042 core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp

@oneandonlydean
oneandonlydean merged commit b973181 into boykopovar:main Oct 20, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants