Repository navigation
An exception through a block's using releases the owned locals of the scopes around it - #595
Merged
Merged
Conversation
…he scopes around it
`let s = "x" + "y"; { using a = new D(); return s; }` made --verify-ownership report `s`: a
real leak, not a verifier false positive. The block with the `using` is wrapped in a catch-less
TryOp whose cleanup region disposes and releases the block's own scope and then lets the
exception keep unwinding out of the function. The function body around it has no cleanup region
of its own, so under -mm=rc `s` was never given back on that path. The switch whose cases
declare a `using` has the same wrap and had the same leak.
The cleanup region of those two catch-less wraps (mlirGenBlockWithUnwindCleanup and the switch)
now also releases, release-only and without consuming the lists, the owned locals of every
enclosing scope up to the first one that is itself a TryOp's body (new
GenContext::isUnwindCleanupScope), or up to the function. It stops there because that TryOp's
landing pad takes the exception: its own cleanup releases its scope, and when it has a catch,
everything further out is still live afterwards.
New test 00using_unwind_outer_locals.ts covers a return and a throw out of a block, a plain scope
in between, a switch case, a loop body, and two shapes where an enclosing try catches, so its
outer locals must stay live. It runs under gc, rc and none (compile + JIT), and the
ownership-verifier sweep covers it. The compiler before this change reports 7 slots in it.
Still open (same family, different shapes, not changed here): a hand-written try/finally with
no catch whose outer locals are not released after the rethrow, a nested plain scope inside a
try that catches, a throw out of a catch clause, and a plain `throw` or throwing call with no
TryOp at all. The last one has no unwind edge in the IR, so the verifier cannot see it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ASDAlexander77
added a commit
that referenced
this pull request
Oct 17, 2026
…w releases the scopes it leaves (#596) Under -mm=rc, --verify-ownership still reported real leaks of an owned local on three unwind shapes that #595 left open, plus a plain `throw`: - `let s = a + b; try { h(); } finally { ... } return s;` - after the finally rethrows, `s` was never given back. The release cannot go in the cleanup region, which runs before the finally and so before it may read `s`. MLIRGen now puts the walk of #595 (mlirGenReleaseOwnedOnUnwind) at the end of the finally, with each release marked UNWIND_ONLY_ATTR_NAME. TryOpLowering keeps them in the copy of the finally the landing pad runs, and erases them from the copies normal completion, `return` and `break` run (through each copy's mapping: the rewriter defers an erasure, so an op erased from the region is still cloned into later copies). - `try { { let s = a + b; h(); } } catch {}` - the TryOp's cleanup releases its body's scope and no other. A block that declares a let/const inside a TryOp body (GenContext::isUnwindCleanupScope somewhere up the chain) is now wrapped like a `using` block, so its cleanup releases it and the scopes up to that body. - `try { h(); } catch { throw 2; }` and `let s = a + b; throw 1;` - a throw statement now releases the scopes it leaves up to the nearest TryOp body, as a `return` does, after taking a reference for the thrown value (mlirGenRetainCaptured at last use) so `throw s` is not freed with `s`. The thrown value is never given back, so it leaks rather than dangles. A catch clause of a try with a finally stops that walk, since the finally's unwind copy releases those scopes. New test 00try_unwind_outer_locals.ts (compile + JIT + TSLANG_CORPUS, so gc/rc/none) covers the finally on throw/complete/return/break, a throw from a catch with and without a finally, a try in a try body, nested scopes and a loop body in a try, and throws from a function body and of a local. The compiler before this change reports 9 slots in it; the verifier sweep covers it. Still open: a call that throws out of a catch with no finally, a typed catch that does not match with no finally, and a throwing call with no try around it - none has a landing pad to release from. Found on the way, all pre-existing: an empty `catch { }` does not catch; under rc a `return` through a finally releases the frame before the finally runs; `return` inside a finally, and a try/catch/finally nested in a finally, crash Win32ExceptionPass. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
ASDAlexander77
added a commit
that referenced
this pull request
Oct 17, 2026
…l path on Windows (#599) * A finally that returns, breaks or holds a try works on the exceptional path on Windows Two shapes crashed tslang in Win32ExceptionPass with 0xC0000005 under every memory model: function f(pre: string) { let s = pre + "1"; try { h(); } finally { return s; } } function g(pre: string) { let s = pre + "2"; try { h(); } finally { try { h(); } catch { print("in " + s); } finally { print("inner " + s); } } return s; } On Windows the copy of a finally that runs while an exception unwinds is a cleanup funclet, and a cleanup funclet can do neither thing these finallys need: - It cannot return to normal flow, only carry on unwinding. The `return` branched out of the cleanup region, so the pass found no `resume` to end it, left the region's `end` null and dereferenced it. - It cannot hold pads of its own. TryOpLowering tagged the inner try's pads as top-level, so the pass closed the cleanup region on reaching them, again with no end. Nesting them inside the cleanup would not have been enough: LLVM gives a pad in a cleanup funclet no EH state under the MSVC personality, so the inner throw would not be caught there and the process would terminate. TryOpLowering now enters the finally's exceptional-path copy through a catch-all pad when that finally holds a `return`, a break/continue out of the try, or a try. A `return` (or jump) in that copy ends the catch first, as one in a catch clause does, so it becomes a catchret and the exception is swallowed. A copy that runs to its end finishes as a cleanup does (end-catch, then `resume`), and Win32ExceptionPass turns a catch region that ends in `resume` into a rethrow from inside the funclet, to the handler the region unwinds to or to the caller. The finally's pad gets an id now, and a try in that copy of the finally (only that copy: the normal-exit, return and jump copies run outside the funclet) names it as the funclet it is nested in. Win32ExceptionPass also reports a region it found no end for with report_fatal_error instead of dereferencing null. New test 00try_finally_funclet.ts: return in finally with and without a throw, a try in finally whose exception carries on out, the same inside an enclosing try that catches, a return in a finally inside a try that catches, and a break in a finally. Compile + JIT, and TSLANG_CORPUS (gc, rc, none). The try-in-finally case passes the string as a parameter rather than holding an owned local: an owned local in a function an exception leaves through a try/finally is a known rc leak (still open from #595), which the ownership-verifier sweep would report. Not changed here: a finally (or catch) that leaves only on some paths - `finally { if (c) return s; }` - still breaks, because the pass's scan assumes a region has one exit. The same holds for `catch { if (c) return s; ... }` before this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * A finally that returns or breaks swallows the exception on the Itanium path too 00try_finally_funclet.ts failed on Linux with "terminate called after throwing an instance of 'int'" for `finally { return s; }` and `finally { break; }` when no try further out catches. There the finally's pad is a cleanup, and an exception with only cleanups to run is never unwound: the search phase finds no handler and the program terminates before the finally runs, so its return never gets the chance to swallow the exception. The finally of a try with no enclosing handler in the function is now entered through a catch-all landing pad when it can leave (the shape the enclosingHandler case already has): a `return` or jump in that copy ends the catch first, and a run to the end rethrows. The end-catch marking of those escaping ops now applies on both paths, as the pad is a catch-all on both. Verified on Linux (WSL): the new test under compile, JIT and the rc/none corpus, and the 261 try/catch/finally/using/throw/ownership tests; on Windows the same subset (267 tests). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
What
tslang --emit=mlir-affine --verify-ownership -mm=rcreportedshere:This is a real leak, not a verifier false positive. The block with the
usingis wrapped in a catch-lessTryOp. Its cleanup region disposes and releases only the block's own scope, then the exception keeps unwinding out of the function. The function body has no cleanup region of its own, so under-mm=rcnothing gavesback on that path. Aswitchwhose cases declare ausinggets the same wrap and had the same leak.Fix
The cleanup region of the two catch-less wraps (
mlirGenBlockWithUnwindCleanupand the switch wrap) now also releases the owned locals of each enclosing scope (mlirGenReleaseOwnedOnUnwind). It only releases, and it leaves the scopes' lists in place for their normal exits. It stops at the first scope that is itself aTryOpbody (newGenContext::isUnwindCleanupScope), or at the function. It has to stop there because thatTryOp's landing pad takes the exception: its own cleanup releases its scope, and if it has a catch, everything further out is still live afterwards.Tests
00using_unwind_outer_locals.tscovers:trycatches, so its outer locals must stay live (a churn step allocates over freed memory before they are read)athens, and in the caught-further-out shape the inner cleanup releases onlya.ctest -C Release -j 8 --timeout 300: 4130/4130 passed.Still open (same family, not changed here)
The verifier still reports these:
let s; try { h(); } finally {}: the rethrow afterfinallydoes not release outer locals.trythat catches.throwfrom acatchclause.throw, or a throwing call, with noTryOpat all. The verifier cannot see this case, because the IR has no unwind edge for it.🤖 Generated with Claude Code