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
Conversation
… 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
Contributor
Author
|
Review note: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Passing a freshly boxed owned
mixedvalue directly as a call argument to afunction that returns that parameter, and consuming the result, corrupted refcounts:
heap debug aborted with
bad refcount.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 sameallocation. 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_aliaspath, andcall_result_may_alias_argexcludes freshchecked-arithmetic boxes there on purpose (an unproven/builtin callee must still
release them, #486). So the proven case (
idvreturns its parameter) fell throughand 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, lettingownership 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 provenpath admits those boxes without weakening the
may_aliaspath (which keeps thechecked-arithmetic exclusion, so #486 stays fixed).
Known trade-off
ReturnArgAlias::Parametersis a MAY summary (a union over branches), so a calleethat returns the parameter only conditionally (
if ($c) return $x; return 7;) isstill 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_aliassuppression already makes for array/hash arguments — a leak ispreferred 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 (muststay clean and still release the argument).
src/ir_lower/tests/ownership.rs: IR-level assertion that the argument release issuppressed when the callee is proven to return it.
Fixes #604
https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L