From 9baa75438b563f3f7379f1c28130f574e15a0f0f Mon Sep 17 00:00:00 2001 From: mirchaemanuel Date: Mon, 6 Jul 2026 15:12:18 +0200 Subject: [PATCH 1/4] fix(ir,codegen): release caught exceptions through the owned-local lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #448. Every handled throw leaked its throwable (~48 bytes per catch): the catch binding moved the in-flight exception (refcount 1, owned by _exc_value) into the variable's slot via Op::CatchBind — a raw store the cleanup machinery never saw. The frame epilogue releases only slots written by Op::StoreLocal, so the catch slot escaped rebind release, function-epilogue release, and program-exit release alike. Three coordinated changes: - ir_lower lower_catch_bind: the bound slot now follows the owned-local lifecycle — the previous binding is released before the rebind (slot is zero-initialized, so the first pass is a runtime no-op) and the local is marked initialized so later explicit stores release the old value. A variable-less catch consumes the reference through a hidden owned temporary instead of dropping it. - ir_lower lower_throw: throwing a borrowed value (a load of an owned local, e.g. rethrowing the caught variable with 'throw $e') retains it, so inner and outer bindings each own a reference; throwing an owning temporary ('throw new E()', 'throw f()') still transfers its reference. - codegen_ir frame: local_slot_has_store also recognizes Op::CatchBind, so catch-bound slots join the epilogue cleanup set and are zero-initialized in the prologue (catch-not-taken paths stay safe). Rethrow chains, aliasing, finally (both paths), nested try, and escape via array retention verified balanced. Regression tests in tests/codegen/runtime_gc/regressions.rs. Known pre-existing edges (unchanged, filed separately): 'return $e' from inside a catch does not compile, and reassigning the catch variable to a new object mis-types the previous-value release as Mixed and leaks. --- CHANGELOG.md | 1 + src/codegen_ir/frame.rs | 16 +- src/ir_lower/stmt/mod.rs | 39 ++++- tests/codegen/runtime_gc/regressions.rs | 198 ++++++++++++++++++++++++ 4 files changed, 241 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b31f875751..009a8e57ab 100644 --- a/CHANGELOG.md +++ b/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 the caught-exception leak (issue #448): every handled `throw` leaked its throwable (~48 bytes per `catch`), because the catch binding moved the in-flight exception into the variable's slot without ever scheduling a release — the slot escaped rebind, function-epilogue, and program-exit cleanup alike. Catch-bound slots now follow the ordinary owned-local lifecycle (released on rebind and at the frame epilogue, zero-initialized so untaken catch paths stay safe), a variable-less `catch` consumes the reference through a hidden temporary, and rethrowing a caught variable (`throw $e`) retains it so inner and outer bindings each own their reference. Long-running throw/catch loops now keep a flat heap. - Fixed a constant-propagation miscompile with by-reference calls in a `match` subject (issue #384): `echo match(bump($i)) { ... } . "|" . $i` kept the pre-call constant for `$i` and printed the stale value, because a call's unknown write set was treated as "no writes" instead of forcing conservative invalidation. A read sequenced after any by-reference-mutating call in the same expression now observes the post-call value, matching PHP; pure calls (`gettype`, `strlen`, …) keep their operands foldable. - Fixed PHP parser/codegen parity for parenthesis-free object instantiation (issue #371): `new Foo`, `new self`, `new static`, `new parent`, and `new $class` now compile with empty constructor arguments, while immediate postfix forms such as `new Foo->bar`, `new Foo::bar()`, `new Foo?->bar`, and `new Foo[0]` are rejected instead of being misparsed. - Fixed three PHP parity regressions in the EIR backend: indexed array elements such as `$a[0]` can now be passed to by-reference parameters with copy-on-write storage split before mutation (issue #360); `IteratorAggregate::getIterator()` may declare the marker `Traversable` return type while `foreach` still dispatches through the returned object's concrete `Iterator` methods (issue #385); and catchable private/protected method access plus readonly-property write errors now preserve PHP's receiver/RHS evaluation order before throwing `Error` (issue #383). The merge also removes a duplicate `_spl_error_class_id` data symbol that could make post-merge user assembly fail to assemble. diff --git a/src/codegen_ir/frame.rs b/src/codegen_ir/frame.rs index ea18af5e00..44203b8fae 100644 --- a/src/codegen_ir/frame.rs +++ b/src/codegen_ir/frame.rs @@ -346,7 +346,8 @@ fn emit_main_local_epilogue_cleanup(ctx: &mut FunctionContext<'_>) { } } -/// Returns main local slots that receive owned refcounted values through `StoreLocal`. +/// Returns main local slots that receive owned refcounted values through `StoreLocal` +/// or take ownership of a caught exception through `CatchBind`. fn main_cleanup_locals(ctx: &FunctionContext<'_>) -> Vec<(String, LocalSlotId, PhpType, usize)> { let param_names = ctx .function @@ -449,10 +450,14 @@ fn ref_cell_owner_locals(ctx: &FunctionContext<'_>) -> Vec<(String, LocalSlotId, locals } -/// Returns true when a local slot is written by an explicit EIR `StoreLocal`. +/// Returns true when a local slot is written by an explicit EIR `StoreLocal` or takes +/// ownership of the in-flight exception through a `CatchBind` (issue #448). Both make +/// the slot own a reference that the epilogue must release (and that the prologue must +/// zero-initialize, since a catch may never fire on some paths). fn local_slot_has_store(function: &Function, slot: LocalSlotId) -> bool { function.instructions.iter().any(|inst| { - inst.op == Op::StoreLocal && matches!(inst.immediate, Some(Immediate::LocalSlot(candidate)) if candidate == slot) + matches!(inst.op, Op::StoreLocal | Op::CatchBind) + && matches!(inst.immediate, Some(Immediate::LocalSlot(candidate)) if candidate == slot) }) } @@ -557,7 +562,8 @@ fn emit_function_local_epilogue_cleanup(ctx: &mut FunctionContext<'_>) { } } -/// Returns function local slots that receive owned refcounted values through `StoreLocal`. +/// Returns function local slots that receive owned refcounted values through `StoreLocal` +/// or take ownership of a caught exception through `CatchBind`. /// /// `include_returned` controls whether slots directly returned by a `Return` terminator are /// part of the set. The zero-initialization pass requests them (`true`) so a returned slot @@ -607,7 +613,7 @@ fn function_cleanup_locals( locals } -/// Returns whether a local kind can own values through ordinary `StoreLocal`. +/// Returns whether a local kind can own values through ordinary `StoreLocal` or `CatchBind`. fn local_kind_needs_epilogue_cleanup(kind: LocalKind) -> bool { matches!( kind, diff --git a/src/ir_lower/stmt/mod.rs b/src/ir_lower/stmt/mod.rs index bef17b5bba..15e31162d5 100644 --- a/src/ir_lower/stmt/mod.rs +++ b/src/ir_lower/stmt/mod.rs @@ -1465,6 +1465,17 @@ fn lower_include_once_guard(ctx: &mut LoweringContext<'_, '_>, label: &str, body /// Lowers a throwing statement into a terminator. fn lower_throw(ctx: &mut LoweringContext<'_, '_>, expr: &Expr) { let value = lower_expr(ctx, expr); + // The in-flight exception cell owns one reference to the thrown object. Throwing + // an owning temporary (e.g. `throw new E()`, `throw f()`) transfers that + // reference; throwing a borrowed value — a load of an owned local such as a + // rethrown catch variable (`throw $e`) — must retain it, so the local's own + // release (rebind or epilogue) stays balanced with the catch-side release of + // the in-flight reference (issue #448). + let value = if ctx.value_is_owning_temporary(value) { + value + } else { + crate::ir_lower::ownership::acquire_if_refcounted(ctx, value, Some(expr.span)) + }; terminate_throw(ctx, value.value); } @@ -1703,18 +1714,30 @@ fn lower_current_exception(ctx: &mut LoweringContext<'_, '_>, span: Span) -> Low ) } -/// Binds and clears the active exception for a matched catch clause. +/// Binds and clears the active exception for a matched catch clause. The bound slot +/// takes over the in-flight reference (the runtime cell is zeroed by the bind), so it +/// participates in the owned-local lifecycle: the previous binding is released before +/// the rebind, and the frame epilogue releases whatever the slot still holds (issue +/// #448). A variable-less catch consumes the reference through a hidden owned +/// temporary so it flows through the same lifecycle instead of leaking. fn lower_catch_bind(ctx: &mut LoweringContext<'_, '_>, catch: &CatchClause, span: Span) { - let (immediate, php_type) = catch.variable.as_ref().map_or((None, PhpType::Void), |variable| { - let php_type = catch_variable_type(catch); - let slot = ctx.declare_local(variable, php_type.clone()); - ctx.set_local_type(variable, php_type.clone()); - (Some(Immediate::LocalSlot(slot)), php_type) - }); + let php_type = catch_variable_type(catch); + let variable = match catch.variable.as_ref() { + Some(variable) => variable.clone(), + None => ctx.declare_owned_hidden_temp(php_type.clone()), + }; + let slot = ctx.declare_local(&variable, php_type.clone()); + ctx.set_local_type(&variable, php_type.clone()); + // Release the slot's previous exception before rebinding: a catch that fires + // again (next loop iteration, later try block) would otherwise overwrite the + // only owned reference. Catch-bound slots are zero-initialized in the prologue, + // so the release is a runtime no-op the first time through. + ctx.release_stored_local_value(&variable, slot, Some(span)); + ctx.mark_local_initialized(&variable); ctx.builder.emit_with_effects( Op::CatchBind, Vec::new(), - immediate, + Some(Immediate::LocalSlot(slot)), IrType::Void, php_type, Ownership::NonHeap, diff --git a/tests/codegen/runtime_gc/regressions.rs b/tests/codegen/runtime_gc/regressions.rs index c871eac725..59f70cdea1 100644 --- a/tests/codegen/runtime_gc/regressions.rs +++ b/tests/codegen/runtime_gc/regressions.rs @@ -1083,3 +1083,201 @@ echo "done"; "promoting an indexed array literal to hash storage must free the source array (issue #408)" ); } + +/// Regression for #448: a caught exception object must be released once the catch +/// handler is done with it. Before the fix the catch binding moved the throwable into +/// the variable's slot without ever scheduling a release (the slot is written by +/// `CatchBind`, not `StoreLocal`, so it escaped every cleanup point), leaking one +/// ~48-byte block per catch. A throw/catch loop must leave the heap clean. +#[test] +fn test_regression_caught_exception_released_per_catch() { + let out = compile_and_run_with_heap_debug( + r#"getMessage()); +"#, + ); + assert!(out.success, "program failed: {}", out.stderr); + assert_eq!(out.stdout, "5|4"); + // Exactly the 5 escaped exceptions remain live: the retaining array itself is + // released by main's epilogue (its element teardown semantics are pre-existing + // and identical to a no-try control pushing fresh objects). The catch slot's own + // cleanup must not free them a second time — a double free would abort the run + // with a bad-refcount fatal before this assertion. + assert!( + out.stderr.contains("HEAP DEBUG: leak summary: live_blocks=5"), + "expected exactly the 5 escaped exceptions to remain live, got: {}", + out.stderr + ); +} + +/// Regression for #448: `finally` on both paths — a caught exception with a finally +/// block, and a propagating exception whose finally runs before the outer catch — +/// must keep the heap flat like the plain catch shapes. +#[test] +fn test_regression_finally_paths_release_exception() { + let out = compile_and_run_with_heap_debug( + r#" Date: Mon, 6 Jul 2026 21:58:35 +0200 Subject: [PATCH 2/4] fix(runtime): x86_64 release helpers accept throwable heap kind 6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The linux-x86_64 CI failures on this PR's regression tests were not an emission defect (the PR's codegen is symmetric across targets) but a latent gate in the pre-existing x86_64 runtime: __rt_decref_object and the __rt_object_free_deep validator only accepted heap kind 4 (plain object), and __rt_decref_any had no kind-6 arm, while throwables are stamped heap kind 6. The ARM64 variants have no kind gate, so exception releases worked there. On x86_64 every release of a caught exception was a silent no-op (the incref path checks only the heap magic, so refcounts only ever went up) — latent while catch bindings were never released, exposed the moment this PR made those releases real. The three x86_64 release paths now treat kind 6 like kind 4, mirroring ARM64: decref_object and the deep-free validator accept both kinds, and decref_any dispatches kind 6 to the object decref helper. Verified on real linux-x86_64 (Docker): the previously-failing regression tests now pass (per-catch release, unbound catch, rethrow chain, multi-type catch); macOS ARM64 suite unchanged (8/8). Related follow-up filed separately: several x86_64 emitters stamp a transposed heap magic (0x4548504c vs canonical 0x454C5048), which makes refcount ops no-op for objects created through those paths (issue #482). --- src/codegen/runtime/arrays/decref_any.rs | 2 ++ src/codegen/runtime/arrays/decref_object.rs | 6 +++++- src/codegen/runtime/arrays/object_free_deep.rs | 6 +++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/codegen/runtime/arrays/decref_any.rs b/src/codegen/runtime/arrays/decref_any.rs index 9f10d80b61..018f44c8ee 100644 --- a/src/codegen/runtime/arrays/decref_any.rs +++ b/src/codegen/runtime/arrays/decref_any.rs @@ -133,6 +133,8 @@ fn emit_decref_any_linux_x86_64(emitter: &mut Emitter) { emitter.instruction("je __rt_decref_any_object"); // objects release through the x86_64 object decref helper emitter.instruction("cmp r10, 5"); // does this heap-backed payload point at a boxed mixed cell? emitter.instruction("je __rt_decref_any_mixed"); // mixed cells release through the x86_64 mixed decref helper + emitter.instruction("cmp r10, 6"); // does this heap-backed payload point at a throwable object (issue #448)? + emitter.instruction("je __rt_decref_any_object"); // throwables release through the x86_64 object decref helper like plain objects emitter.instruction("jmp __rt_decref_any_done"); // unknown/raw heap kinds need no release work in the current x86_64 bootstrap runtime emitter.label("__rt_decref_any_string"); diff --git a/src/codegen/runtime/arrays/decref_object.rs b/src/codegen/runtime/arrays/decref_object.rs index 8b6cc978e6..6bac63ef14 100644 --- a/src/codegen/runtime/arrays/decref_object.rs +++ b/src/codegen/runtime/arrays/decref_object.rs @@ -110,8 +110,12 @@ fn emit_decref_object_linux_x86_64(emitter: &mut Emitter) { emitter.instruction(&format!("cmp r11d, 0x{:x}", X86_64_HEAP_MAGIC_HI32)); // ignore foreign pointers that do not carry the elephc x86_64 heap marker emitter.instruction("jne __rt_decref_object_skip"); // only elephc-owned objects participate in x86_64 decref bookkeeping emitter.instruction("and r10, 0xff"); // isolate the low-byte uniform heap kind tag for a final ownership sanity check - emitter.instruction("cmp r10, 4"); // is this heap-backed payload really an object instance? + emitter.instruction("cmp r10, 4"); // is this heap-backed payload a plain object instance? + emitter.instruction("je __rt_decref_object_counted"); // plain objects release through the shared refcount bookkeeping + emitter.instruction("cmp r10, 6"); // is this heap-backed payload a throwable object instance (issue #448)? emitter.instruction("jne __rt_decref_object_skip"); // other heap kinds must not be released through the object decref helper + + emitter.label("__rt_decref_object_counted"); emitter.instruction("mov r10d, DWORD PTR [rax - 12]"); // load the 32-bit object refcount from the uniform heap header emitter.instruction("sub r10d, 1"); // decrement the object refcount for the releasing x86_64 owner emitter.instruction("mov DWORD PTR [rax - 12], r10d"); // store the decremented object refcount back into the uniform heap header diff --git a/src/codegen/runtime/arrays/object_free_deep.rs b/src/codegen/runtime/arrays/object_free_deep.rs index 5a8ceda5e1..5c18eee0cc 100644 --- a/src/codegen/runtime/arrays/object_free_deep.rs +++ b/src/codegen/runtime/arrays/object_free_deep.rs @@ -283,8 +283,12 @@ fn emit_object_free_deep_linux_x86_64(emitter: &mut Emitter) { emitter.instruction(&format!("cmp r11d, 0x{:x}", X86_64_HEAP_MAGIC_HI32)); // ignore foreign pointers that do not carry the elephc x86_64 heap marker emitter.instruction("jne __rt_object_free_deep_done"); // only elephc-owned objects participate in x86_64 deep-free bookkeeping emitter.instruction("and r10, 0xff"); // isolate the low-byte uniform heap kind tag for a final ownership sanity check - emitter.instruction("cmp r10, 4"); // is this heap-backed payload really an object instance? + emitter.instruction("cmp r10, 4"); // is this heap-backed payload a plain object instance? + emitter.instruction("je __rt_object_free_deep_kind_ok"); // plain objects release through the shared deep-free walk + emitter.instruction("cmp r10, 6"); // is this heap-backed payload a throwable object instance (issue #448)? emitter.instruction("jne __rt_object_free_deep_done"); // other heap kinds must not be released through the object deep-free helper + + emitter.label("__rt_object_free_deep_kind_ok"); emitter.instruction("push rbp"); // preserve the caller frame pointer before reserving object deep-free spill slots emitter.instruction("mov rbp, rsp"); // establish a stable frame base for the saved object pointer, descriptor pointer, count, and loop index emitter.instruction("sub rsp, 32"); // reserve local storage for the object pointer, descriptor pointer, property count, and loop index From 3c3547918fd83710b91268364a3b2b4abe53ea83 Mon Sep 17 00:00:00 2001 From: Vincenzo Petrucci Date: Sun, 19 Jul 2026 19:05:27 +0200 Subject: [PATCH 3/4] fix(codegen): make catch ownership storage-aware --- src/codegen/context.rs | 1 + src/codegen/frame.rs | 8 +- src/codegen/local_analysis.rs | 132 +++++++++++++++++- src/codegen/lower_inst.rs | 35 +---- .../runtime/arrays/decref_any.rs | 41 +++++- src/ir/instr.rs | 6 +- src/ir_lower/context.rs | 51 ++++++- src/ir_lower/stmt/mod.rs | 29 ++-- tests/codegen/runtime_gc/regressions.rs | 91 +++++++++++- 9 files changed, 322 insertions(+), 72 deletions(-) diff --git a/src/codegen/context.rs b/src/codegen/context.rs index 2838885ad2..9aa897b852 100644 --- a/src/codegen/context.rs +++ b/src/codegen/context.rs @@ -893,6 +893,7 @@ impl<'a> FunctionContext<'a> { | Op::CallableArrayNew | Op::BufferNew | Op::GeneratorNew + | Op::CatchBind | Op::Call | Op::FunctionVariantCall | Op::BuiltinCall diff --git a/src/codegen/frame.rs b/src/codegen/frame.rs index 9feed50da9..b46e89bbcb 100644 --- a/src/codegen/frame.rs +++ b/src/codegen/frame.rs @@ -477,8 +477,7 @@ fn emit_main_local_epilogue_cleanup(ctx: &mut FunctionContext<'_>) { emit_eval_context_epilogue_cleanup(ctx); } -/// Returns main local slots that receive owned refcounted values through `StoreLocal` -/// or take ownership of a caught exception through `CatchBind`. +/// Returns main local slots that receive owned refcounted values through `StoreLocal`. fn main_cleanup_locals(ctx: &FunctionContext<'_>) -> Vec<(String, LocalSlotId, PhpType, usize)> { let param_names = ctx .function @@ -816,8 +815,7 @@ fn emit_function_local_epilogue_cleanup( } } -/// Returns function local slots that receive owned refcounted values through `StoreLocal` -/// or take ownership of a caught exception through `CatchBind`. +/// Returns function local slots that receive owned refcounted values through `StoreLocal`. /// /// `skip_return_slot` excludes the one local whose refcounted owner is transferred by the /// current return terminator. It is deliberately path-local: another `return` in the same @@ -902,7 +900,7 @@ pub(super) fn emit_owned_local_cleanup( } } -/// Returns whether a local kind can own values through ordinary `StoreLocal` or `CatchBind`. +/// Returns whether a local kind can own values through ordinary `StoreLocal`. fn local_kind_needs_epilogue_cleanup(kind: LocalKind) -> bool { matches!( kind, diff --git a/src/codegen/local_analysis.rs b/src/codegen/local_analysis.rs index 251e6084c9..4288f1a71e 100644 --- a/src/codegen/local_analysis.rs +++ b/src/codegen/local_analysis.rs @@ -7,10 +7,12 @@ //! //! Key details: //! - Ref-cell state is a forward may-analysis, so later promotions never affect earlier ops. +//! - Runtime exception handlers conservatively seed every potentially promoted slot because +//! their incoming edges are implicit rather than represented by EIR terminators. //! - `UnsetLocal` returns a promoted slot to raw local storage on subsequent paths. //! - Closure, iterator, alias, and explicit binding operations all update representation state. -//! - `CatchBind` counts as a store: the catch slot takes ownership of the in-flight -//! exception and must join epilogue cleanup / prologue zero-init (issue #448). +//! - `CatchBind` produces an owned SSA value; its following storage opcode determines +//! which local lifecycle, if any, participates in cleanup (issue #448). use std::collections::{HashSet, VecDeque}; @@ -36,9 +38,7 @@ impl LocalSlotAnalysis { let mut stored_slots = HashSet::new(); let mut ever_ref_cell_slots = initially_ref_cell_slots.clone(); for inst in &function.instructions { - // CatchBind moves the in-flight exception into the slot (issue #448), so it - // must be treated like StoreLocal for epilogue cleanup and prologue zero-init. - if matches!(inst.op, Op::StoreLocal | Op::CatchBind) { + if inst.op == Op::StoreLocal { if let Some(Immediate::LocalSlot(slot)) = inst.immediate { stored_slots.insert(slot); } @@ -51,7 +51,11 @@ impl LocalSlotAnalysis { } let (release_ops_with_possible_ref_cell, ref_cell_slots_before_inst) = - analyze_release_local_slot_states(function, &initially_ref_cell_slots); + analyze_release_local_slot_states( + function, + &initially_ref_cell_slots, + &ever_ref_cell_slots, + ); let owned_parameter_slots = owned_parameter_slots( function, &stored_slots, @@ -68,7 +72,7 @@ impl LocalSlotAnalysis { } } - /// Returns whether this slot receives an owned value via `StoreLocal` or `CatchBind`. + /// Returns whether this slot receives an owned value via `StoreLocal`. pub(super) fn has_store(&self, slot: LocalSlotId) -> bool { self.stored_slots.contains(&slot) } @@ -159,6 +163,7 @@ fn loaded_local_slot(function: &Function, value: ValueId) -> Option fn analyze_release_local_slot_states( function: &Function, initially_ref_cell_slots: &HashSet, + ever_ref_cell_slots: &HashSet, ) -> (HashSet, HashSet<(InstId, LocalSlotId)>) { if function.blocks.is_empty() { return (HashSet::new(), HashSet::new()); @@ -171,6 +176,16 @@ fn analyze_release_local_slot_states( } block_inputs[entry_index] = Some(initially_ref_cell_slots.clone()); let mut worklist = VecDeque::from([function.entry]); + for handler in exception_handler_blocks(function) { + let handler_index = handler.as_raw() as usize; + if handler_index >= block_inputs.len() { + continue; + } + block_inputs[handler_index] = Some(ever_ref_cell_slots.clone()); + if handler != function.entry { + worklist.push_back(handler); + } + } while let Some(block_id) = worklist.pop_front() { let block_index = block_id.as_raw() as usize; let Some(mut state) = block_inputs[block_index].clone() else { @@ -230,6 +245,24 @@ fn analyze_release_local_slot_states( (releases, ref_cell_slots_before_inst) } +/// Returns blocks entered through runtime exception-handler edges that are absent +/// from ordinary EIR terminators. +fn exception_handler_blocks(function: &Function) -> HashSet { + function + .instructions + .iter() + .filter_map(|inst| { + if inst.op != Op::TryPushHandler { + return None; + } + let Some(Immediate::I64(raw)) = inst.immediate else { + return None; + }; + u32::try_from(raw).ok().map(BlockId::from_raw) + }) + .collect() +} + /// Applies one instruction's local representation change to the forward state. fn apply_ref_cell_transfer( function: &Function, @@ -453,6 +486,91 @@ mod tests { assert!(analysis.inst_may_observe_ref_cell(InstId::from_raw(1), slot)); } + /// Verifies runtime exception-handler edges conservatively preserve ref-cell + /// representation established before the protected region. + #[test] + fn exception_handler_observes_preexisting_ref_cell_aliases() { + let mut function = + Function::new("catch_ref_cell".to_string(), IrType::Void, PhpType::Void); + let value_slot = function.add_local( + Some("value".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Object), + PhpType::Object("LogicException".to_string()), + LocalKind::PhpLocal, + ); + let owner_slot = function.add_local( + None, + IrType::Heap(crate::ir::IrHeapKind::Object), + PhpType::Object("LogicException".to_string()), + LocalKind::RefCell, + ); + let alias_slot = function.add_local( + Some("target".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Object), + PhpType::Object("LogicException".to_string()), + LocalKind::PhpLocal, + ); + let handler_load; + { + let mut builder = Builder::new(&mut function); + let entry = builder.create_named_block("entry", Vec::new()); + let handler = builder.create_named_block("catch", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + builder.emit( + Op::PromoteLocalRefCell, + Vec::new(), + Some(Immediate::LocalSlotPair { + first: value_slot, + second: owner_slot, + }), + IrType::Void, + PhpType::Object("LogicException".to_string()), + Ownership::NonHeap, + ); + builder.emit( + Op::AliasLocalRefCell, + Vec::new(), + Some(Immediate::LocalSlotPair { + first: alias_slot, + second: value_slot, + }), + IrType::Void, + PhpType::Object("LogicException".to_string()), + Ownership::NonHeap, + ); + builder.emit( + Op::TryPushHandler, + Vec::new(), + Some(Immediate::I64(handler.as_raw() as i64)), + IrType::Void, + PhpType::Void, + Ownership::NonHeap, + ); + builder.terminate(Terminator::Unreachable); + + builder.position_at_end(handler); + handler_load = builder.emit( + Op::LoadRefCell, + Vec::new(), + Some(Immediate::LocalSlot(alias_slot)), + IrType::Heap(crate::ir::IrHeapKind::Object), + PhpType::Object("LogicException".to_string()), + Ownership::MaybeOwned, + ); + builder.terminate(Terminator::Return { value: None }); + } + + let analysis = LocalSlotAnalysis::new(&function); + let handler_load = handler_load.expect("load_ref_cell produces an SSA result"); + let load_inst = match function.value(handler_load).expect("load result exists").def { + ValueDef::Instruction { inst, .. } => inst, + ValueDef::BlockParam { .. } => panic!("load result cannot be a block parameter"), + }; + assert!(analysis.inst_may_observe_ref_cell(load_inst, alias_slot)); + assert!(analysis.has_dynamic_ref_cell_state(alias_slot)); + } + /// Verifies a widened scalar parameter stays owned even if optimization removes its stores. #[test] fn widened_parameter_owns_prologue_mixed_box_without_remaining_store() { diff --git a/src/codegen/lower_inst.rs b/src/codegen/lower_inst.rs index 4dff94df3c..877bb96e0a 100644 --- a/src/codegen/lower_inst.rs +++ b/src/codegen/lower_inst.rs @@ -2876,39 +2876,18 @@ fn lower_catch_current(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Res store_if_result(ctx, inst) } -/// Binds the active exception to an optional catch variable and clears the runtime slot. +/// Takes the active exception into an owned SSA result and clears the runtime slot. fn lower_catch_bind(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result<()> { - if let Some(Immediate::LocalSlot(slot)) = inst.immediate { - let storage_ty = ctx.local_php_type(slot)?; - let target_ty = if inst.result_php_type.codegen_repr() == PhpType::Void { - storage_ty.clone() - } else { - inst.result_php_type.codegen_repr() - }; - let offset = ctx.local_offset(slot)?; - abi::emit_load_symbol_to_result(ctx.emitter, "_exc_value", &target_ty); - let store_ty = catch_bind_store_type(ctx, &target_ty, &storage_ty); - abi::emit_store(ctx.emitter, &store_ty, offset); - } + let result = inst + .result + .ok_or_else(|| CodegenIrError::invalid_module("catch_bind missing owned result"))?; + let result_ty = ctx.value_php_type(result)?; + abi::emit_load_symbol_to_result(ctx.emitter, "_exc_value", &result_ty); + ctx.store_result_value(result)?; abi::emit_store_zero_to_symbol(ctx.emitter, "_exc_value", 0); Ok(()) } -/// Returns the local-storage representation used after loading the raw exception object. -fn catch_bind_store_type( - ctx: &mut FunctionContext<'_>, - target_ty: &PhpType, - storage_ty: &PhpType, -) -> PhpType { - let storage_ty = storage_ty.codegen_repr(); - let target_ty = target_ty.codegen_repr(); - if storage_ty == PhpType::Mixed && target_ty != PhpType::Mixed { - emit_box_current_value_as_mixed(ctx.emitter, &target_ty); - return PhpType::Mixed; - } - target_ty -} - /// Lowers a direct instance-method call on a statically known object receiver. fn lower_method_call(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result<()> { let object = expect_operand(inst, 0)?; diff --git a/src/codegen_support/runtime/arrays/decref_any.rs b/src/codegen_support/runtime/arrays/decref_any.rs index 7df8b05f40..837caad932 100644 --- a/src/codegen_support/runtime/arrays/decref_any.rs +++ b/src/codegen_support/runtime/arrays/decref_any.rs @@ -19,7 +19,7 @@ const X86_64_HEAP_MAGIC_HI32: u64 = 0x454C5048; /// /// Reads the heap kind tag from the value's header, validates the pointer against the /// managed heap window, and dispatches to the appropriate concrete release helper -/// (string, array, hash, object, mixed). Skips GC-tracked children during active cycle +/// (string, array, hash, object/throwable, mixed). Skips GC-tracked children during active cycle /// collection to avoid double-frees when the collector reclaims them directly. /// /// Input: x0 (ARM64) or rax (x86_64) = heap-backed value pointer @@ -55,7 +55,7 @@ pub fn emit_decref_any(emitter: &mut Emitter) { emitter.instruction("and x13, x11, #0xff"); // isolate the low-byte heap kind tag emitter.instruction("cmp x13, #2"); // is this a refcounted indexed array? emitter.instruction("b.lo __rt_decref_any_dispatch"); // strings should still be freed immediately - emitter.instruction("cmp x13, #5"); // is this within the refcounted array/hash/object/mixed range? + emitter.instruction("cmp x13, #6"); // is this within the refcounted array/hash/object/mixed/throwable range? emitter.instruction("b.hi __rt_decref_any_dispatch"); // raw/untyped blocks are not part of refcounted graph cleanup emitter.instruction("mov x14, #1"); // prepare a single-bit reachable mask emitter.instruction("lsl x14, x14, #16"); // x14 = GC reachable bit in the kind word @@ -75,6 +75,8 @@ pub fn emit_decref_any(emitter: &mut Emitter) { emitter.instruction("b.eq __rt_decref_any_object"); // release objects through __rt_decref_object emitter.instruction("cmp x11, #5"); // is this a boxed mixed value? emitter.instruction("b.eq __rt_decref_any_mixed"); // release mixed cells through __rt_decref_mixed + emitter.instruction("cmp x11, #6"); // is this a throwable object? + emitter.instruction("b.eq __rt_decref_any_object"); // release throwables through the object decref helper emitter.instruction("ret"); // unknown/raw kinds need no release emitter.label("__rt_decref_any_string"); @@ -99,7 +101,7 @@ pub fn emit_decref_any(emitter: &mut Emitter) { /// x86_64 Linux implementation of the uniform release dispatcher. /// Uses the x86_64 heap wrapper header format with a high-word magic marker to distinguish /// managed heap payloads from foreign or static pointers before dispatching to concrete -/// release helpers (string, array, hash, object, mixed). +/// release helpers (string, array, hash, object/throwable, mixed). /// Input: rax = heap-backed value pointer /// Output: none (tail-calls to specialized release helpers) fn emit_decref_any_linux_x86_64(emitter: &mut Emitter) { @@ -155,3 +157,36 @@ fn emit_decref_any_linux_x86_64(emitter: &mut Emitter) { emitter.label("__rt_decref_any_done"); emitter.instruction("ret"); // nothing to release for null, foreign, or unsupported heap kinds } + +#[cfg(test)] +mod tests { + use super::*; + use crate::codegen_support::platform::{Platform, Target}; + + /// Verifies every supported architecture routes throwable heap kind 6 through + /// object release, including ARM64's active-cycle collection range. + #[test] + fn test_decref_any_dispatches_throwable_kind_on_all_supported_architectures() { + for platform in [Platform::MacOS, Platform::Linux] { + let mut emitter = Emitter::new(Target::new(platform, Arch::AArch64)); + emit_decref_any(&mut emitter); + let assembly = emitter.output(); + assert!( + assembly.contains(" cmp x13, #6\n"), + "ARM64 collector range excludes throwables on {platform:?}:\n{assembly}" + ); + assert!( + assembly.contains(" cmp x11, #6\n b.eq __rt_decref_any_object\n"), + "ARM64 dispatcher excludes throwables on {platform:?}:\n{assembly}" + ); + } + + let mut emitter = Emitter::new(Target::new(Platform::Linux, Arch::X86_64)); + emit_decref_any(&mut emitter); + let assembly = emitter.output(); + assert!( + assembly.contains(" cmp r10, 6\n je __rt_decref_any_object\n"), + "x86_64 dispatcher excludes throwables:\n{assembly}" + ); + } +} diff --git a/src/ir/instr.rs b/src/ir/instr.rs index 2c9542d61e..2e3593d086 100644 --- a/src/ir/instr.rs +++ b/src/ir/instr.rs @@ -507,8 +507,9 @@ impl Op { ConstEnumCase => E::ALLOC_HEAP, LoadCalledClassId => E::READS_LOCAL, LoadLocal | LoadRefCell | LoadStaticLocal | ClosureCapture => E::READS_LOCAL, - StoreLocal | UnsetLocal | StoreRefCell | ListUnpack | CatchBind | FinallyEnter - | FinallyExit => E::WRITES_LOCAL, + StoreLocal | UnsetLocal | StoreRefCell | ListUnpack | FinallyEnter | FinallyExit => { + E::WRITES_LOCAL + } PromoteLocalRefCell => { E::READS_LOCAL | E::WRITES_LOCAL | E::ALLOC_HEAP | E::WRITES_HEAP | E::REFCOUNT_OP } @@ -526,6 +527,7 @@ impl Op { | ClassAttrArgs | ClassGetAttributes | CatchCurrent => E::READS_GLOBAL, + CatchBind => E::READS_GLOBAL | E::WRITES_GLOBAL, StoreGlobal | StoreStaticLocal | StoreStaticProperty diff --git a/src/ir_lower/context.rs b/src/ir_lower/context.rs index bbee2070ac..e25496d445 100644 --- a/src/ir_lower/context.rs +++ b/src/ir_lower/context.rs @@ -1023,19 +1023,26 @@ impl<'m, 'f> LoweringContext<'m, 'f> { self.builder.widen_local_storage_type(slot, widen_type); let source = value; let source_is_owning_temporary = self.value_is_owning_temporary(value); + let transfer_catch_source_to_store = matches!( + self.builder.value_defining_op(value.value), + Some(Op::CatchBind) + ) && previous_kind != LocalKind::StaticLocal; let release_source_after_store = self.value_needs_release_after_retaining_store(value) - && !matches!(previous_kind, LocalKind::HiddenTemp | LocalKind::OwnedTemp); + && !matches!(previous_kind, LocalKind::HiddenTemp | LocalKind::OwnedTemp) + && !transfer_catch_source_to_store; let transfer_callable_source_to_store = source_is_owning_temporary && matches!(php_type.codegen_repr(), PhpType::Callable); + let transfer_source_to_store = + transfer_callable_source_to_store || transfer_catch_source_to_store; // Retain before cleanup because a borrowed result can alias the old slot. let value = if (uses_global || previous_kind == LocalKind::PhpLocal) - && !transfer_callable_source_to_store + && !transfer_source_to_store && !self.is_ref_bound_local(name) { crate::ir_lower::ownership::acquire_if_refcounted(self, value, span) } else if (uses_global || previous_kind == LocalKind::PhpLocal) - && !transfer_callable_source_to_store + && !transfer_source_to_store { // For ref-bound locals, acquire only when NOT narrowing Mixed→Int. // When the source is Mixed and the ref cell's previous type is Int, @@ -1087,7 +1094,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> { if uses_global { self.store_global_name(name, slot, value, span); self.set_local_type(name, php_type); - if release_source_after_store && !transfer_callable_source_to_store { + if release_source_after_store && !transfer_source_to_store { crate::ir_lower::ownership::release_if_owned(self, source, span); } return value; @@ -1116,7 +1123,10 @@ impl<'m, 'f> LoweringContext<'m, 'f> { if !is_ref_bound { self.set_local_type(name, php_type); } - if release_source_after_store && !transfer_callable_source_to_store && !ref_cell_narrowed_mixed_to_int { + if release_source_after_store + && !transfer_source_to_store + && !ref_cell_narrowed_mixed_to_int + { crate::ir_lower::ownership::release_if_owned(self, source, span); } value @@ -1625,6 +1635,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> { | Op::CallableArrayNew | Op::BufferNew | Op::GeneratorNew + | Op::CatchBind // `yield`/`yield from` return owned Mixed cells (the sent // value from `__rt_gen_suspend`, the delegated return from // `__rt_gen_delegate`); a discarded result must be released. @@ -2102,6 +2113,36 @@ impl<'m, 'f> LoweringContext<'m, 'f> { LoweredValue { value, ir_type } } + /// Emits a value-producing opcode whose result is an unconditional owned reference. + /// + /// This is reserved for runtime transfers such as `CatchBind`, where the opcode clears + /// the source runtime cell and hands its sole reference to the returned SSA value. + pub(crate) fn emit_owned_value( + &mut self, + op: Op, + operands: Vec, + immediate: Option, + php_type: PhpType, + effects: Effects, + span: Option, + ) -> LoweredValue { + let ir_type = value_ir_type(&php_type); + let value = self + .builder + .emit_with_effects( + op, + operands, + immediate, + ir_type, + php_type, + Ownership::Owned, + effects, + span, + ) + .expect("owned value opcode produces a value"); + LoweredValue { value, ir_type } + } + /// Emits an `is_truthy` conversion when a value is not already I64. pub(crate) fn truthy(&mut self, input: LoweredValue, span: Option) -> LoweredValue { if input.ir_type == IrType::I64 { diff --git a/src/ir_lower/stmt/mod.rs b/src/ir_lower/stmt/mod.rs index 4837f0214d..b9aac5017e 100644 --- a/src/ir_lower/stmt/mod.rs +++ b/src/ir_lower/stmt/mod.rs @@ -1995,36 +1995,27 @@ fn lower_current_exception(ctx: &mut LoweringContext<'_, '_>, span: Span) -> Low ) } -/// Binds and clears the active exception for a matched catch clause. The bound slot -/// takes over the in-flight reference (the runtime cell is zeroed by the bind), so it -/// participates in the owned-local lifecycle: the previous binding is released before -/// the rebind, and the frame epilogue releases whatever the slot still holds (issue -/// #448). A variable-less catch consumes the reference through a hidden owned -/// temporary so it flows through the same lifecycle instead of leaking. +/// Takes and clears the active exception for a matched catch clause, then stores the +/// owned result through the ordinary variable-storage planner. This keeps local, +/// global, static, and reference-cell destinations consistent while preserving the +/// single in-flight reference transferred by the runtime (issue #448). A variable-less +/// catch consumes the reference through a hidden owned temporary so it follows the +/// same lifecycle instead of leaking. fn lower_catch_bind(ctx: &mut LoweringContext<'_, '_>, catch: &CatchClause, span: Span) { let php_type = catch_variable_type(catch); let variable = match catch.variable.as_ref() { Some(variable) => variable.clone(), None => ctx.declare_owned_hidden_temp(php_type.clone()), }; - let slot = ctx.declare_local(&variable, php_type.clone()); - ctx.set_local_type(&variable, php_type.clone()); - // Release the slot's previous exception before rebinding: a catch that fires - // again (next loop iteration, later try block) would otherwise overwrite the - // only owned reference. Catch-bound slots are zero-initialized in the prologue, - // so the release is a runtime no-op the first time through. - ctx.release_stored_local_value(&variable, slot, Some(span)); - ctx.mark_local_initialized(&variable); - ctx.builder.emit_with_effects( + let caught = ctx.emit_owned_value( Op::CatchBind, Vec::new(), - Some(Immediate::LocalSlot(slot)), - IrType::Void, - php_type, - Ownership::NonHeap, + None, + php_type.clone(), Op::CatchBind.default_effects(), Some(span), ); + ctx.store_local(&variable, caught, php_type, Some(span)); } /// Returns the local type to use for a catch variable. diff --git a/tests/codegen/runtime_gc/regressions.rs b/tests/codegen/runtime_gc/regressions.rs index 71db0a76fc..888c0ee54f 100644 --- a/tests/codegen/runtime_gc/regressions.rs +++ b/tests/codegen/runtime_gc/regressions.rs @@ -1596,9 +1596,8 @@ echo "done"; /// Regression for #448: a caught exception object must be released once the catch /// handler is done with it. Before the fix the catch binding moved the throwable into -/// the variable's slot without ever scheduling a release (the slot is written by -/// `CatchBind`, not `StoreLocal`, so it escaped every cleanup point), leaking one -/// ~48-byte block per catch. A throw/catch loop must leave the heap clean. +/// the variable's slot without ever scheduling a release, leaking one ~48-byte block +/// per catch. A throw/catch loop must leave the heap clean. #[test] fn test_regression_caught_exception_released_per_catch() { let out = compile_and_run_with_heap_debug( @@ -1793,6 +1792,92 @@ echo $hits; ); } +/// Regression for #448: a catch variable declared through `global` must replace the +/// shared global value, release prior iterations, and remain readable after the function. +#[test] +fn test_regression_catch_binding_updates_global_storage() { + let out = compile_and_run_with_heap_debug( + r#" Date: Sun, 19 Jul 2026 19:05:37 +0200 Subject: [PATCH 4/4] docs(changelog): clarify caught exception ownership --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fc10effb0..6083765df6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to elephc, a PHP-to-native compiler written in Rust. Releases are listed newest first. ## [Unreleased] -- Fixed the caught-exception leak (issue #448): every handled `throw` leaked its throwable (~48 bytes per `catch`), because the catch binding moved the in-flight exception into the variable's slot without ever scheduling a release — the slot escaped rebind, function-epilogue, and program-exit cleanup alike. Catch-bound slots now follow the ordinary owned-local lifecycle (released on rebind and at the frame epilogue, zero-initialized so untaken catch paths stay safe), a variable-less `catch` consumes the reference through a hidden temporary, and rethrowing a caught variable (`throw $e`) retains it so inner and outer bindings each own their reference. Long-running throw/catch loops now keep a flat heap. +- Fixed the caught-exception leak (issue #448): every handled `throw` leaked its throwable (~48 bytes per `catch`), because the catch binding moved the in-flight exception into the variable's slot without ever scheduling a release — the slot escaped rebind, function-epilogue, and program-exit cleanup alike. Catch binding now transfers an owned EIR value through the ordinary storage planner, so locals, globals, static locals, and reference cells release and replace their previous values correctly; untaken local catch paths stay safe through zero-initialization, and a variable-less `catch` consumes the reference through a hidden temporary. Rethrowing a caught variable — statement-form `throw $e` or expression-form such as `true ? throw $e : 0` — retains it so inner and outer bindings each own their reference. The ARM64 uniform release dispatcher and linux-x86_64 object/any/deep-free helpers now accept throwable heap kind 6 (previously only kind 4), so those releases run on every supported target. Long-running throw/catch loops now keep a flat heap. - 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).