diff --git a/CHANGELOG.md b/CHANGELOG.md index de2a47351..7939c4877 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Releases are listed newest first. - 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 owned constructor argument temporaries leaking after fixed-class object creation (issues #540 and #516): EIR now releases temporary objects, arrays, strings, and boxed Mixed values once `ObjectNew` returns while preserving borrowed and by-reference arguments. Constructors that retain callable state, including `Fiber` and callback-filter iterators, now acquire their own descriptor reference before caller cleanup, preventing dangling captures while keeping ordinary descriptor cleanup balanced across every supported target. - Fixed `file_get_contents()` results leaking after retaining consumers (issue #540): the builtin now declares fresh caller-owned result storage in the single-source registry, allowing EIR to release both successful boxed string results and boxed `false` results after casts without affecting borrowed builtin results. Heap-debug remains clean with the EIR optimizer enabled or disabled. +- Fixed owned refcounted temporaries leaking when boxed as Mixed (issues #484, #540, and #516): EIR now releases the producer reference after `MixedBox` retains the payload, covering nullable/Mixed returns and other boxing sites for fresh objects, arrays, hashes, callables, and nested cells. Borrowed values remain valid and untouched. This closes root cause 3 from issue #540. ## [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 6ebd90946..e92b95a3e 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 f7c2db415..288595449 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. @@ -13462,14 +13420,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 }; @@ -14487,6 +14438,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], @@ -14756,14 +14709,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 01ecfd6fd..d47b79969 100644 --- a/tests/codegen/runtime_gc/regressions.rs +++ b/tests/codegen/runtime_gc/regressions.rs @@ -2297,3 +2297,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#"