Skip to content

fix(ir_lower): balance refcounts for a directly-passed owned mixed argument returned by the callee#618

Open
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/604-owned-mixed-call-arg
Open

fix(ir_lower): balance refcounts for a directly-passed owned mixed argument returned by the callee#618
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/604-owned-mixed-call-arg

Conversation

@mirchaemanuel

@mirchaemanuel mirchaemanuel commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Passing a freshly boxed owned mixed value directly as a call argument to a
function that returns that parameter, and consuming the result, corrupted refcounts:
heap debug aborted with bad refcount.

<?php
function idv($x) { return $x; }
$c = 0;
for ($i = 0; $i < 20; $i++) { $r = idv($i + 1); $c = $c + $r; }
echo $c, "\n"; // PHP: 210

Root cause

User functions return refcounted parameter storage without acquiring it for the
caller
, so the argument box ($i + 1) and the returned box are the same
allocation. The caller released it as an argument temporary after the call and then
re-acquired the same box to store the result — the single reference was dropped once
too often, and the acquire ran on an already-freed block (incref-on-freed → bad
refcount).

The existing argument-release suppression for this aliasing only fired on the
conservative may_alias path, and call_result_may_alias_arg excludes fresh
checked-arithmetic boxes there on purpose (an unproven/builtin callee must still
release them, #486). So the proven case (idv returns its parameter) fell through
and double-released.

Fix

Add a proven-return path to the suppression: when the callee is proven to return a
parameter, suppress the argument release even for a fresh boxed $i + 1, letting
ownership flow through the result (released once after the retaining store). The pure
type-compatibility check is split into arg_and_result_types_can_alias, so the proven
path admits those boxes without weakening the may_alias path (which keeps the
checked-arithmetic exclusion, so #486 stays fixed).

Known trade-off

ReturnArgAlias::Parameters is a MAY summary (a union over branches), so a callee
that returns the parameter only conditionally (if ($c) return $x; return 7;) is
still reported as possibly returning it, and the fix suppresses the argument release
on every call. When the runtime takes the aliasing branch the box flows through the
result and is freed exactly once (clean); when it takes the non-aliasing branch the
suppressed owned box leaks. This is the same deliberate leak-over-crash trade-off the
existing may_alias suppression already makes for array/hash arguments — a leak is
preferred over the previous refcount corruption/crash. #619 tracks
runtime alias disambiguation to recover those leaked boxes.

Tests

  • tests/codegen/runtime_gc/regressions.rs: loop repro (210), single-call (6),
    method dispatch (210), discarded result (20), conditional-return alias path
    (210), and controls for the via-local and callee-returns-a-constant paths (must
    stay clean and still release the argument).
  • src/ir_lower/tests/ownership.rs: IR-level assertion that the argument release is
    suppressed when the callee is proven to return it.

Fixes #604

https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L

… aliases an owned box

A freshly boxed owned `mixed` value passed directly as a call argument to a
function that returns that parameter, whose result is then consumed, corrupted
refcounts: heap debug aborted with `bad refcount` (issue illegalstudio#604).

The argument box and the returned box are the same allocation (`return $x` hands
the parameter straight back). The caller released the box as an argument
temporary after the call and then re-acquired the same box to store the result,
so the single reference was dropped once too often — the acquire ran on an
already-freed block.

The argument-release suppression that exists for this aliasing only fired on the
conservative `may_alias` path, and `call_result_may_alias_arg` deliberately
excludes fresh checked-arithmetic boxes there (an unproven callee must still
release them, issue illegalstudio#486). Add a proven-return path: when the callee is proven
to return this parameter, suppress the argument release even for a fresh boxed
`$i + 1`, letting ownership flow through the result (released once after the
retaining store). Split the pure type-compatibility check into
`arg_and_result_types_can_alias` so the proven path admits those boxes without
weakening the `may_alias` path.

Regression tests cover the loop repro, the single-call and method shapes, the
discarded-result shape, and controls for the via-local and callee-returns-a-
constant paths (which must stay clean and must still release the argument).

Claude-Session: https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L
@mirchaemanuel

Copy link
Copy Markdown
Contributor Author

Review note: ReturnArgAlias::Parameters is a MAY summary, so conditional-return callees also get the suppression — verified empirically that the aliasing branch is clean and the non-aliasing branch leaks the suppressed box, the same trade-off the long-standing may_alias suppression makes for container arguments (reverified with a fresh-array arg on the same shape). The residual leak and the runtime-disambiguation fix are tracked in #619.

@github-actions github-actions Bot added area:eir Touches EIR definitions, lowering, validation, or passes. size:s Small pull request. type:fix Corrects broken or incompatible behavior. labels Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:eir Touches EIR definitions, lowering, validation, or passes. size:s Small pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Direct owned boxed-mixed call argument with a consumed return value corrupts refcounts (heap debug: bad refcount)

1 participant