Repository navigation
An exception out of a try/finally, a nested block in a try, or a throw releases the scopes it leaves - #596
Merged
Conversation
…w releases the scopes it leaves 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>
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.
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,swasnever 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 An exception through a block'susingreleases the owned locals of the scopes around it #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,
returnandbreakrun (through each copy's mapping: the rewriter defers anerasure, 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 andno other. A block that declares a let/const inside a TryOp body (GenContext::isUnwindCleanupScope
somewhere up the chain) is now wrapped like a
usingblock, so its cleanup releases it and thescopes up to that body.
try { h(); } catch { throw 2; }andlet s = a + b; throw 1;- a throw statement now releasesthe scopes it leaves up to the nearest TryOp body, as a
returndoes, after taking a referencefor the thrown value (mlirGenRetainCaptured at last use) so
throw sis not freed withs. Thethrown 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 areturnthrough a finally releases the frame before the finally runs;
returninside a finally, and atry/catch/finally nested in a finally, crash Win32ExceptionPass.
Full
ctest -C Release -j 8 --timeout 300: 100% passed, 0 failed out of 4138.🤖 Generated with Claude Code