Skip to content

fix(ir,codegen): release caught exceptions through the owned-local lifecycle (#448)#477

Open
mirchaemanuel wants to merge 2 commits into
illegalstudio:mainfrom
mirchaemanuel:fix/448-caught-exception-release
Open

fix(ir,codegen): release caught exceptions through the owned-local lifecycle (#448)#477
mirchaemanuel wants to merge 2 commits into
illegalstudio:mainfrom
mirchaemanuel:fix/448-caught-exception-release

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

Summary

Fixes #448. Every handled throw leaked its throwable (~48 bytes per catch), so long-running throw/catch loops grew the heap without bound.

Root cause

The exception's single reference (allocated at refcount 1, published to _exc_value by throw) is moved into the catch variable's slot by Op::CatchBind — a raw store the cleanup machinery never sees. The frame epilogue only releases slots written by Op::StoreLocal, so a catch-bound slot escaped rebind release, function-epilogue release, and program-exit release alike; a variable-less catch (\Throwable) dropped the in-flight reference outright. The system was "balanced" only because the catch side leaked: making the slot owned without touching throw would double-free on rethrow (throw $e leaves two slots sharing one reference).

Fix (three coordinated changes)

  • lower_catch_bind (ir_lower) — the bound slot follows the ordinary owned-local lifecycle: the previous binding is released before the rebind (the 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.
  • lower_throw (ir_lower) — throwing a borrowed value (a load of an owned local, e.g. rethrowing the caught variable) retains it, so inner and outer bindings each own a reference; throwing an owning temporary (throw new E(), throw f()) still transfers.
  • local_slot_has_store (codegen frame) — also recognizes Op::CatchBind, so catch-bound slots join the epilogue cleanup set and the prologue zero-init set (catch-not-taken paths stay safe); the direct-return exclusion continues to apply.

Verification

Adversarially reviewed across 23 shapes — no double free, no bad-refcount abort, no divergence: throw/catch loops (50 and 1000 iterations), unbound catch, rethrow chains up to 3 deep reusing the same variable name, aliasing ($x = $e, throw $x), sequential try/catch reusing $e, multi-clause and multi-type catches, nested try, finally on both the caught and the uncaught-propagating paths, throw of globals/properties/array elements (acquire lanes checked in the IR), the uncaught fatal path, and escape via array retention (byte-identical to a no-try control). Generator-internal catches improve: previously the caught exceptions leaked; now they are freed.

8 regression tests in tests/codegen/runtime_gc/regressions.rs. Full suites green: exceptions/generators/fibers/spl (472), runtime_gc/control/oop/types (1028), error_tests (993); build warning-free.

Known edges (pre-existing, verified identical on main, filed separately)

  • return $e from inside a catch does not compile (unsupported: runtime_call … returning Object) — catch-specific; returning an ordinary object local works.
  • Reassigning the caught variable to a new object ($e = new X()) mistypes the previous-value release as Mixed and leaks — this fix is neutral on that shape (identical block counts to main).
  • An exception message built from a runtime scratch string (new E(str_repeat(...))) is not persisted and can be corrupted by later concat/itoa activity — unrelated to the catch lifecycle.
  • A destructor that throws while the catch-release runs is a pre-existing hazard class (same pattern as ordinary owned-local rebind release), noted for completeness.

…fecycle

Fixes illegalstudio#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.
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 illegalstudio#482).
@mirchaemanuel

Copy link
Copy Markdown
Contributor Author

CI fix pushed (dc334486) — the linux-x86_64 failures were real, and interesting: not an emission defect in this PR (the generated code is symmetric across targets, verified by asm diff), but a latent gate in the pre-existing x86_64 runtime that this PR's releases were the first to hit.

Root cause: throwables are stamped heap kind 6, but the x86_64 __rt_decref_object and the __rt_object_free_deep validator only accepted kind 4 (plain object), and __rt_decref_any had no kind-6 arm — while the ARM64 variants have no kind gate at all. So on x86_64 every release of a caught exception was a silent no-op (the incref path checks only the heap magic, so refcounts could only go up). Latent for as long as catch bindings were never released — i.e., exactly issue #448 — and exposed the moment this PR made those releases real. That's why macOS/linux-aarch64 were green and only x86_64 failed.

Fix: the three x86_64 release paths now treat kind 6 like kind 4, mirroring ARM64's behavior. Verified on real linux-x86_64 via the Docker harness: the previously-failing regression tests (per-catch release, unbound catch, rethrow chain, multi-type catch) all pass; the macOS suite is unchanged.

While tracing this I also found that several x86_64 emitters stamp a transposed heap magic (0x4548504c vs the canonical 0x454C5048 the refcount helpers check), which no-ops refcounting for objects created through those paths — filed as #482 (includes two occurrences of mine in enums.rs, faithfully copied from the pre-existing ValueError emitter; a shared named constant would prevent the class).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Caught exception objects are never freed (throwable leaks once per catch)

1 participant