Recognize framed pointer tail calls safely (#256)

Preserve the existing terminal-tail recognizer and accept a contiguous framed method-pointer thunk followed by its separate null-error call. Reject stores that overwrite saved FP/LR, unproven store addresses and stack/frame/link-register clobbers, including WSP. Stores are limited to fixed caller stack-argument slots above the saved frame. Keep the fallback narrower than #188's proposed unframed block recognition. Add synthetic AArch64 regressions and document the rule beside the other tail-call findings; no game-specific IDs, fixed addresses or profile changes. Validation: 179 tool tests run, 174 passed and 5 skipped; 10 tail-call tests pass; the legacy terminal recognizer is AST-identical to main; git diff --check passes. Real game modules were not rewritten for this integration. Co-authored-by: chengjie <chengjie177@gmail.com>
2026-10-26 14:46:10 +08:00
1 parent 4acde72a6e
commit 2019ff866b
@@ -738,7 +738,7 @@ def jump_masked_pred(insl, j):
    t = [tb + struct.unpack_from('<i', IMG, tb + 4 * i)[0] for i in range(n)]
    addresses = {p.address for p in insl}
    return t if all(x in addresses for x in t) else None
-def tail_call(insl, j):
+def tail_call_at_end(insl, j):
    """The br x18 is an indirect tail call, not a jump table: the last write of x18 before it is a plain pointer load and the br ends the function. Ace Attorney Investigations Collection
    (Unity 2022.3 IL2CPP) main+0x1d271bc: ldr x18,[x0,#0x18]; ...; ldr x16,[sp]; ...; br x18, a thunk that moves stack arguments and jumps to the method pointer at +0x18"""
    if j + 1 != len(insl): return False
@@ -746,6 +746,42 @@ def tail_call(insl, j):
        if 'x18' not in [p.reg_name(r) for r in p.regs_access()[1]]: continue
        return p.mnemonic == 'ldr' and bool(re.fullmatch(r'x18, \[(x\d+|sp)(, #(0x[0-9a-f]+|\d+))?\]', p.op_str))
    return False
+def tail_call(insl, j):
+    """Keep the existing terminal-tail rule; extend only the framed thunk with
+    a separate error call after its tail jump (Warm Snow, Unity IL2CPP).
+    The whole thunk must decode contiguously, restore its frame, and contain
+    only one forward null guard, this tail jump, and the trailing error call.
+    This deliberately does not generalize arbitrary mid-function br x18."""
+    if j + 1 == len(insl): return tail_call_at_end(insl, j)
+    if j < 4 or any(b.address != a.address + 4 for a, b in zip(insl, insl[1:])): return False
+    suffix = insl[j + 1:]
+    if suffix[0].mnemonic != 'bl' or any(p.mnemonic != 'udf' for p in suffix[1:]): return False
+    if insl[0].address <= suffix[0].operands[-1].imm <= insl[-1].address: return False
+    if insl[0].mnemonic != 'stp' or insl[0].op_str != 'x29, x30, [sp, #-0x10]!': return False
+    if insl[1].mnemonic != 'mov' or insl[1].op_str != 'x29, sp': return False
+    if insl[j - 1].mnemonic != 'ldp' or insl[j - 1].op_str != 'x29, x30, [sp], #0x10': return False
+    guard = insl[2]
+    if guard.mnemonic != 'cbz' or guard.operands[-1].imm != suffix[0].address: return False
+    load = None
+    for k in range(3, j - 1):
+        p = insl[k]
+        # Argument moves and memory transfers only. No hidden control flow,
+        # stack-pointer writes, or unmodelled instructions inside the new thunk form.
+        if p.mnemonic not in ('mov', 'ldr', 'ldur', 'ldp', 'str', 'stur', 'stp'): return False
+        writes = {p.reg_name(r) for r in p.regs_access()[1]}
+        if writes & {'sp', 'wsp', 'fp', 'lr', 'x29', 'w29', 'x30', 'w30'}: return False
+        if p.mnemonic in ('str', 'stur', 'stp'):
+            # regs_access does not report memory writes. Only fixed caller stack
+            # argument slots are proven clear of the saved FP/LR at sp + [0, 16).
+            # Other bases may alias that frame, and a register offset is unproven.
+            if p.writeback: return False
+            mem = p.operands[-1].mem
+            if p.reg_name(mem.base) not in ('sp', 'fp', 'x29') or mem.index or mem.disp < 16: return False
+        if writes & {'x18', 'w18'}:
+            if load is not None or p.mnemonic != 'ldr' or not re.fullmatch(r'x18, \[(x\d+|sp)(, #(0x[0-9a-f]+|\d+))?\]', p.op_str): return False
+            load = p
+    return load is not None
+
 def jump_bytes_far(insl, j):
    """The byte / halfword table form with its add further back (Octopath Traveler 0 main+0x37f9bc8: adr x18,#L; ldrb w0,[x17,x15]; add x18,x18,x0,lsl #2, then 17 register moves before br x18).
    Up to 32 instructions back, and x18 must not change between the add and the br. Tried only where the 16-instruction search and the offset forms find nothing. Returns (base, width) or None"""
@@ -0,0 +1,108 @@
+"""Exercise actual AArch64 thunk shapes without any game files."""
+import ast
+from pathlib import Path
+import re
+import struct
+import sys
+import unittest
+
+from capstone import Cs, CS_ARCH_ARM64, CS_MODE_ARM
+
+TOOLS = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(TOOLS))
+from nso_asm import STP, STUR, STRX
+
+tree = ast.parse((TOOLS / 'patch_nso.py').read_text())
+scope = {'re': re}
+exec(compile(ast.Module(body=[n for n in tree.body if isinstance(n, ast.FunctionDef)
+    and n.name in ('tail_call', 'tail_call_at_end')], type_ignores=[]), str(TOOLS / 'patch_nso.py'), 'exec'), scope)
+
+# Standard frame, null guard to error block, pointer load, argument move,
+# frame restore, tail jump, error call, trap padding.
+THUNK = [0xa9bf7bfd, 0x910003fd, 0xb40000a1, 0xf9400812,
+         0xaa0103e0, 0xa8c17bfd, 0xd61f0240, 0x94000400, 0]
+
+
+def decode(words):
+    md = Cs(CS_ARCH_ARM64, CS_MODE_ARM)
+    md.detail = True
+    return list(md.disasm(struct.pack('<' + 'I' * len(words), *words), 0))
+
+
+class TailCall(unittest.TestCase):
+    def check(self, words, j=6):
+        ins = decode(words)
+        self.assertEqual(len(ins), len(words))
+        return scope['tail_call'](ins, j)
+
+    def test_framed_thunk_with_error_block(self):
+        self.assertTrue(self.check(THUNK))
+        self.assertTrue(self.check(THUNK[:-1]))
+
+    def test_legacy_terminal_patterns_are_unchanged(self):
+        for words in ([0xf9400812, 0xd61f0240],
+                      [0xf9400812, 0xb4000081, 0xaa0103e0, 0xd61f0240],
+                      [0xaa0103f2, 0xd61f0240]):
+            ins = decode(words)
+            self.assertEqual(self.check(words, len(words) - 1), scope['tail_call_at_end'](ins, len(ins) - 1))
+        self.assertTrue(self.check([0xf9400812, 0xb4000081, 0xaa0103e0, 0xd61f0240], 3))
+
+    def test_clobbers_calls_branches_and_traps_rejected(self):
+        for word in (0x52800012, 0xaa0003f2, 0x94000400, 0xd63f0000,
+                     0x14000001, 0xb4000020, 0xd65f03c0, 0, 0x910043ff):
+            with self.subTest(word=hex(word)):
+                words = THUNK.copy(); words[4] = word
+                self.assertFalse(self.check(words))
+
+    def test_required_frame_and_guard(self):
+        for index, word in ((0, 0xd503201f), (1, 0xd503201f), (2, 0xb4000021),
+                            (2, 0xb50000a1), (3, 0xaa0003f2), (5, 0xd503201f),
+                            (7, 0xd503201f), (7, 0x97fffffd), (8, 0xaa0003e0)):
+            with self.subTest(index=index, word=hex(word)):
+                words = THUNK.copy(); words[index] = word
+                self.assertFalse(self.check(words))
+
+    def test_decoding_gap_is_rejected(self):
+        ins = decode(THUNK)
+        del ins[4]
+        self.assertFalse(scope['tail_call'](ins, 5))
+
+    def test_unframed_mid_function_pointer_jump_rejected(self):
+        self.assertFalse(self.check([0xf9400812, 0xaa0103e0, 0xd61f0240, 0x94000400], 2))
+        # The broader rule proposed in #188 accepts an unframed guarded block.
+        # Keep the framed fallback's boundary even when a null guard is present.
+        self.assertFalse(self.check([0xb4000061, 0xf9400812, 0xd61f0240, 0x94000400], 2))
+
+    def test_stores_must_not_overwrite_saved_frame(self):
+        for base in (31, 29):   # sp and the unchanged frame pointer
+            for word in (STRX(0, base), STRX(0, base, 1), STUR(0, base, 15),
+                         STP(0, 1, base, -8), STP(0, 1, base, 8)):
+                with self.subTest(base=base, word=hex(word)):
+                    words = THUNK.copy(); words[4] = word
+                    self.assertFalse(self.check(words))
+
+    def test_stack_and_frame_register_aliases_rejected(self):
+        for word in (0x1100001f, 0x9100001f,   # mov wsp, w0; mov sp, x0
+                     0x2a0003fd, 0xaa0003fd,   # mov w29, w0; mov x29, x0
+                     0x2a0003fe, 0xaa0003fe):  # mov w30, w0; mov x30, x0
+            with self.subTest(word=hex(word)):
+                words = THUNK.copy(); words[4] = word
+                self.assertFalse(self.check(words))
+
+    def test_caller_stack_argument_stores_are_accepted(self):
+        for base in (31, 29):
+            for word in (STRX(0, base, 2), STUR(0, base, 16), STP(0, 1, base, 16)):
+                with self.subTest(base=base, word=hex(word)):
+                    words = THUNK.copy(); words[4] = word
+                    self.assertTrue(self.check(words))
+
+    def test_unproven_store_addresses_are_rejected(self):
+        for word in (STRX(0, 9, 2), STUR(0, 9, 16), STP(0, 1, 9, 16),
+                     0xf8206be0):   # str x0, [sp, x0]: dynamic offset may hit the frame
+            with self.subTest(word=hex(word)):
+                words = THUNK.copy(); words[4] = word
+                self.assertFalse(self.check(words))
+
+
+if __name__ == '__main__':
+    unittest.main()
@@ -20,6 +20,7 @@ One line per symptom in the [subsystem references](../reference/): what you see
 - `jump table <addr>` assertion on a byte table whose `add` is far from the `br` (`adr x18,#L; ldrb w0,[x17,x15]; add x18,x18,x0,lsl #2`, then 17 register moves before `br x18`). → [Byte tables whose add is far from the br](../reference/rewriter.md#byte-tables-whose-add-is-far-from-the-br)
 - `jump table <addr>` assertion in `patch_nso.py`, in a Unity / IL2CPP `main`, at a `blr x18`. → [Indirect calls through x18](../reference/rewriter.md#indirect-calls-through-x18)
 - `jump table <addr>` assertion on a `br x18` with no table in front of it. → [Indirect tail calls through x18 in IL2CPP thunks](../reference/rewriter.md#indirect-tail-calls-through-x18-in-il2cpp-thunks)
+- `jump table 53e9c` while rewriting Warm Snow 2.1.4's Unity IL2CPP `main`. → [Loaded method-pointer tail call before an error block](../reference/rewriter.md#loaded-method-pointer-tail-call-before-an-error-block)
 - job system hangs; a queue claims thousands of tasks but every slot is empty. → [Exit stub order in exclusive windows](../reference/rewriter.md#exit-stub-order-in-exclusive-windows)
 - `branch into window <addr>` assertion. → [Out-of-line retry blocks of atomic updates](../reference/rewriter.md#out-of-line-retry-blocks-of-atomic-updates)
 - branches to an `ldxr … stxr` loop's head landed mid-window. → [A window never spans a second exclusive load](../reference/rewriter.md#a-window-never-spans-a-second-exclusive-load)
@@ -109,6 +109,14 @@ To enroll a cut for any game's own module:
 - **Found on:** [Ace Attorney Investigations Collection](../../dm/games/010005501E68C000/README.md) (Unity 2022.3 IL2CPP): `main+0x1d271bc` and `+0x204ebec`.
 - **Check:** every module the rewriter accepted before comes out byte-identical.

+### Loaded method-pointer tail call before an error block
+
+- **Symptom:** `jump table 53e9c` while rewriting Warm Snow 2.1.4's Unity IL2CPP `main`.
+- **Cause:** a framed thunk loads a method pointer into `x18`, moves arguments, restores its frame and executes `br x18`. A separate null-error call follows it. The terminal-tail recognizer requires the jump to end the function.
+- **Fix:** keep the existing terminal-tail rule unchanged. Only for nonterminal jumps, recognize a contiguous decoded thunk with the standard 16-byte FP/LR frame, a single forward `cbz` to the trailing error call, plain argument moves/transfers, one pointer load into `x18`, and the matching frame restore. Reject additional control flow, stack-pointer/frame-register/link-register writes, pointer clobbers and undecoded gaps. Stores must use a fixed offset of at least 16 from the unchanged `sp` or frame pointer, so they write caller stack arguments above the saved FP/LR. Checking register writes alone does not protect that saved pair; stores through other bases or dynamic offsets may alias it and are rejected. An unframed loaded-pointer jump followed by other blocks remains unsupported: ending a basic block alone does not prove a tail call.
+- **Found on:** Warm Snow 2.1.4, `main` build `306fc054203275803dcbceba7fb7c392`, at `+0x53e9c`. Its game profile is deliberately outside this change.
+- **Check:** `dm/tools/tests/test_tail_call.py` covers acceptance and rejection with synthetic AArch64 instructions, including saved-frame stores, safe caller argument stores, unproven store addresses, 32-bit and 64-bit SP/FP/LR clobbers (including `wsp`), the unframed guarded block and the previously accepted terminal tail with an intervening conditional branch. Actual game rewrite/build results and remaining coverage are recorded in the PR; no game assets are required by the tests.
+
 ## Exclusive windows

 ### Exit stub order in exclusive windows