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).
 - Fixed symbolic defaults for object-typed parameters: enum-case defaults are now resolved and type-checked semantically after class schemas are complete across functions, methods and constructors, constructor-promoted properties, and closures, including named, `self::`, `static::`, and `parent::` receivers. Missing cases and scalar class constants are rejected instead of slipping through syntactic typing, while `ReflectionParameter::getDefaultValueConstantName()` preserves the source-level constant spelling.
 - Added PHP-compatible `::class` support on object expressions: `$object::class` now returns the receiver's concrete runtime class name, evaluates the receiver exactly once, and rejects statically known non-object receivers, while existing named and static forms remain unchanged.
 - Added support for `static $x;` function-static declarations without an initializer, in both the native parser and the Magician `eval()` parser: the missing initializer desugars to `= null`, matching PHP, where `static $x;` and `static $x = null;` are identical (including `isset()` behavior).

src/codegen/eval_callable_helpers.rs

@@ -1028,10 +1028,7 @@ fn store_descriptor_entry_incoming_arg(
             } else {
                 let caller_offset =
                     descriptor_entry_caller_stack_offset(emitter, stack_offset.expect("stack offset"));
-                let spill_reg = match emitter.target.arch {
-                    crate::codegen::platform::Arch::AArch64 => "d15",
-                    crate::codegen::platform::Arch::X86_64 => "xmm15",
-                };
+                let spill_reg = abi::float_spill_scratch_reg(emitter.target);
                 abi::load_from_caller_stack(emitter, spill_reg, caller_offset);
                 spill_reg
             };
@@ -1100,10 +1097,7 @@ fn load_descriptor_entry_actual_arg(
             let reg = if assignment.in_register() {
                 abi::float_arg_reg_name(emitter.target, assignment.start_reg)
             } else {
-                match emitter.target.arch {
-                    crate::codegen::platform::Arch::AArch64 => "d15",
-                    crate::codegen::platform::Arch::X86_64 => "xmm15",
-                }
+                abi::float_spill_scratch_reg(emitter.target)
             };
             abi::load_at_offset(emitter, reg, offset);
             if let Some(out_offset) = stack_offset {

src/codegen/lower_inst.rs

@@ -1395,10 +1395,7 @@ fn store_descriptor_entry_incoming_arg(
                     emitter,
                     stack_offset.expect("stack offset"),
                 );
-                let spill_reg = match emitter.target.arch {
-                    Arch::AArch64 => "d15",
-                    Arch::X86_64 => "xmm15",
-                };
+                let spill_reg = abi::float_spill_scratch_reg(emitter.target);
                 abi::load_from_caller_stack(emitter, spill_reg, caller_offset);
                 spill_reg
             };
@@ -1473,10 +1470,7 @@ fn load_descriptor_entry_actual_arg(
             let reg = if assignment.in_register() {
                 abi::float_arg_reg_name(emitter.target, assignment.start_reg)
             } else {
-                match emitter.target.arch {
-                    Arch::AArch64 => "d15",
-                    Arch::X86_64 => "xmm15",
-                }
+                abi::float_spill_scratch_reg(emitter.target)
             };
             abi::load_at_offset(emitter, reg, offset);
             if let Some(out_offset) = stack_offset {

src/codegen/runtime_callable_invoker.rs

@@ -10,6 +10,8 @@
 //! - Capture values are loaded from the callable descriptor, not caller frame state.
 //! - Argument materialization supports indexed arrays, associative arrays, defaults, variadics,
 //!   by-reference marker cells, and target-aware ABI calls without depending on `Context`.
+//! - The trampoline preserves the callee-saved registers it scratches (issue #487), including on
+//!   the eval exception-boundary escape path, so allocator-parked caller values survive the invoke.
 
 use crate::codegen::callable_descriptor;
 use crate::codegen::callable_invoker_args::{
@@ -28,11 +30,49 @@ use crate::types::{FunctionSig, PhpType};
 
 const INVOKER_DESCRIPTOR_OFFSET: usize = 8;
 const INVOKER_CONCAT_OFFSET: usize = 16;
-const INVOKER_FRAME_SIZE: usize = 32;
+/// Spill slot for the boxed argument container across the eval exception-boundary `setjmp`.
 const INVOKER_ARG_ARRAY_OFFSET: usize = 24;
+/// First frame slot of the callee-saved register save area (issue #487).
+/// Placed after descriptor/concat/arg-array so the eval boundary path can reuse those slots.
+const INVOKER_SAVED_REGS_OFFSET: usize = 32;
+/// AArch64 save-area width (8 callee-saved scratch regs × 8 bytes); sizes the shared frame.
+const INVOKER_CALLEE_SAVE_BYTES: usize = 8 * 8;
+/// Exclusive end of the callee-saved save area (`INVOKER_SAVED_REGS_OFFSET` … end-8).
+const INVOKER_SAVED_REGS_END: usize = INVOKER_SAVED_REGS_OFFSET + INVOKER_CALLEE_SAVE_BYTES;
+/// Frame size covering the footer plus every local slot through the save area.
+/// `frame_size - 16` must cover the last save offset; rounded up to 16 for ABI `sp` alignment.
+const INVOKER_FRAME_SIZE: usize = ((INVOKER_SAVED_REGS_END - 8) + 16 + 15) & !15;
 const INVOKER_BOUNDARY_FRAME_SIZE: usize = INVOKER_FRAME_SIZE + TRY_HANDLER_SLOT_SIZE + 16;
 const INVOKER_BOUNDARY_BASE_OFFSET: usize = INVOKER_BOUNDARY_FRAME_SIZE - 16;
 
+/// Callee-saved registers the invoker body uses as scratch. Must stay in sync with the
+/// hard-coded scratch choices in the indexed/assoc/mixed argument loaders and
+/// `abi::nested_call_reg` (`x19` / `r12`). The register allocator parks caller values in
+/// these registers across `callable_descriptor_invoke`, so the trampoline must preserve
+/// them like any compiled function prologue (issue #487).
+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"],
+    }
+}
+
+/// Stores the caller's callee-saved registers the invoker body is about to scratch.
+fn emit_invoker_callee_saved_saves(emitter: &mut Emitter) {
+    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);
+    }
+}
+
+/// Reloads the caller's callee-saved registers after the invoker body finishes.
+///
+/// The boxed Mixed result travels in the return registers, which these loads never touch.
+fn emit_invoker_callee_saved_restores(emitter: &mut Emitter) {
+    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);
+    }
+}
+
 /// Runtime invoker metadata emitted beside callable descriptors.
 pub(super) struct RuntimeCallableInvoker<'a> {
     pub(super) label: &'a str,
@@ -116,6 +156,10 @@ fn emit_runtime_callable_invoker_impl(
     emitter.raw(".align 2");
     emitter.label_global(invoker.label);
     abi::emit_frame_prologue(emitter, 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).
+    emit_invoker_callee_saved_saves(emitter);
     abi::store_at_offset(
         emitter,
         abi::int_arg_reg_name(emitter.target, 0),
@@ -156,12 +200,16 @@ fn emit_runtime_callable_invoker_impl(
     if catch_native_throws {
         emit_invoker_exception_boundary_pop(emitter, INVOKER_BOUNDARY_BASE_OFFSET);
     }
+    // Restore before tearing down the frame (and on the escape path below): the boxed
+    // Mixed result travels in return registers, which these loads never touch.
+    emit_invoker_callee_saved_restores(emitter);
     abi::emit_frame_restore(emitter, frame_size);
     abi::emit_return(emitter);
     if catch_native_throws {
         emitter.label(&escape_label);
         emit_invoker_exception_boundary_pop(emitter, INVOKER_BOUNDARY_BASE_OFFSET);
         emit_null_invoker_result(emitter);
+        emit_invoker_callee_saved_restores(emitter);
         abi::emit_frame_restore(emitter, frame_size);
         abi::emit_return(emitter);
     }
@@ -204,14 +252,14 @@ fn emit_invoker_exception_boundary_push(
                 "stur x10, [x29, #-{}]",
                 handler_base - TRY_HANDLER_DIAG_DEPTH_OFFSET
             )); // save diagnostic suppression depth for restoration
-            emitter.instruction(&format!("sub x10, x29, #{}", handler_base)); // compute the boundary handler record address
+            emitter.instruction(&format!("sub x10, x29, #{}", handler_base));   // compute the boundary handler record address
             abi::emit_store_reg_to_symbol(emitter, "x10", "_exc_handler_top", 0);
             emitter.instruction(&format!(
                 "sub x0, x29, #{}",
                 handler_base - TRY_HANDLER_JMP_BUF_OFFSET
             )); // pass the boundary jmp_buf to setjmp
             emitter.bl_c("setjmp"); // snapshot the bridge stack before entering the callable
-            emitter.instruction(&format!("cbnz x0, {}", escape_label)); // non-zero setjmp result means a callable Throwable escaped
+            emitter.instruction(&format!("cbnz x0, {}", escape_label));         // non-zero setjmp result means a callable Throwable escaped
         }
         Arch::X86_64 => {
             abi::emit_load_symbol_to_reg(emitter, "r10", "_exc_handler_top", 0);
@@ -230,8 +278,8 @@ fn emit_invoker_exception_boundary_push(
                 handler_base - TRY_HANDLER_JMP_BUF_OFFSET
             )); // pass the boundary jmp_buf to setjmp
             emitter.bl_c("setjmp"); // snapshot the bridge stack before entering the callable
-            emitter.instruction("test eax, eax"); // did control arrive through longjmp?
-            emitter.instruction(&format!("jne {}", escape_label)); // non-zero setjmp result means a callable Throwable escaped
+            emitter.instruction("test eax, eax");                               // did control arrive through longjmp?
+            emitter.instruction(&format!("jne {}", escape_label));              // non-zero setjmp result means a callable Throwable escaped
         }
     }
 }
@@ -265,10 +313,10 @@ fn emit_invoker_exception_boundary_pop(emitter: &mut Emitter, handler_base: usiz
 fn emit_null_invoker_result(emitter: &mut Emitter) {
     match emitter.target.arch {
         Arch::AArch64 => {
-            emitter.instruction("mov x0, xzr"); // return null so magician takes the pending Throwable
+            emitter.instruction("mov x0, xzr");                                 // return null so magician takes the pending Throwable
         }
         Arch::X86_64 => {
-            emitter.instruction("xor eax, eax"); // return null so magician takes the pending Throwable
+            emitter.instruction("xor eax, eax");                                // return null so magician takes the pending Throwable
         }
     }
 }

src/codegen_support/abi/calls/incoming.rs

@@ -8,13 +8,13 @@
 //! Key details:
 //! - Incoming cursor state must match outgoing assignment rules or calls will corrupt frame slots.
 
-use crate::codegen_support::{emit::Emitter, platform::Arch};
+use crate::codegen_support::emit::Emitter;
 use crate::types::PhpType;
 
 use super::super::frame::{load_from_caller_stack, store_at_offset};
 use super::super::registers::{
-    float_arg_reg_limit, float_arg_reg_name, int_arg_reg_limit, int_arg_reg_name,
-    secondary_scratch_reg, tertiary_scratch_reg, IncomingArgCursor,
+    float_arg_reg_limit, float_arg_reg_name, float_spill_scratch_reg, int_arg_reg_limit,
+    int_arg_reg_name, secondary_scratch_reg, tertiary_scratch_reg, IncomingArgCursor,
 };
 
 /// Stores a function parameter from the next available ABI register or caller stack slot
@@ -34,10 +34,7 @@ pub fn emit_store_incoming_param(
     cursor: &mut IncomingArgCursor,
 ) {
     let ty = ty.codegen_repr();
-    let float_spill_reg = match emitter.target.arch {
-        Arch::AArch64 => "d15",
-        Arch::X86_64 => "xmm15",
-    };
+    let float_spill_reg = float_spill_scratch_reg(emitter.target);
     let int_spill_reg = secondary_scratch_reg(emitter);
     let int_hi_spill_reg = tertiary_scratch_reg(emitter);
     let int_reg_limit = int_arg_reg_limit(emitter.target);

src/codegen_support/abi/calls/outgoing.rs

@@ -16,8 +16,9 @@ use crate::types::PhpType;
 
 use super::super::frame::emit_adjust_sp;
 use super::super::registers::{
-    float_arg_reg_limit, float_arg_reg_name, int_arg_reg_limit, int_arg_reg_name,
-    secondary_scratch_reg, tertiary_scratch_reg, OutgoingArgAssignment, STACK_ARG_SENTINEL,
+    float_arg_reg_limit, float_arg_reg_name, float_spill_scratch_reg, int_arg_reg_limit,
+    int_arg_reg_name, secondary_scratch_reg, tertiary_scratch_reg, OutgoingArgAssignment,
+    STACK_ARG_SENTINEL,
 };
 use super::stack::{emit_load_temporary_stack_slot, emit_store_to_sp};
 
@@ -109,10 +110,7 @@ fn emit_copy_stack_arg_slot(
         Arch::AArch64 => tertiary_scratch_reg(emitter),
         Arch::X86_64 => "r11",
     };
-    let float_reg = match emitter.target.arch {
-        Arch::AArch64 => "d15",
-        Arch::X86_64 => "xmm15",
-    };
+    let float_reg = float_spill_scratch_reg(emitter.target);
     match ty {
         PhpType::Float => {
             emit_load_temporary_stack_slot(emitter, float_reg, src_offset);

src/codegen_support/abi/mod.rs

@@ -40,8 +40,9 @@ pub use frame::{
 #[cfg(test)]
 pub use frame::{emit_preserve_return_value, emit_restore_return_value};
 pub(crate) use registers::{
-    float_arg_reg_name, float_result_reg, int_arg_reg_name, int_result_reg, secondary_scratch_reg,
-    string_result_regs, symbol_scratch_reg, tertiary_scratch_reg,
+    float_arg_reg_name, float_result_reg, float_spill_scratch_reg, int_arg_reg_name,
+    int_result_reg, secondary_scratch_reg, string_result_regs, symbol_scratch_reg,
+    tertiary_scratch_reg,
 };
 pub use registers::{
     nested_call_reg, process_argc_reg, process_argv_reg, temp_int_reg, IncomingArgCursor,

src/codegen_support/abi/registers.rs

@@ -67,6 +67,15 @@ pub(crate) fn float_arg_reg_name(target: Target, idx: usize) -> &'static str {
     }
 }
 
+/// Returns the caller-saved floating-point scratch register reserved for ABI spill staging.
+/// AArch64 uses d31 so staging never clobbers the callee-saved d8-d15 range; x86_64 uses xmm15.
+pub(crate) fn float_spill_scratch_reg(target: Target) -> &'static str {
+    match target.arch {
+        Arch::AArch64 => "d31",
+        Arch::X86_64 => "xmm15",
+    }
+}
+
 /// Returns the register used to hold integer/pointer function results.
 /// AArch64: x0. x86_64: rax.
 pub(crate) fn int_result_reg(emitter: &Emitter) -> &'static str {

src/codegen_support/abi/tests/arguments.rs

@@ -32,6 +32,32 @@ fn test_emit_store_incoming_param_uses_registers_then_caller_stack() {
     assert!(out.contains("    ldr x10, [x29, #32]\n"));
 }
 
+/// Verifies that an incoming AArch64 float overflow uses caller-saved d31 rather than d15.
+#[test]
+fn test_emit_store_incoming_float_overflow_uses_volatile_scratch() {
+    let mut emitter = test_emitter();
+    let mut cursor = IncomingArgCursor::default();
+
+    for (index, name) in ["a", "b", "c", "d", "e", "f", "g", "h", "i"]
+        .iter()
+        .enumerate()
+    {
+        emit_store_incoming_param(
+            &mut emitter,
+            name,
+            &PhpType::Float,
+            (index + 1) * 8,
+            false,
+            &mut cursor,
+        );
+    }
+
+    let out = emitter.output();
+    assert!(out.contains("    ; param $i from caller stack +32\n"));
+    assert!(out.contains("    ldr d31, [x29, #32]\n"));
+    assert!(!out.contains("d15"));
+}
+
 /// Tests that `emit_frame_slot_address` correctly handles offsets larger than
 /// a single immediate instruction can encode by emitting multiple sub instructions
 /// to reach offsets like 5000 on ARM64.
@@ -123,6 +149,26 @@ fn test_materialize_outgoing_args_keeps_overflow_on_stack() {
     assert!(out.contains("    add sp, sp, #160\n"));
 }
 
+/// Verifies that outgoing AArch64 float overflow staging never clobbers callee-saved d15.
+#[test]
+fn test_materialize_outgoing_float_overflow_uses_volatile_scratch() {
+    let mut emitter = test_emitter();
+    let arg_types = vec![PhpType::Float; 9];
+    let assignments = build_outgoing_arg_assignments_for_target(
+        Target::new(Platform::MacOS, Arch::AArch64),
+        &arg_types,
+        0,
+    );
+
+    let overflow_bytes = materialize_outgoing_args(&mut emitter, &assignments);
+    let out = emitter.output();
+
+    assert_eq!(overflow_bytes, 16);
+    assert!(out.contains("    ldr d31, [sp, #32]\n"));
+    assert!(out.contains("    str d31, [sp]\n"));
+    assert!(!out.contains("d15"));
+}
+
 /// Tests that `emit_store_local_slot_to_symbol` handles string slots with large
 /// offsets (>4095) by emitting the necessary adrp/add page calculations and
 /// decomposed sub instructions to reach the slot, then stores both x10 and x11