Repository navigation
fix(relinker): report the syscall number of a forbidden syscall instruction - #2206
Conversation
683a999 to
f8d73cd
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
The syscall-number lookback can report a stale value after an intervening instruction writes RAX. I reproduced this on the PR head by adding 48 83 C0 01 (add rax, 1) between the mov rax, 9 and syscall in kConstantStub, and expecting 10. The test fails: the scanner still reports syscall number 9.
ConstantSyscallNumber currently checks only FlowKind::Sequential before continuing backward; it does not check whether the intervening instruction changes RAX/EAX. Please stop the inference when an intervening instruction may write RAX (or accurately propagate the value for supported instructions), and add a regression for this case. The original test suite passes; the added local repro exposes the diagnostic error.
f8d73cd to
529476c
Compare
|
Windows run: Windows 11 Pro 25H2 (10.0.26200.9457), RTX 5060 Ti (NVIDIA 617.42), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release,
AI-assisted: yes |
529476c to
a43e2ac
Compare
|
#1935 is on main now and conflicts in |
adriadam10
left a comment
There was a problem hiding this comment.
Checked on Linux: syscall_scanner and syscall_number pass, and both fail with SyscallScanner.cpp reverted to main, so they pin the change. The lookback logic (constant load directly before the instruction, or one mov r10, rcx in between, otherwise no number) looks right, and the rax-clobber case from the earlier review is covered.
One blocker, same as @oneandonlydean asked above:
- #1935 is on
mainnow and conflicts with this PR incore/relinker/relinker/src/analysis/SyscallScanner.cppandcore/relinker/CMakeLists.txt(git merge-treereports content conflicts in both). Please rebase and make the lookback work with the prefixed-opcode offset #1935 introduces, then re-runsyscall_scannerandsyscall_number.
Not covered by the Windows run linked above: it ran the previous head, and syscall_number.py was added afterwards.
a43e2ac to
fe54a44
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
I re-reviewed current head 5fc98348b71bedb1b2f92a2ee9437f84965b93ab. The earlier issues I raised are addressed: an unrecognized intervening instruction stops syscall-number inference, and the lookback uses decoded opcode offsets so prefixed instructions work with #1935's scanner change.
| Validation | Result |
|---|---|
| Linux Release relinker-only build | Passed |
| Full relinker CTest suite | 68/68 passed |
Conventions against current upstream/main |
Passed |
git diff --check |
Passed |
My earlier CHANGES_REQUESTED review no longer reflects this head, so I approve the code changes. This does not replace the repository-required manual Linux/Windows Build and Linux/Windows/macOS relinker workflow; those full Build checks are still skipped on the PR and remain required before merge.
|
Independent run on macOS (Apple silicon M2, macOS 26.7.1, Apple clang 21.0.0, CMake 4.2.3, Release, relinker-only build). 5fc9834 merged onto main b5f5362 (clean merge):
AI-assisted: yes |
|
Independent validation on Windows Built the
Results:
Rigor check (is the test real?): I disabled the number-reporting branch in Note on the C++ fixture: I confirmed in Honest limits: I only verified the syscall-number diagnostic (a message improvement). I did not run a real title (you note none was run either), and I did not test Linux/macOS. Written with AI assistance. |
…uction The syscall check names only the code offset, so a title that links a FreeBSD libc (the ps5-payload-dev SDK statically links one into every homebrew binary) stops with no indication of which call it reached. Those stubs load the number with a constant before the instruction (mov rax, 9 for link), so decode the nearest constant load into rax and add it to the message: Forbidden syscall instruction at code offset 0xa: syscall number 9 The number is reported only when it can be established. The constant load must be the instruction directly before the syscall, or one mov r10, rcx (the SDK's first-argument move, which does not write rax) in front of it; any other instruction stops the inference instead of returning a stale value, so mov rax, 9; add rax, 1; syscall reports no number rather than 9. Only the three invocation opcodes report a number: sysret is a return and keeps the current message, as does a stub that loads rax from a register, like the SDK's generic syscall() wrapper. The check still throws; only the text changes. Tested: relinker-only build, full ctest on Linux x86-64 (45/45). The C++ syscall_scanner test covers the opcode matrix and the review regression; the Python test_syscall_number covers the same contract through the harness the relinker lane already runs on Ubuntu, Windows and macOS, where the C++ fixture helpers (sys/mman.h, posix_spawn) do not build. AI-assisted: yes
The lookback classifies preceding instructions at their decoded opcode offset, the same place the forbidden-instruction check looks since boykopovar#1935, so a legacy-prefixed mov rax, imm or mov r10, rcx is recognized. The REX byte stays part of the patterns, so mov r8 and mov r8d cannot masquerade as rax loads. Anything unrecognized still stops the inference. Adds prefixed fixtures to both test suites: a prefixed constant stub that must report syscall number 9, and a prefixed clobbered stub that must report no number. AI-assisted: yes. The submitting author is responsible for every changed line.
5fc9834 to
1a45dee
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Rebased onto current main (only conflict: the add_test lines in core/relinker/CMakeLists.txt next to to_rosetta, both kept). Relinker build, syscall_scanner and syscall_number pass, and the scoped ctest is 92/92. The failed write-watch (macos-15-intel) job is a write_watch_threads timeout unrelated to this relinker-only change. Also confirmed by @MarcZX100 (Linux), @rmz-24 and @klosixgaming-arch (Windows), @DavidSamir (macOS). Merging.
What
Part of #2136: the syscall check now names the syscall the guest tried to reach, instead of only the code offset.
Seven catalog homebrew titles stop on the raw FreeBSD syscall stubs that
ps5-payload-dev's libc carries (libc/syscalls.c), and the current message gives nothing to act on:The fixed stubs set the number with a constant right before the instruction (
48 C7 C0 09 00 00 00ismov rax, 9, FreeBSDlink;B8 39 00 00 00ismov eax, 57,symlink).SyscallScannernow looks back for a constant load into rax and adds it:The number is only reported when it can be established. The constant load must be the instruction directly before the syscall, or one
mov r10, rcx(the SDK's first-argument move, which does not write rax) in front of it. Any other intervening instruction stops the inference rather than returning a stale value, somov rax, 9; add rax, 1; syscallreports no number instead of 9. A stub that takes the number from a register — the SDK's genericsyscall()/__syscall()wrapper reads rax from rdi — also reports no number and keeps today's text. The check still throws and nothing else changes;--skip-syscall-checkis unaffected.This does not pick one of the three directions in #2136, and the relinker still refuses the title. It makes the failure say which call it hit, which all three directions need, and it is the number #2136's author had to recover by hand with a disassembler.
Tested
Linux x86-64, relinker-only build (
-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON), GCC 16.2.1:ctest45/45.syscall_scannercovers all four forbidden opcodes — the 64-bit constant stub → number 9, the 32-bit constant stub → number 57,int 0x80→ number 11,sysenter→ no number,sysretwith a constant load in front of it → no number, a stub that loads rax from a register → no number, and a stub whose intervening instruction writes rax → no number (the regression for the review).test_syscall_numbercovers the same contract through the Python harness, so it also runs on Windows and macOS, where the C++ fixture helpers do not build.tools/check_conventions.py --base origin/mainandtools/check_test_registration.pyboth pass.Windows: not run here. @rmz-24 built
529476c7(the previous head, unchanged in this respect) ontomaind70b8998with MinGW-w64 GCC 15.2.0 and reported 510/510 on the fullctest.Not tested: a real title; no PS5 or homebrew title was run.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)