Skip to content

fix(relinker): agree on the syscall number between the two constant-load paths - #10439

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
KSWORK2001:fix/relinker-syscall-number-paths
Oct 26, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
KSWORK2001:fix/relinker-syscall-number-paths

Conversation

@KSWORK2001

@KSWORK2001 KSWORK2001 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

What

The syscall scan names the FreeBSD syscall number in its refusal message through two paths: ConstantSyscallNumber walks the decoded instructions before the pattern through ConstantRaxLoad, and ConstantSyscallNumberInBytes/ConstantRaxSetEndingAt scans the raw bytes back from the pattern outside the unwind functions. The two disagreed (#10422): the decoded walk reported mov rax, imm32 (48 C7 C0 imm32) without the sign-extension the register receives, and recognized neither mov rax, imm64 (48 B8 imm64) nor xor eax, eax-style zeroing (31/33 C0), while the byte window recognized all of these with sign-extension. The same load therefore named a different number depending on where it sat, and mov rax, -1; syscall reported 4294967295 where rax holds 0xFFFFFFFFFFFFFFFF.

ConstantRaxLoad now recognizes the byte window's forms with the same extension: 48 C7 C0 imm32 sign-extended, 48 B8 imm64, B8 imm32 zero-extended, and 31/33 C0 zeroing. The REX bits that redirect the written register exclude a form (REX.B moves the B8 register to r8-r15, REX.R extends the xor source, REX.B the xor destination), while REX.X extends no register and is accepted (42 31 C0 is xor eax, eax); an operand-size prefix also excludes the xor forms, since 66 31 C0 clears only AX. The backward walk now steps over a register-to-register mov/xor that consists only of a REX prefix, an opcode and a ModRM byte and writes a register other than rax — MovesOtherRegisterThanRax is applied to the slice that starts at the REX byte, and only when the whole instruction is that slice, so f2 49 89 ca steps because REX.B makes it write r10 while 8b 44 8b f8 (a memory load into eax) or 05 31 c9 00 00 (an immediate add to eax) stops the walk. ConstantRaxLoad and ConstantSyscallNumber return std::optional<std::uint64_t>, so the sign-extended value no longer truncates through std::optional's converting constructor.

Refuse and skip outcomes are unchanged inside the unwind functions and in images without unwind data, where both states end in a refusal either way. Outside the unwind functions the change is stricter: a pattern there is skipped when no number is established, and the byte window anchors at the syscall's opcode byte, so an F2/F3 prefix on a preceding instruction displaces that window and main established no number for a prefixed pattern. The decoded walk reads through prefixes, so such patterns are now refused with their number: in the test_sce_code_gaps fixture, 31 C0 F3 0F 05 C3 and 48 B8 3B 00 00 00 00 00 00 00 F2 49 89 CA 0F 05 C3 are skipped on main and refused here with numbers 0 and 59. Both cases are pinned as prefixed-zeroed and prefixed-mov-rax-imm64 in LOADED_NUMBERS. Refusing is the conservative direction: a real syscall with an established number no longer slips through a gap.

With this change the decoded walk answers the same as the prefix-aware byte window of #10537 for the encodings measured in review, including 42 31 C0 (0), 44 B8 imm32 (the immediate), mov eax,imm32; 47 31 C0 / 45 31 C0 / 45 89 C0 / 4D 89 C1 (the immediate), and the 20-byte tail of the SDK inline mprotect stub (74); rex-x-zeroed, rex-r-load and low-word-ending-in-move pin three of those on both paths in LOADED_NUMBERS, and the rest are mirrored in test_syscall_number.py (which runs the decoded walk without unwind data), with the over-acceptance controls mov eax,imm32; 8b 44 8b f8 and mov eax,imm32; 05 imm32 reporting no number. Three numbers remain that only the byte window reports: the partial rax writes it folds (b0, 66 b8, as low-word-after-zeroing and low-word-ending-in-move show), 66 48 31 C0, where the operand-size prefix still excludes the xor although REX.W makes it xor rax, rax, and a 41 B0 imm8 (a write to r8b) or any other r8-r15 byte write after a load, which the decoded walk conservatively stops at rather than recognizing as a write to another register. On main's unprefixed byte window, mov eax,imm32; 47/45 31 C0 still answers 0 by reading only the last two bytes; #10537's prefix-aware recognizer corrects that side.

Tested

Relinker-only build on Apple silicon (macOS 27.0.1, AppleClang 21), main 999e612599 against this branch. The new cases in SyscallScannerTests.cpp and test_syscall_number.py fail on main (mov rax, -1 reports 4294967295, mov rax, imm64 and xor eax, eax report no number, 42 31 C0 reports no number) and pass on the branch, including the negative controls (66 31 C0, 66 48 31 C0, 45 31 C0, 44 31 C0, 41 B8 imm32, mov eax,imm32; 41 B0 imm8, and the over-acceptance controls mov eax,imm32; 8b 44 8b f8, mov eax,imm32; 05 imm32 report no number). The two prefixed gap patterns above were run against main's relinker (skipped, exit 0) and the branch's (refused with 0 and 59); mov eax,59; 47 31 C0 was reproduced reporting 0 on the previous head and reports 59 now; decoded 42 31 C0 reports 0; the over-acceptance cases mov eax,59; 8b 44 8b f8 and mov eax,59; 05 imm32 were reproduced reporting 59 on the previous head and report no number now, and the gap mov eax,59; 66 b8 89 c1 was reproduced reporting 59 on the previous head and reports 49545 now. SyscallScannerTests.cpp runs on the Linux CI host; it was also compiled and run directly here against the branch and against main's relinker to record the failure. Full suite: 231/231 on the native arm64 build and 231/231 on the x86-64 Rosetta build (CMAKE_OSX_ARCHITECTURES=x86_64), before the change on main and with it. check_conventions.py --base origin/main passes; pr_overlap.py reports one shared file with #8042, whose SyscallScanner.cpp change is a single unrelated line (an instruction-scanner construction flag).

Checklist

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 10479 core/relinker/relinker/tests/test_syscall_number.py L17

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 10861 core/relinker/relinker/src/analysis/SyscallScanner.cpp

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

Reviewed head e8fd90f merged onto upstream/main aeeaefa (2026-10-24); the merge is clean, and so are merges onto the current main 50aa179 and together with #10479 and #10537.

Environment: Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 UCRT, CMake 4.3.1, Ninja 1.13.2, Python 3.13.9, -DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON, Release. My machine was under heavy unrelated CPU load, so I report targeted tests only; the relinker matrix is green on the PR.

Results

  • Merged tree: syscall_number, sce_code_gaps, guest_syscall_trap, sdk_mman_syscall pass. Red/green: with SyscallScanner.cpp reverted and the PR's tests kept, syscall_number fails with ('sign-extended', 'did not report syscall number 18446744073709551615'); restored, it passes. With #10479 and #10537 merged on top, syscall_number and sce_code_gaps (including #10537's new cases) still pass. The C++ syscall_scanner is Linux-only in CI and was not run here; the Python test carries the same byte sequences.
  • Encodings checked with objdump -M intel: 48 c7 c0 ff ff ff ff = mov rax, 0xffffffffffffffff (sign-extended), 48 31 c0 = xor rax, rax, 66 31 c0 = xor ax, ax, 45 31 c0 = xor r8d, r8d, 44 31 c0 = xor eax, r8d. Every form the PR accepts does set rax to the reported constant; I found no over-acceptance.
  • check_conventions.py passes; pr_overlap.py lists only #8042 as not compared (one unrelated line, and #8042 no longer merges against main).

Should fix (text)

  • "Every form the decoded walk newly recognizes was already recognized by the byte window, so which conversions are refused or skipped is unchanged" does not hold. Outside the unwind functions a gap pattern is skipped only when the decoded walk returns no number (SyscallScanner.cpp:302-313), and the decoded walk steps over F2/F3 prefixes that the byte window (MovesOtherRegisterThanRax, ConstantRaxSetEndingAt) cannot. With test_sce_code_gaps.executable and --skip-sce-module --registry: 48 B8 09 00 00 00 00 00 00 00 F2 49 89 CA 0F 05 C3 exits 0 on main ("Skipped 1 syscall byte pattern(s) ... 0x21e") and exits 2 on this branch ("Forbidden syscall instruction at code offset 0x21e: syscall number 9"); 31 C0 F3 0F 05 C3 likewise flips from skipped (0x212) to refused with number 0; the unprefixed 48 B8 imm64; 49 89 CA; 0F 05 and the 48 C7 C0 forms are refused on both. The change is stricter, not looser, which is fine, but the body should say that gap patterns with an F2/F3-prefixed mov r10, rcx or a prefixed syscall after a 48 B8 imm64 or 31/33 C0 load are now refused where main skipped them.

Nits

  • Body: "the sign-extended value no longer truncates through std::optional's converting constructor" misdescribes main: ConstantRaxLoad returned a std::uint32_t read without sign-extension (SyscallScanner.cpp:131-139 on main), and the only optional conversion (number = ConstantSyscallNumber(...) into optional<uint64_t>, line 304) widened. The widened return type is what lets the sign-extended value through; nothing truncated before.
  • kExtendedSourceZeroedStub / "stub that zeroes eax from r8d" (SyscallScannerTests.cpp:124 and :207, and extended-source-zeroed in the Python test): 44 31 C0 is xor eax, r8d (31 /r is XOR r/m32, r32; REX.R extends the source), eax becomes eax ^ r8d; the expected "no number" is right, the name is not.
  • The 33 C0 arm is accepted but never tested: 48 33 C0 (0) and a negative control such as 44 33 C0 = xor r8d, eax (no number) would pin it.

Notes

  • The decoded path still rejects some legal constant loads: 40 31 C0 (REX with no bits), 66 48 31 C0 (REX.W overrides 66), 4C C7 C0 imm32 and 44 B8 imm32 (REX.R is ignored for these); consequence is only a missing number in the message, and compilers do not emit them.
  • Cross-PR: #10537 replaces exactly the byte-window 66 31 C0 arm this body says is left alone, and it reports number 0 for 44 31 C0 and 41 33 C0, where this PR's decoded path (correctly) reports none; after both merge the two paths disagree on those bytes. I have left a note on #10537. Whichever of the two lands second should restate the resulting byte-window behaviour in its body.

Not checked: the macOS counts (231/231); the C++ syscall_scanner; a full suite; two consecutive REX bytes (48 48 31 C0 is reported as 0 by the PR; I did not verify the hardware semantics).

I found no blocking issue: the encodings are right, the counter-check holds, and the fix does what #10422 asks. The one thing to settle before merge is the body's claim about unchanged refuse/skip outcomes. For disclosure: I have open relinker PRs (#10564, #10402, #10401, #10403, #10390, #10391); none touches SyscallScanner.cpp.

Written with AI assistance (Claude Code). I ran the builds, tests and fixture checks above on my Windows 11 machine.

return value;
}

if ((!hasRex || rex == 0x48) && length - opcode == 2 && (bytes[opcode] == 0x31 || bytes[opcode] == 0x33) && bytes[opcode + 1] == 0xC0

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.

This arm and the 48 B8 arm above change refuse/skip outcomes outside the unwind functions, contrary to the body: a gap pattern is skipped only when the decoded walk has no number (lines 302-313), and the decoded walk steps over F2/F3 prefixes the byte window cannot. 31 C0 F3 0F 05 C3 and 48 B8 09 00 00 00 00 00 00 00 F2 49 89 CA 0F 05 C3 (test_sce_code_gaps.executable, --skip-sce-module --registry) are skipped on main and refused with numbers 0 and 9 on this branch. Stricter, so fine, but worth stating in the body.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed on both sides: with the test_sce_code_gaps fixture, 31 C0 F3 0F 05 C3 and 48 B8 09 00 00 00 00 00 00 00 F2 49 89 CA 0F 05 C3 are skipped on main's relinker (exit 0, one skipped pattern) and refused with numbers 0 and 9 on the branch. My subset argument held only for unprefixed patterns, because the byte window anchors at the syscall's opcode byte and an F2/F3 prefix displaces it. The body now states the stricter outcome instead of claiming it is unchanged, and both patterns are pinned as prefixed-zeroed and prefixed-mov-rax-imm64 in LOADED_NUMBERS (with 0x3b loads, refused as 59).

RequireNoNumber(binary, kClobberedStub, "stub whose intervening instruction writes rax");
RequireNoNumber(binary, kWordZeroedStub, "stub that zeroes only ax");
RequireNoNumber(binary, kExtendedRegisterZeroedStub, "stub that zeroes r8d");
RequireNoNumber(binary, kExtendedSourceZeroedStub, "stub that zeroes eax from r8d");

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.

Nit: 44 31 C0 is xor eax, r8d (31 /r = XOR r/m32, r32, REX.R extends the source), so eax is not zeroed; the expectation is right, the name and description are not. kExtendedSourceXorStub / "stub that xors eax with r8d" (and extended-source-xor in the Python test) would match the bytes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to kExtendedSourceXorStub / "stub that xors eax with r8d" and extended-source-xor in the Python test. 45 31 C0 (xor r8d, r8d) keeps the zeroed name, which matches its semantics.

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

Follow-up on exact head e8fd90f4e0533d6b3a760336f9957644b700842a, considering the current #10537 head 1f6c936b36446eeadc0c1e0f8bd37a4de8c65aa0. #10537 expands the byte-window recognizer, so the two paths now disagree on additional valid register encodings relevant to #10422:

  • 42 31 C0 is xor eax,eax (REX.X does not extend either register field). The byte window now reports 0, but ConstantRaxLoad rejects every REX value except 0x48, so the decoded path reports no syscall number.
  • 47 31 C0 is xor r8d,r8d. The byte window skips this write and can continue to a preceding mov eax,imm32; the decoded backward walk only skips 49 89 CA, so it stops at this unrelated write and reports no number instead of the loaded value.

This reintroduces the same path-dependent diagnostic the issue is meant to remove. Please align the decoded path with the prefix-aware byte recognizer (and add differential cases covering both unwind-backed and gap/fallback paths), or coordinate an alternative that makes both paths report the same value. My review of #10537's current build ran ctest --test-dir build -R '^(sce_code_gaps|syscall_number)$' --output-on-failure: 2/2 passed, but those tests do not compare the same encodings across both paths.

@roboterspinne

Copy link
Copy Markdown
Contributor

Measured MarcZX10's two cases and their neighbours on e8fd90f merged with #10537's 1f6c936 onto upstream/main c9b2660, and on e8fd90f alone over the same base (Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 UCRT, CMake 4.3.1, relinker-only Release; syscall_number, sce_code_gaps, guest_syscall_trap, sdk_mman_syscall and input_magic pass on the pair). Each sequence ends in 0f 05 c3. "Gap" is test_sce_code_gaps.executable with --skip-sce-module --registry, where the decoded walk runs first and ConstantSyscallNumberInBytes only when it has no number (lines 313-317); "decoded" is test_syscall_number.fixture, an image without unwind data, where the byte window never runs (line 315). Placing the sequences inside the 16-byte unwind-covered function of executable(function=...) (all but the b8 4a row fit) gives the same answers as the decoded column. The number is the one the refusal names; "skipped" is the gap heuristic giving up without one.

bytes before 0f 05 gap, with #10537 gap, this PR alone decoded
42 31 c0 0 0 no number
b8 3b 00 00 00 42 31 c0 0 0 no number
b8 3b 00 00 00 45 31 c0 59 0 no number
b8 3b 00 00 00 47 31 c0 59 0 no number
b8 3b 00 00 00 45 31 d2 45 31 c0 45 31 c9 59 0 no number
b8 3b 00 00 00 45 89 c0 59 59 no number
b8 3b 00 00 00 4d 89 c1 59 59 no number
b8 4a 00 00 00 48 89 f7 4c 89 c6 45 31 d2 45 31 c0 45 31 c9 74 0 no number
66 48 31 c0 0 0 no number
44 b8 3b 00 00 00 59 59 no number
b8 3b 00 00 00 41 b0 05 59 skipped no number
44 31 c0, 41 33 c0 (alone or after the load), 45 31 c0, 47 31 c0, 66 31 c0 (alone) skipped 0 no number
31 c0, 48 31 c0, b8 3b 00 00 00, b8 3b 00 00 00 49 89 ca 0, 0, 59, 59 0, 0, 59, 59 0, 0, 59, 59

Both cases hold, but #10537 does not create the disagreement: on e8fd90f alone every one of these rows already differs, because main's ConstantRaxSetEndingAt (line 205 here) takes any xx 31 c0 as zeroing while the decoded column says nothing. #10537 changes the gap column's value for the prefixed forms (0 becomes 59, 74 or skipped) and leaves the decoded column as it is. The cause is in this PR: ConstantSyscallNumberInBytes on main already steps over the register-to-register mov/xor forms (89/8B/31/33 with ModRM mod 11) that write a register other than rax (MovesOtherRegisterThanRax, used at SyscallScanner.cpp:246 on main, line 256 here), while ConstantSyscallNumber steps over 49 89 ca only (lines 180-184), and ConstantRaxLoad takes the zeroing xor only without REX or with exactly 0x48 (line 154) and b8 imm32 only without REX (line 148), where the body says "without a REX register extension" (REX.X in 42 extends no register). The realistic rows are the mixed ones: 45 31 c0 is what compilers emit for xor r8d, r8d, and the b8 4a ... row is the 20-byte tail of the SDK's inline mprotect stub (the bytes at SyscallScanner.cpp:59-61, syscall 74), which --trap-sdk-syscall-stubs traps only as the complete sequence; 42 and 47 are forms I would not expect from a compiler.

A fix that needs nothing from #10537: let ConstantSyscallNumber step over an instruction whose REX, opcode and ModRM bytes MovesOtherRegisterThanRax accepts (that covers 49 89 ca, which writes r10), take the two-byte xor in ConstantRaxLoad when (rex & 0x05) == 0 and b8 imm32 when (rex & 0x09) == 0, as the byte window does after #10537. That settles every row above except the partial writes (41 b0 05, and main's b0/66 b8 forms that ImmediateLowRaxWriteEndingAt folds in), which the decoded walk has no equivalent for, and 66 48 31 c0, which the 66 check still refuses although REX.W makes it xor rax, rax; naming those as the remaining gap-only numbers in the body would be enough. Rows in test_syscall_number.py mirroring the gap cases of test_sce_code_gaps.py for the sequences above would pin both paths to one answer for them, as MarcZX10 asks. The body's sentence that the two paths still disagree on 66 31 C0 outside the unwind functions stops being true once #10537 lands (that gap is then skipped).

Written with AI assistance (Claude Code). I ran the builds and checks above on my Windows 11 machine.

@KSWORK2001
KSWORK2001 force-pushed the fix/relinker-syscall-number-paths branch from e8fd90f to ad6fa74 Compare October 24, 2026 15:04
return value;
}

if ((!hasRex || rex == 0x48) && length - opcode == 2 && (bytes[opcode] == 0x31 || bytes[opcode] == 0x33) && bytes[opcode + 1] == 0xC0

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.

I reproduced the remaining path disagreement on this exact head with the locally built relinker. 42 31 C0 0F 05 C3 is refused without a number in the decoded fixture, but the same bytes in an SCE gap are refused as syscall number 0. 42 sets only REX.X; 31 C0 still zeroes EAX, so the decoded path should report 0 too. A second gap fixture, B8 3B 00 00 00 47 31 C0 0F 05 C3, reports number 0 even though 47 31 C0 zeroes R8D and leaves the earlier EAX value 59 unchanged; the decoded walk instead stops at that unrelated write. These exact cases are not in the passing current tests. Please make the decoded walk agree with the byte path for these prefixes and add differential fixtures that run the same encodings through both paths.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both cases are fixed at 0832fe9, following the direction in the measurements below: the backward walk now steps over every register-to-register mov/xor that MovesOtherRegisterThanRax accepts (the predicate the byte window already uses; 49 89 CA is one of them), ConstantRaxLoad takes the two-byte xor when (rex & 0x05) == 0 and b8 imm32 when (rex & 0x09) == 0. Measured on the branch: decoded 42 31 C0 reports 0; the gap fixture mov eax,59; 47 31 C0 reports 59 (0 on the previous head); 44 b8 3b reports 59; the 20-byte SDK mprotect tail reports 74. Differential coverage: rex-x-zeroed and rex-r-load are in LOADED_NUMBERS on the gap path, and test_syscall_number.py mirrors the measured sequences on the decoded path (extended-index-xor, load-then-extended-zeroed/xor/move/to-r9, prefix-aware-load, mprotect-tail), with 66 48 31 c0, 41 b8 imm32 and mov eax,imm32; 41 b0 imm8 as no-number controls. The partial low writes and 66 48 31 c0 are named in the body as the remaining gap-only numbers. Full relinker suites pass on arm64 and x86-64 Rosetta; the remaining mov eax,imm32; 47/45 31 C0 gap rows depend on the byte-window side and agree once #10537's prefix-aware recognizer lands.

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

Re-reviewed exact head ad6fa74e4ac99bafe2e33605360d51ee75ba12bc. The head delta from e8fd90f4 only renames an XOR test; the path disagreement relevant to #10422 remains. I built this exact head with GCC 12.2 in a relinker-only Release configuration. ctest -R '^(sce_code_gaps|syscall_number|syscall_scanner)$' --output-on-failure passed 3/3, but two extra exact-head fixtures reproduced the uncovered bug: decoded 42 31 C0 has no number while the SCE-gap scan reports 0; a gap fixture with mov eax,59; 47 31 C0 reports 0 though the XOR only writes R8D and should preserve 59. git diff --check upstream/main...HEAD and the exact-head conventions check pass. Hosted relinker matrix and ASan pass; product jobs are skipped. Please resolve the decoded/byte-path mismatch and add differential coverage before merge.

@KSWORK2001
KSWORK2001 force-pushed the fix/relinker-syscall-number-paths branch from ad6fa74 to 0832fe9 Compare October 25, 2026 15:18
@KSWORK2001

Copy link
Copy Markdown
Contributor Author

Followed your analysis exactly at 0832fe9: ConstantSyscallNumber steps over anything MovesOtherRegisterThanRax accepts, the xor gate is (rex & 0x05) == 0 and the b8 imm32 gate is (rex & 0x09) == 0. Every row of your table now has the decoded column equal to your with-#10537 gap column, except the ones you called out as remaining: the partial low writes (41 b0 05, b0, 66 b8) and 66 48 31 c0, which the body now names as gap-only. rex-x-zeroed and rex-r-load were added to LOADED_NUMBERS (both paths agree on them today), the other sequences are mirrored in test_syscall_number.py, and the stale 66 31 C0 sentence in the body is replaced by the remaining-numbers paragraph, noting that the mov eax,imm32; 47/45 31 C0 gap rows depend on #10537's recognizer. Thank you for the measurement table — it made the fix mechanical.

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

Reviewed head 0832fe91b merged onto main 0119103fa (clean; still clean on 79f061ba5, which touches none of these files), and that merge plus #10537 1f6c936b. Linux: WSL2 Ubuntu 24.04, GCC 13.3, relinker-only Release.

What I ran

  • ctest -R '^(sce_code_gaps|syscall_number|syscall_scanner)$': 3/3 pass on the merge and on the merge with #10537. With main's SyscallScanner.cpp and your tests kept, all 3 fail (mov rax, -1 gives 4294967295, prefixed-zeroed is skipped). So the C++ syscall_scanner now has a Linux red/green too.
  • MarcZX10's two cases are fixed: decoded 42 31 c0 gives 0, and the gap mov eax,59; 47 31 c0 gives 59.
  • roboterspinne's rows, each through the decoded fixture, a gap, and the unwind function. With #10537 the decoded and gap answers match on every row except 66 48 31 c0 and mov eax,59; 41 b0 05. Those are the gap-only cases the body names.

Should fix: the new step-over accepts instructions that write eax

ConstantSyscallNumber now calls MovesOtherRegisterThanRax(bytes, match.Length) on decoded instructions of any length (SyscallScanner.cpp:194). That helper was written for 2- or 3-byte raw windows. For any other length it sets rex to 0 and takes the last two bytes as opcode and ModRM (:161-167). So any 4+ byte instruction whose tail looks like 89/8B/31/33 + a mod-11 byte gets stepped over, even when it writes eax.

Example: b8 3b 00 00 00 8b 44 8b f8 0f 05 c3. objdump gives mov eax,0x3b; mov eax,DWORD PTR [rbx+rcx*4-0x8]; syscall. The tail 8b f8 reads as mov edi, eax.

  • main: no number in the decoded fixture and inside the function; the gap is skipped.
  • this PR: syscall number 59 on all three paths.

Same with 8b 44 33 f8 (mov eax,[rbx+rsi-8]), 05 00 00 31 c9 (add eax,0xc9310000) and 66 b8 89 c1 (mov ax,0xc189). The last one also creates a new disagreement in a gap. There main's byte window gives the right 49545, but the decoded walk now runs first and names 59. #10537 doesn't change any of these.

Here's what I tried locally: step over only when the instruction is just (REX) + opcode + ModRM, and pass the helper the slice that starts at the REX byte:

const std::size_t opcode = Codegen::DecodedInstruction{bytes, match.Length}.OpcodeOffset();
const std::size_t begin = opcode > 0 && (bytes[opcode - 1] & 0xF0) == 0x40 ? opcode - 1 : opcode;
if (match.Length - opcode != 2 || !MovesOtherRegisterThanRax(bytes + begin, match.Length - begin))
    return std::nullopt;

With that, the 3 tests still pass and the four sequences above get main's answers back (no number, skipped, 49545 in the gap). Every roboterspinne row gives the same answer as on your head. f2 49 89 ca still steps over, but now because REX.B makes it r10. Today it passes only because the dropped REX leaves rm 2 (rdx). A no-number case like b8 3b 00 00 00 8b 44 8b f8 0f 05 c3 would pin this.

Nits

  • kExtendedLoadStub is 41 B8, which is mov r8d. Its description says "loads r9d".
  • Body: "steps over every register-to-register mov/xor whose REX, opcode and ModRM bytes MovesOtherRegisterThanRax accepts" holds only for 2- and 3-byte instructions right now (see above).

Checked and fine: every ConstantRaxLoad form against the SDM encodings. 48 C7 C0 sign-extends. B8 with W=0 and B=0 zero-extends. 31/33 C0 with R=0 and B=0, without 66, zeroes rax. A REX before a legacy prefix is ignored. I found no over-acceptance there.

Not checked: Windows and macOS, the full suite, real binaries.

@roboterspinne

Copy link
Copy Markdown
Contributor

The over-acceptance ahmadmuj reports comes from my suggestion. I proposed letting ConstantSyscallNumber step over an instruction whose REX, opcode and ModRM bytes MovesOtherRegisterThanRax accepts, but I never said the instruction must consist of only those bytes. That helper was written for the 2- and 3-byte raw windows of the byte scan; on a whole decoded instruction it only looks at the last two bytes. Sorry for sending you down that path.

I reproduced it on Windows (this head on main ed0d0ae, relinker-only Release, GCC 15.2). b8 3b 00 00 00 0d 00 00 89 c1 0f 05 (or eax,0xC1890000) and b8 3b 00 00 00 8b 44 8b f8 0f 05 (mov eax,[rbx+rcx*4-8]) now report syscall number 59, both in the decoded fixture and in an SCE gap. main gives no number there and skips the gap. b8 3b 00 00 00 66 b8 89 c1 0f 05 (mov ax,0xC189) reports 59 in a gap, where main reports the correct 49545. main and ad6fa74 stepped over only 49 89 ca.

I applied ahmadmuj's snippet unchanged. All of those inputs get main's answers back. 89 c1, f2 49 89 ca, f2 49 89 c8 and 66 89 c1 after mov eax, 59 report 59; f2 49 89 c8 and 66 89 c1 give no number on this head. The mprotect tail still reports 74. syscall_number, sce_code_gaps, sdk_mman_syscall, guest_syscall_trap and input_magic pass, both alone and merged with #10537.

These two rows fail on this head and pass with the snippet, in case they help:

# test_syscall_number.py, no-number list
LOAD_THEN_ADD_ENDING_IN_MOVE = bytes.fromhex("b83b00000005000089c1") + b"\x0f\x05\xc3"
# test_sce_code_gaps.py, LOADED_NUMBERS
("low-word-ending-in-move", bytes.fromhex("b8 3b 00 00 00 66 b8 89 c1 0f 05 c3"), 0x219, 0xC189),

The second row pins the gap case where main is already right.

One correction to my earlier table: 41 b0 05 is mov r8b, 5, not a partial write to rax. After mov eax, 59 the gap's 59 is right, and the decoded "no number" is a conservative miss. My table and the body both group it with the al/ax writes.

Not run: SyscallScannerTests.cpp (Linux only), and the full suite with the snippet.

Written with AI assistance (Claude Code). I ran the builds and checks above on my Windows 11 machine.

@KSWORK2001
KSWORK2001 force-pushed the fix/relinker-syscall-number-paths branch from 0832fe9 to 0f45390 Compare October 26, 2026 03:00
@KSWORK2001

Copy link
Copy Markdown
Contributor Author

Your fix is applied verbatim at 0f45390, with both nits corrected (the description now says the stub loads r8d, and the body states the walk steps over a mov/xor consisting only of a REX prefix, an opcode and a ModRM byte). Reproduced all your cases on the previous head and confirmed main's answers are back on this one: mov eax,59; 8b 44 8b f8 and mov eax,59; 05 imm32 report no number again, and the gap mov eax,59; 66 b8 89 c1 reports 49545. f2 49 89 ca still steps, now for the right reason. Added your no-number case to both SyscallScannerTests.cpp (kLoadThenMemoryLoadStub, kLoadThenImmediateAddStub) and test_syscall_number.py, and roboterspinne's low-word-ending-in-move row pins the gap case. Full suites pass on arm64 and x86-64 Rosetta; conventions pass. Thank you for the catch and the ready-made fix.

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

@KSWORK2001 Thanks, checked 0f45390 merged onto main 204c674 (2026-10-26); the merge is clean. Windows 11, GCC 15.2, relinker-only Release.

  • Full ctest: 306 of 307 pass; patch_advisor_unit fails the same way on plain main, and long_paths is skipped. syscall_number, sce_code_gaps, sdk_mman_syscall, guest_syscall_trap and input_magic pass.
  • Counter-check against the new tests:
    • The previous head's SyscallScanner.cpp fails syscall_number at load-then-memory-load and sce_code_gaps at low-word-ending-in-move; both report 59.
    • main's fails at sign-extended and prefixed-zeroed.
  • The decoys from the last round get main's answers back: no number, or a skipped gap, and 49545 for 66 b8 89 c1 in a gap. After mov eax, 59, 89 c1, f2 49 89 ca, f2 49 89 c8 and 66 89 c1 report 59, and the mprotect tail reports 74.
  • The hosted Build passed on Linux, macOS and Windows.
  • It still merges cleanly with #10537. With #10479 there is still one conflict in test_syscall_number.py, where both add constants after PREFIXED_CLOBBERED; whichever lands second keeps both blocks.

That settles the points from my review and my follow-up; I found nothing else to change.

Written with AI assistance (Claude Code). I ran the checks above on my Windows 11 machine.

@KSWORK2001

Copy link
Copy Markdown
Contributor Author

Thank you for the thorough verification — the counter-checks against the previous head and plain main, and the merge checks against #10537 and #10479, are exactly what this needed. The #10479 conflict is the expected additive one in test_syscall_number.py; whichever lands second keeps both constant blocks. Awaiting the remaining re-reviews.

@adriadam10 adriadam10 self-assigned this Oct 26, 2026
@adriadam10
adriadam10 dismissed MarcZX10’s stale review October 26, 2026 17:18

Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.

@adriadam10
adriadam10 merged commit cabd860 into boykopovar:main Oct 26, 2026
14 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the decoded and byte-window paths against each other on the encodings raised in review (42 31 C0, 47/45 31 C0, 44 B8, mov eax,imm32 followed by 8b 44 8b f8 / 05 imm32, sign-extended and imm64 loads); the new rows are pinned in SyscallScannerTests.cpp, test_syscall_number.py and test_sce_code_gaps.py and fail on main. The earlier change request was on an older head and is met. Relinker CI is green on this head; merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix error Closes an issue (Fixes #N) relinker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

relinker: the two syscall-number paths disagree on sign-extension and on the constant-load forms they recognize

5 participants