From 974a073ac4335ce0eae6698458f415c3475291c1 Mon Sep 17 00:00:00 2001 From: mirchaemanuel Date: Tue, 7 Jul 2026 11:30:09 +0200 Subject: [PATCH] fix(ir): release the producer's reference when boxing an owned value as Mixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #484. __rt_mixed_from_value retains refcounted payloads (objects, arrays, hashes, callables, nested cells) and persists strings, so the boxed cell always carries its own reference or copy — but EIR lowering treated Op::MixedBox as consuming its operand and never released the producer's reference. Boxing an owning temporary therefore left the payload one reference high forever: 'function g(): ?P { return new P(); }' leaked one P per call (and fresh arrays boxed on return leaked identically) even though the boxed cell itself was freed on rebind. A new LoweringContext::box_value_as_mixed helper emits the MixedBox and then releases the operand when it is an owning temporary (same ownership pattern as the throw-of-borrowed fix in #477); the 11 value-carrying emission sites now go through it. Borrowed operands (a boxed local) are untouched — the box's retain is their +1 — and concat-scratch strings remain exempt from heap release, so string boxing behavior is unchanged (its separate persisted-copy leak is issue (non-refcounted) and need no release. Regression tests in tests/codegen/runtime_gc/regressions.rs for the object and array flavours. Broad suites green (1872 codegen across callables/closures/ runtime_gc/control/types/exceptions/arrays/oop/generators + 993 error tests). --- CHANGELOG.md | 1 + src/ir_lower/context.rs | 27 +++++++++ src/ir_lower/expr/mod.rs | 74 ++++--------------------- src/ir_lower/function.rs | 3 + src/ir_lower/stmt/mod.rs | 27 +-------- tests/codegen/runtime_gc/regressions.rs | 52 +++++++++++++++++ 6 files changed, 96 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2299c866f..418e8f53a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Releases are listed newest first. - Fixed owned boxed Mixed temporaries leaking after string conversion (issue #527): explicit `(string)` casts plus implicit concatenation and interpolation now release detached Mixed sources, including by-value `foreach` element reads and non-null unions represented as Mixed; borrowed, persistent, moved, and non-heap values remain release no-ops. - Fixed boxed Mixed values leaking when nested loops reinitialize a local whose storage widens after lowering (issue #534): EIR now records a deferred `ReleaseLocalSlot` before the overwrite, prunes it for final scalar or ref-bound slots, and lowers it using the final slot representation. Cleanup is flow-sensitive across conditional ref-cell promotion, aliases, by-reference `foreach` and pointer paths, and widened parameters, preventing both missed releases and double frees with the EIR optimizer enabled or disabled. - Fixed function- and method-local indexed arrays leaking previous COW hash generations after promotion to string-keyed associative storage (issue #538): lowering now releases the boxed Mixed slot owner before consuming mutations and finalizes provisional concrete-load releases against the slot's final storage type. This preserves real COW aliases while avoiding an artificial clone on every insertion, leaving heap-debug clean with the EIR optimizer enabled or disabled across every supported target. +- Fixed a per-call heap leak when an owned refcounted value is boxed into a Mixed cell (issue #484): the runtime retains the payload for the box, but the lowering never released the producer's own reference, so `function g(): ?P { return new P(); }` leaked one object per call (and fresh arrays boxed the same way leaked identically) even though the boxed cell itself was freed. Boxing now releases the producer's reference for owning temporaries; borrowed values (e.g. a boxed local) are untouched. ## [0.26.1] - Expanded flow-sensitive type narrowing for PHP's common guard patterns: `int|false` and other false-sentinel unions now preserve the literal `false` subtype and narrow to their success type after a divergent `=== false` guard, without incorrectly removing a full `bool` member; `=== null` and `is_null()` guards narrow nullable values; and stable object properties can be narrowed through `instanceof`, ternaries, and throw guards. Property facts are invalidated after writes or receiver rebindings and are not retained across property hooks or `__get`, whose repeated reads may differ. diff --git a/src/ir_lower/context.rs b/src/ir_lower/context.rs index e48b61d16..59854152e 100644 --- a/src/ir_lower/context.rs +++ b/src/ir_lower/context.rs @@ -2026,6 +2026,33 @@ impl<'m, 'f> LoweringContext<'m, 'f> { ); } + /// Boxes a value into a Mixed cell and releases the producer's reference when the + /// operand is an owning temporary. `__rt_mixed_from_value` retains refcounted payloads + /// (objects, arrays, hashes, callables, nested cells) and persists strings, so the + /// boxed cell always carries its own reference or copy; keeping the producer's + /// reference too leaked one payload per boxing (issue #484). Borrowed operands (e.g. + /// a loaded local) are left untouched — the box's retain is their +1. + pub(crate) fn box_value_as_mixed( + &mut self, + value: LoweredValue, + php_type: PhpType, + span: Option, + ) -> LoweredValue { + let release_source = self.value_is_owning_temporary(value); + let boxed = self.emit_value( + Op::MixedBox, + vec![value.value], + None, + php_type, + Op::MixedBox.default_effects(), + span, + ); + if release_source { + crate::ir_lower::ownership::release_if_owned(self, value, span); + } + boxed + } + /// Emits a value-producing opcode with computed storage and ownership metadata. pub(crate) fn emit_value( &mut self, diff --git a/src/ir_lower/expr/mod.rs b/src/ir_lower/expr/mod.rs index f00a5b614..790b60ed9 100644 --- a/src/ir_lower/expr/mod.rs +++ b/src/ir_lower/expr/mod.rs @@ -270,14 +270,7 @@ fn lower_null(ctx: &mut LoweringContext<'_, '_>, expr: &Expr) -> LoweredValue { /// Lowers a nullsafe expression that is known to short-circuit to PHP null. fn lower_boxed_null(ctx: &mut LoweringContext<'_, '_>, expr: &Expr) -> LoweredValue { let null = lower_null(ctx, expr); - ctx.emit_value( - Op::MixedBox, - vec![null.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(expr.span), - ) + ctx.box_value_as_mixed(null, PhpType::Mixed, Some(expr.span)) } /// Lowers a binary operation. @@ -3826,14 +3819,7 @@ fn lower_named_descriptor_invoker_arg_container( } } } - ctx.emit_value( - Op::MixedBox, - vec![hash.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(span), - ) + ctx.box_value_as_mixed(hash, PhpType::Mixed, Some(span)) } /// Returns the variable name when this literal argument should be passed by reference. @@ -4729,14 +4715,7 @@ fn build_bound_closure_binding( return None; } let new_this_value = lower_expr(ctx, &new_this); - let boxed_this = ctx.emit_value( - Op::MixedBox, - vec![new_this_value.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(expr.span), - ); + let boxed_this = ctx.box_value_as_mixed(new_this_value, PhpType::Mixed, Some(expr.span)); signature.return_type = result_type; let bound = StaticCallableBinding::Closure { name, @@ -7035,14 +7014,7 @@ fn coerce_variadic_tail_value( if ctx.builder.value_php_type(value.value).codegen_repr() == PhpType::Mixed { return value; } - ctx.emit_value( - Op::MixedBox, - vec![value.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(span), - ) + ctx.box_value_as_mixed(value, PhpType::Mixed, Some(span)) } /// Returns true when a call argument uses unpacking syntax. @@ -9778,14 +9750,7 @@ fn lower_untyped_descriptor_invoker_hash_container( } } } - ctx.emit_value( - Op::MixedBox, - vec![hash.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(span), - ) + ctx.box_value_as_mixed(hash, PhpType::Mixed, Some(span)) } /// Copies an indexed spread source into a descriptor-invoker hash with numeric keys. @@ -9902,14 +9867,7 @@ fn coerce_descriptor_invoker_mixed_value( if ctx.builder.value_php_type(value.value).codegen_repr() == PhpType::Mixed { return value; } - ctx.emit_value( - Op::MixedBox, - vec![value.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(span), - ) + ctx.box_value_as_mixed(value, PhpType::Mixed, Some(span)) } /// Returns the result storage type for an indirect callable with no static signature. @@ -13469,14 +13427,7 @@ fn lower_nullsafe_method_call( ctx.builder.position_at_end(null_block); let null_value = lower_null(ctx, expr); let null_value = if result_type.codegen_repr() == PhpType::Mixed { - ctx.emit_value( - Op::MixedBox, - vec![null_value.value], - None, - result_type.clone(), - Op::MixedBox.default_effects(), - Some(expr.span), - ) + ctx.box_value_as_mixed(null_value, result_type.clone(), Some(expr.span)) } else { null_value }; @@ -14496,6 +14447,8 @@ fn lower_yield_from_array( Some(span), ) .expect("const_null produces a value"); + // A fresh null is non-refcounted: there is no producer reference to release, + // so this boxes directly rather than via box_value_as_mixed (issue #484). ctx.emit_value( Op::MixedBox, vec![null_value], @@ -14765,14 +14718,7 @@ fn coerce_value_for_temp( return value; } match target_ty { - PhpType::Mixed => ctx.emit_value( - Op::MixedBox, - vec![value.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(span), - ), + PhpType::Mixed => ctx.box_value_as_mixed(value, PhpType::Mixed, Some(span)), PhpType::Int | PhpType::Bool | PhpType::Void | PhpType::Never => { coerce_to_int_at_span(ctx, value, Some(span)) } diff --git a/src/ir_lower/function.rs b/src/ir_lower/function.rs index 9a1abd6f9..57734ee63 100644 --- a/src/ir_lower/function.rs +++ b/src/ir_lower/function.rs @@ -1038,6 +1038,9 @@ fn emit_default_return_value(ctx: &mut LoweringContext<'_, '_>) -> crate::ir::Va None, ) .expect("const_null produces a value"); + // A fresh null is non-refcounted: there is no producer reference to + // release, so this boxes directly rather than via box_value_as_mixed + // (issue #484). ctx.emit_value( Op::MixedBox, vec![null_value], diff --git a/src/ir_lower/stmt/mod.rs b/src/ir_lower/stmt/mod.rs index 729763bcb..e1d4b6d22 100644 --- a/src/ir_lower/stmt/mod.rs +++ b/src/ir_lower/stmt/mod.rs @@ -1170,14 +1170,7 @@ fn coerce_typed_assign_value( return value; } match target_ty { - PhpType::Mixed => ctx.emit_value( - Op::MixedBox, - vec![value.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(span), - ), + PhpType::Mixed => ctx.box_value_as_mixed(value, PhpType::Mixed, Some(span)), _ => value, } } @@ -1369,14 +1362,7 @@ fn initialize_foreach_mixed_local_if_needed( ctx.declare_local(name, PhpType::Mixed); ctx.set_local_type(name, PhpType::Mixed); let null = emit_null_value(ctx, Some(span)); - let boxed = ctx.emit_value( - Op::MixedBox, - vec![null.value], - None, - PhpType::Mixed, - Op::MixedBox.default_effects(), - Some(span), - ); + let boxed = ctx.box_value_as_mixed(null, PhpType::Mixed, Some(span)); ctx.store_foreach_initializer_local_only(name, boxed, PhpType::Mixed, Some(span)); } @@ -3189,14 +3175,7 @@ fn coerce_to_return_type( coerce_return_scalar_source(ctx, value, span, coerce_to_tagged_scalar) } IrType::Heap(_) if ctx.return_php_type.codegen_repr() == PhpType::Mixed => { - ctx.emit_value( - Op::MixedBox, - vec![value.value], - None, - ctx.return_php_type.clone(), - Op::MixedBox.default_effects(), - span, - ) + ctx.box_value_as_mixed(value, ctx.return_php_type.clone(), span) } IrType::Heap(_) => ctx.emit_value( Op::RuntimeCall, diff --git a/tests/codegen/runtime_gc/regressions.rs b/tests/codegen/runtime_gc/regressions.rs index fe67bc7be..419dec56b 100644 --- a/tests/codegen/runtime_gc/regressions.rs +++ b/tests/codegen/runtime_gc/regressions.rs @@ -2138,3 +2138,55 @@ echo $left . "|" . $right; out.stderr ); } + +/// Regression for #484: boxing an owned object into a Mixed cell (a `?Class` return +/// coerces through `mixed_box`) retains the payload in the runtime, so the lowering must +/// release the producer's own reference. Before the fix `function g(): ?P { return new +/// P(); }` leaked one `P` per call even though the boxed cell itself was freed on rebind. +#[test] +fn test_regression_mixed_boxed_object_return_releases_producer() { + let out = compile_and_run_with_heap_debug( + r#"v; +} +echo $acc; +"#, + ); + assert!(out.success, "program failed: {}", out.stderr); + assert_eq!(out.stdout, "350"); + assert!( + out.stderr.contains("HEAP DEBUG: leak summary: clean"), + "expected the boxed objects to be released, got: {}", + out.stderr + ); +} + +/// Regression for #484 (array flavour): boxing an owned array into a Mixed cell retains +/// it in the runtime; the producer's reference must be released or the array leaks once +/// per boxing (e.g. a `?array`-returning function building a fresh literal). +#[test] +fn test_regression_mixed_boxed_array_return_releases_producer() { + let out = compile_and_run_with_heap_debug( + r#"