fix(codegen): callable-invoker trampoline preserves callee-saved registers (#487)

CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to elephc, a PHP-to-native compiler written in Rust.
 Releases are listed newest first.
 
 ## [Unreleased]
+- Fixed a silent miscompile around indirect callable invocations (issue #487): the generated descriptor-invoker trampoline used callee-saved registers as scratch without preserving them, so any value the register allocator kept live across a closure / callable-string / first-class-callable call — most visibly the accumulator in `$acc += $f()` inside a loop — was clobbered and read back as the trampoline's leftover scratch. The invoker now saves and restores the callee-saved registers it touches, like every compiled function prologue (AArch64 x19–x26, x86_64 r12/rbx/r13–r15).
 - Removed the legacy direct AST → ASM backend completely. EIR is now the only
   codegen implementation path.
 - Int-backed enum `from()` / `tryFrom()` now accept a dynamically-typed (`mixed`) argument (issue #449): a `foreach` value over a heterogeneous array, an untyped parameter, etc. are coerced on their runtime type before the enum lookup — integer/numeric-string resolve (or throw `ValueError`), float truncates, bool/null coerce, and array/object/resource/closure throw `TypeError` naming the given type. Previously any `mixed` argument was rejected at compile time. Target-aware on every supported backend.

src/codegen/runtime_callable_invoker.rs

@@ -25,7 +25,24 @@ use crate::types::{FunctionSig, PhpType};
 
 const INVOKER_DESCRIPTOR_OFFSET: usize = 8;
 const INVOKER_CONCAT_OFFSET: usize = 16;
-const INVOKER_FRAME_SIZE: usize = 32;
+/// First frame slot of the callee-saved register save area (issue #487).
+const INVOKER_SAVED_REGS_OFFSET: usize = 24;
+/// Frame size covering the footer, descriptor/concat slots, and the callee-saved save
+/// area (8 registers × 8 bytes on AArch64).
+const INVOKER_FRAME_SIZE: usize = 96;
+
+/// Callee-saved registers the invoker body uses as scratch. The invoker is an ordinary
+/// ABI function reached through `blr`/`call`, and the register allocator parks caller
+/// values in these registers across `callable_descriptor_invoke` sites, so the trampoline
+/// must preserve them like any compiled function prologue does (issue #487): without the
+/// save/restore, a value such as the loaded LHS of `$acc += $f()` read back as the
+/// trampoline's leftover scratch (the args-array length) after every indirect call.
+fn invoker_saved_callee_regs(arch: Arch) -> &'static [&'static str] {
+    match arch {
+        Arch::AArch64 => &["x19", "x20", "x21", "x22", "x23", "x24", "x25", "x26"],
+        Arch::X86_64 => &["r12", "rbx", "r13", "r14", "r15"],
+    }
+}
 
 /// Runtime invoker metadata emitted beside callable descriptors.
 pub(super) struct RuntimeCallableInvoker<'a> {
@@ -85,6 +102,12 @@ pub(super) fn emit_runtime_callable_invoker(
     emitter.raw(".align 2");
     emitter.label_global(invoker.label);
     abi::emit_frame_prologue(emitter, INVOKER_FRAME_SIZE);
+    // Preserve the caller's callee-saved registers before the body scratches them: the
+    // register allocator keeps values live across the indirect invoke in these registers
+    // (issue #487).
+    for (index, reg) in invoker_saved_callee_regs(emitter.target.arch).iter().enumerate() {
+        abi::store_at_offset(emitter, reg, INVOKER_SAVED_REGS_OFFSET + index * 8);
+    }
     abi::store_at_offset(
         emitter,
         abi::int_arg_reg_name(emitter.target, 0),
@@ -103,6 +126,11 @@ pub(super) fn emit_runtime_callable_invoker(
         data,
     );
     emit_box_current_value_as_mixed(emitter, &ret_ty.codegen_repr());
+    // Restore the caller's callee-saved registers (the boxed result travels in the
+    // return registers, which the restore loads never touch).
+    for (index, reg) in invoker_saved_callee_regs(emitter.target.arch).iter().enumerate() {
+        abi::load_at_offset(emitter, reg, INVOKER_SAVED_REGS_OFFSET + index * 8);
+    }
     abi::emit_frame_restore(emitter, INVOKER_FRAME_SIZE);
     abi::emit_return(emitter);
 }

tests/codegen/callables/expr_calls.rs

@@ -1451,3 +1451,60 @@ echo $cb();
     );
     assert_eq!(out, "B");
 }
+
+/// Regression for #487: a value the register allocator parks in a callee-saved register
+/// across an indirect callable invoke (here the loaded LHS of `$acc += $f()`) must survive
+/// the call. The generated descriptor-invoker trampoline used callee-saved registers as
+/// scratch without saving them, so the accumulator read back as the trampoline's leftover
+/// (the empty args-array length, 0) and only the last call's value survived.
+#[test]
+fn test_compound_assign_closure_call_rhs_accumulates() {
+    let out = compile_and_run(
+        r#"<?php
+$f = function (): int { return 4; };
+$acc = 0;
+for ($n = 0; $n < 20; $n++) { $acc += $f(); }
+echo $acc;
+"#,
+    );
+    assert_eq!(out, "80");
+}
+
+/// Regression for #487: same defect through a callable string and a first-class callable,
+/// with subtraction and multiplication (any operator whose LHS lives across the invoke).
+#[test]
+fn test_compound_assign_indirect_call_rhs_operators() {
+    let out = compile_and_run(
+        r#"<?php
+function four(): int { return 4; }
+$byName = 'four';
+$fcc = four(...);
+$a = 100;
+for ($n = 0; $n < 20; $n++) { $a -= $byName(); }
+$m = 1;
+for ($n = 0; $n < 5; $n++) { $m *= $fcc(); }
+echo $a, "|", $m;
+"#,
+    );
+    assert_eq!(out, "20|1024");
+}
+
+/// Regression for #487: the non-compound spelling and a property target exercise the same
+/// live-across-invoke registers.
+#[test]
+fn test_value_live_across_closure_invoke_survives() {
+    let out = compile_and_run(
+        r#"<?php
+class Box { public int $p = 0; }
+$f = function (): int { return 4; };
+$acc = 0;
+$b = new Box();
+for ($n = 0; $n < 20; $n++) {
+    $acc = $acc + $f();
+    $b->p += $f();
+}
+echo $acc, "|", $b->p;
+"#,
+    );
+    assert_eq!(out, "80|80");
+}