Repository navigation
Port upstream relinker fixes (slice C), incl. Rosetta 256-bit store split - #436
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f0bca23)
(cherry picked from commit d700567)
(cherry picked from commit d3fc14a)
…hing The guest identity predicate accepted .debug_prx while Domain::ImportModule did not: a dependency named libGuest.debug_prx normalised to libGuest_debug_prx, so a qualified import declared in module libGuest was left unscoped. When another guest module exported the same symbol, the conversion failed with an ambiguous guest import, while .prx, .sprx and .suprx resolved to the declaring module. Module suffixes are now one list in the domain layer, used by the identity predicate, import module normalisation and the default .prx suffix. Needed-name stem aliasing keeps its own list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 7c1b64b)
… suffix 0F 0F was decoded as a two-byte instruction. 3DNow! instructions are 0F 0F ModRM [SIB] [displacement] imm8, the trailing byte selecting the operation (AMD64 Architecture Programmer's Manual Volume 3, Appendix A, 3DNow! opcodes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VZWLdTjf1S57iLgn2rcaFb (cherry picked from commit feb19a8)
(cherry picked from commit 409f53e)
…truction info DecodeInstruction did not treat 0F 0F as having a ModRM byte, so a RIP-relative 3DNow! operand was not reported to the analyses and the --to-intel stub relocation that read HasRipRelativeDisp. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VZWLdTjf1S57iLgn2rcaFb (cherry picked from commit 36c78ba)
(cherry picked from commit 0a324d8)
(cherry picked from commit 72d5b8c)
(cherry picked from commit 63a2764)
(cherry picked from commit 5eb22de)
…on symbol OpenOrbis create-fself adds one to the symbol index of every relocation it copies from .rela.dyn, so each R_X86_64_RELATIVE entry of a homebrew built with the OpenOrbis toolchain names symbol 1, a local STT_SECTION symbol with value 0. FreeBSD rtld computes RELATIVE as base + addend without reading the symbol. The pipeline, the code analysis and the strict filter now accept index 0 or a STT_SECTION symbol with value 0, and still throw for any other symbol. The relocation is written with symbol index 0, where the pipeline previously dropped RELATIVE entries with a nonzero index. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit cded9ba)
Rosetta 2 ends the process when an unaligned 256-bit store crosses into
a page the store may not write ("rosetta error: unexpectedly need to
EmulateForward on a synchronous exception") instead of raising the
fault. A store inside such a page, and a 128-bit store across one,
fault normally. The macOS write watch keeps watched pages read-only
between collections, so a guest that stores 32 bytes across the start
of a watched page ends the process.
--to-rosetta now lowers VMOVUPS, VMOVUPD and VMOVDQU with a 256-bit
memory destination to a stub that stores the low half with the same
instruction at VEX.L=0 and the high half with VEXTRACTF128 at the
operand plus 16, so each half faults like any other store. RIP-relative
operands are relocated from the instruction's place in the site. A
store whose upper half has no 32-bit displacement fails the conversion.
Aligned stores (VMOVAPS, VMOVAPD, VMOVDQA and the non-temporal forms)
cannot cross a page and are left alone.
Every store is lowered. A 4-byte store has no room for the jump to its
stub, so its stub also takes one neighbouring instruction. The
following one: a return ends the stub, a call pushes its original
return address and jumps, a jump or conditional branch is re-encoded
with a 32-bit displacement, and another 256-bit store is lowered in the
same stub. Otherwise the preceding one, unless it is a call, a jump, a
return or a trap. Register calls, LOOP, JRCXZ and branches with a
prefix that changes them do not move. When neither neighbour can move,
the store becomes a short jump to an island: a run of movable
instructions within reach is moved to its own stub, and the bytes it
frees hold the jump to the store's stub. A store with neither fails the
conversion.
The converter tests compare the halves with clang's encodings for base,
SIB, disp8 overflowing to disp32, disp32, RSP-relative, FS-absolute,
r13-based and RIP-relative operands, and check that aligned stores,
loads, register forms and masked stores are not split. They check the
stub of a short store followed by a return, a call (direct, with an
address-size prefix, and through a RIP-relative pointer), short, near
and RIP-relative jumps, short and near conditional branches and another
store; that a following register call, LOOP, JRCXZ, prefixed branch or
CLWB makes the store take the instruction before it; islands past
instructions already moved, a store that needs its own stub, a branch
target, a register call and the short-jump reach; and that a store with
no room fails.
PPSA12544: 55295 stores split in the executable, 278 of them in the
stub of the store before them; every module converts with none kept.
AI-assisted: yes
(cherry picked from commit c763a78)
…ts stub A short fs-prefixed TLS access is patched with a 5-byte jump by relocating the following instructions into the stub, but any branch there was rejected outright. NHL 27 hits this on every stack-protector epilogue: fs:xor rax,[rbx] immediately followed by jne __stack_chk_fail. Allow relative conditional branches and short jumps: re-encode rel8 forms as their rel32 equivalents, rebase the displacement when the stub is emitted, and keep rejecting loop/jrcxz, calls, returns, and segment-prefixed followers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit c6f934a)
(cherry picked from commit 341dafb)
(cherry picked from commit e2c24cf)
(cherry picked from commit 4b6bb2f)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 70f03dc)
A relocation whose dynamic symbol entry is all zero, like the null symbol, now takes the value 0 plus its addend, as the gABI defines for STN_UNDEF. The Linux output imports such an entry as an unnamed weak symbol, which the host loader resolves to 0. The Windows and macOS outputs write the value into the slot instead of failing with "Empty import symbol name". Such references are left out of the call registry. An unnamed symbol entry that is not all zero still fails the relink. OliSe Player (PPSA01153) has seven of these entries and calls through the slots only when they are not 0 (boykopovar#4437). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 1205d03)
… instructions The AMD-only converter moves the instructions next to a short AMD-only instruction into its stub, and refuses to move one that a branch enters. It knew only direct jump and call targets, so a switch case entered through a jump table could land inside the jump written over the moved instructions. The converter now also reads relative jump tables: a RIP-relative LEA whose address lies in a loaded segment starts a table of 32-bit offsets from that address, read while each target starts an instruction of the segment being converted. To read them, Convert takes the program headers and converts their executable load segments; the relinker passes all headers, and ElfReader::ReadCodeSegments, now unused, is removed. The converter test builds a dispatch through such a table whose second case enters the instruction after a 4-byte 256-bit store, and checks that the store takes the instruction before it instead. It also checks that reading stops at an entry that does not start an instruction, that only a LEA forms a table address, that tables are read only inside loaded segments, that only executable load segments are converted, and that a segment past the end of the file fails the conversion. PPSA12544: one 256-bit store in Il2cppUserAssemblies moved the instruction after it, which is case 1 of a switch; it now goes through an island. The other relinked files are unchanged. AI-assisted: yes (cherry picked from commit 1541244)
(cherry picked from commit 33c955a)
(cherry picked from commit c096721)
Weak undefined imports in the input ELF became STB_GLOBAL in the relinked output, so a title that weak-imports a function no module exports stopped at startup instead of binding the import to null. Keep STB_WEAK on the relinked dynamic symbols. On Windows, write 0 plus the addend to the slot of a weak import that no searched module exports. Fixes boykopovar#4497 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit cb1c18d)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit bdf003a)
PS4 executables keep their relocated read-only data, GOT and init/fini arrays in a PT_SCE_RELRO segment outside every PT_LOAD. The segment filter dropped it, so Windows and macOS output stopped at the first relocation into it, and Linux output placed the relinker's extra block over its addresses without an error. The ELF reader now reports such a segment as a read-write PT_LOAD. A PT_SCE_RELRO inside a PT_LOAD keeps being skipped, and one that partially overlaps a PT_LOAD throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d0502b4)
(cherry picked from commit faf1cd7)
The upstream change erases the address from a hash set; the fork's visited set is a per-byte bitmap and enqueue() already checks reachable, so the erase is unnecessary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Amd64OnlyInstructionMatcher now references DecodeVexWideStore and WideStoreLowering; add WideStoreOperands.cpp and WideStoreLowering.cpp. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 789b715124
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!ripRelative && highDisplacement > std::numeric_limits<std::int32_t>::max()) | ||
| throw CodegenException("256-bit store displacement leaves no room for its upper half"); |
There was a problem hiding this comment.
Handle 32-bit address displacement wrapping
When a wide store has a 67 address-size override, its disp32 is a 32-bit address component and adding 16 for the upper half wraps modulo 2^32 rather than being constrained to signed 32-bit range. For example, 67 C5 FC 11 04 25 F0 FF FF 7F validly stores its halves at 0x7ffffff0 and 0x80000000, but this check throws 256-bit store displacement leaves no room, so --to-rosetta rejects valid input. Track the address-size prefix and encode the upper displacement with 32-bit wrapping in that mode.
Useful? React with 👍 / 👎.
Brings in the relinker fixes (#436, #439). No conflicts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
Upstream port, slice C (relinker / codegen / elfpatcher). Refs #415.
Ported (cherry-picked with -x, oldest first)
All 26 non-merge upstream commits touching
core/relinkerin2ccbd376f..upstream/main: f0bca23, d700567, d3fc14a, 7c1b64b, feb19a8, 409f53e, 36c78ba, 0a324d8, 72d5b8c, 63a2764, 5eb22de, cded9ba, c763a78 (split unaligned 256-bit stores for Rosetta), c6f934a, 341dafb, e2c24cf, 4b6bb2f, 70f03dc, 1205d03, 1541244, 33c955a, c096721, cb1c18d, bdf003a, d0502b4, faf1cd7.core/codegenandcore/elfpatcherhave no separate upstream changes in range (they live undercore/relinker).Conflict resolutions (fork behaviour kept)
Amd64OnlyInstructionMatcherand the substitution table keep the fork's CPUID stub (RosettaCpuid) next to the new wide-store lowering, in both_matchRosettaand the refactored_emitOutOfLine.--macos --to-rosettapath unchanged.docs/user/USAGE.mdtakes the upstream wording.symbolInfolambda are kept; weak imports now emitSTB_WEAKthrough it.visited.erase(va)does not apply to the fork's bitmap visited set (enqueue already checksreachable), so it is dropped.Skipped
None of the commits. Open upstream PRs (boykopovar#4340, boykopovar#4593, boykopovar#4802, boykopovar#4801, boykopovar#4501) not ported: not reviewed for this chunk, left for a follow-up.
Tests
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_RELINKER_ONLY=ON -DCMAKE_OSX_ARCHITECTURES=x86_64then build andctest -j4: 124/124 passed (C++ converter/scanner tests and the python pytest-style relinker tests, including the new weak_imports, sdk_mman_syscall and wide-store converter tests). The Rosetta libs configuration (-DANYPS5_ROSETTA_LIBS=ON) does not register the relinker ctests, so the relinker-only x86_64 configuration was used. No game smoke.🤖 Generated with Claude Code