Skip to content

An exception out of a try/finally, a nested block in a try, or a throw releases the scopes it leaves #596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ASDAlexander77 merged 1 commit into from
Oct 17, 2026
Merged

An exception out of a try/finally, a nested block in a try, or a throw releases the scopes it leaves #596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions tslang/include/TypeScript/Defines.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@
// MLIROwnedReference.h.
#define OWNED_RESULT_NAMED_ATTR_NAME "__owned_result_named"

// Marks a release MLIRGen put at the end of a hand-written `finally` for the exceptional leg only:
// what the scopes around the `try` owe once the exception carries on past it. The finally region
// is one region in MLIRGen but several copies after TryOpLowering - the copy the landing pad runs
// before resuming, and the copies normal completion, a `return` and a `break` run - and only the
// first leaves the function's scopes behind; on every other copy they are still live, and their
// own exits release them. TryOpLowering keeps these in the unwind copy and erases them from the
// rest.
#define UNWIND_ONLY_ATTR_NAME "__unwind_only"

// Marks a `ts.CreateBoundFunction` whose `this` is a capture box built for it a moment earlier,
// rather than a receiver that belongs to somebody else. Only such a closure owns its `this`, and
// only it gets the type tag at CLOSURE_TYPE_INDEX - a bound method must not take ownership of
Expand Down
37 changes: 37 additions & 0 deletions tslang/lib/TypeScript/LowerToAffineLoops.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1618,13 +1618,44 @@ struct TryOpLowering : public TsPattern<mlir_ts::TryOp>
tsContext->finallyBlockOf[tryOp.getOperation()] = finallyBlock;
finallyBlockLast = continuation->getPrevNode();

// That copy is the one the landing pad runs before the exception carries on, and the
// only one whose way out leaves the scopes around the try: the releases MLIRGen put at
// the end of the finally for them stay in it and go from every later copy. Not from a
// try nested in the finally - its own lowering sorts its own.
//
// Erased from each later copy through its mapping, and from the region itself only once
// the last copy is made: the rewriter defers an erasure, so an op erased from the
// region still goes into every copy cloned from it afterwards.
SmallVector<Operation *> unwindOnlyOps;
tryOp.getFinally().walk<mlir::WalkOrder::PreOrder>([&](Operation *op) {
if (isa<mlir_ts::TryOp>(op))
{
return WalkResult::skip();
}

if (op->hasAttr(UNWIND_ONLY_ATTR_NAME))
{
unwindOnlyOps.push_back(op);
}

return WalkResult::advance();
});

auto eraseUnwindOnlyOps = [&](const mlir::IRMapping &mapping) {
for (auto *op : unwindOnlyOps)
{
rewriter.eraseOp(mapping.lookup(op));
}
};

// add clone for 'return'
if (returns.size() > 0)
{
mlir::IRMapping returnFinallyMapping;
rewriter.cloneRegionBefore(tryOp.getFinally(), *continuation->getParent(), continuation->getIterator(),
returnFinallyMapping);
propagateTsContextEntries(returnFinallyMapping);
eraseUnwindOnlyOps(returnFinallyMapping);
auto returnFinallyBlockLast = continuation->getPrevNode();
rewriter.setInsertionPoint(returnFinallyBlockLast->getTerminator());
auto resultOpOfReturnFinallyBlock = cast<mlir_ts::ResultOp>(returnFinallyBlockLast->getTerminator());
Expand Down Expand Up @@ -1655,6 +1686,7 @@ struct TryOpLowering : public TsPattern<mlir_ts::TryOp>
rewriter.cloneRegionBefore(tryOp.getFinally(), *continuation->getParent(),
continuation->getIterator(), jumpFinallyMapping);
propagateTsContextEntries(jumpFinallyMapping);
eraseUnwindOnlyOps(jumpFinallyMapping);
auto jumpFinallyBlockLast = continuation->getPrevNode();
rewriter.setInsertionPoint(jumpFinallyBlockLast->getTerminator());
auto resultOpOfJumpFinallyBlock = cast<mlir_ts::ResultOp>(jumpFinallyBlockLast->getTerminator());
Expand All @@ -1679,6 +1711,11 @@ struct TryOpLowering : public TsPattern<mlir_ts::TryOp>
}
}

for (auto *op : unwindOnlyOps)
{
rewriter.eraseOp(op);
}

rewriter.inlineRegionBefore(tryOp.getFinally(), continuation);
exitBlockLast = continuation->getPrevNode();
}
Expand Down
68 changes: 59 additions & 9 deletions tslang/lib/TypeScript/MLIRGenImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,29 @@ class MLIRGenImpl
return false;
}

// Whether this block declares a `let` or `const` of its own - a local that may own a
// reference, and that the TryOp around it would not release if an exception left the block
// (see mlirGen(Block)). Over-approximates: the type that decides it is not known yet.
bool blockDeclaresLocals(ts::Block blockAST, int skipStatements = 0)
{
auto index = 0;
for (auto statement : blockAST->statements)
{
if (index++ < skipStatements || (SyntaxKind)statement != SyntaxKind::VariableStatement)
{
continue;
}

auto flags = statement.as<VariableStatement>()->declarationList->flags;
if ((flags & NodeFlags::Let) == NodeFlags::Let || (flags & NodeFlags::Const) == NodeFlags::Const)
{
return true;
}
}

return false;
}

// Whether every top-level `using` this block declares initializes directly from
// `new SomeClass(...)`.
//
Expand Down Expand Up @@ -836,7 +859,8 @@ class MLIRGenImpl
return mlir::success();
}

void releaseScopeOwnedVars(mlir::Location location, const SmallVector<mlir::Value> &ownedVars)
void releaseScopeOwnedVars(mlir::Location location, const SmallVector<mlir::Value> &ownedVars,
bool unwindOnly = false)
{
// reverse declaration order, the order a scope is unwound in: a later local may
// hold the only other reference to what an earlier one points at
Expand All @@ -846,22 +870,33 @@ class MLIRGenImpl
// released as a cell whatever its contents are owned by - the block has to go
// back regardless. A slot that is neither is here only because it might have
// turned into a cell and did not; see trackPossibleCell.
mlir::Operation *releaseOp = nullptr;
if (isCapturedVariableCell(storage))
{
builder.create<mlir_ts::ReleaseCellOp>(location, storage);
releaseOp = builder.create<mlir_ts::ReleaseCellOp>(location, storage);
}
else if (isOwnedLocalSlot(storage))
{
builder.create<mlir_ts::ReleaseSlotOp>(location, storage);
releaseOp = builder.create<mlir_ts::ReleaseSlotOp>(location, storage);
}

if (releaseOp && unwindOnly)
{
releaseOp->setAttr(UNWIND_ONLY_ATTR_NAME, builder.getUnitAttr());
}
}
}

// The rest of the unwind leg of a catch-less TryOp's cleanup region - the one wrapped around
// a block or a switch for its `using`. Once the region has given back its own scope, the
// exception keeps unwinding, and the scopes around it that have no cleanup region of their
// own get no other chance: a function's body holding `let s = a + b` and, further in,
// `{ using r = new R(); ... }` leaked `s` on every exception through the inner block.
// a block or a switch for its `using`, or around a block with locals inside a try body. Once
// the region has given back its own scope, the exception keeps unwinding, and the scopes
// around it that have no cleanup region of their own get no other chance: a function's body
// holding `let s = a + b` and, further in, `{ using r = new R(); ... }` leaked `s` on every
// exception through the inner block.
//
// The same walk serves the two other places an exception leaves a scope: a `throw` statement,
// which raises it there, and the copy of a hand-written `finally` that runs on the unwind leg
// and then resumes it (`unwindOnly`, see UNWIND_ONLY_ATTR_NAME).
//
// Walks outwards from the scope around the wrapped one, releasing as it goes, and stops at
// the first scope that is itself a TryOp's body (isUnwindCleanupScope) without touching its
Expand All @@ -876,17 +911,32 @@ class MLIRGenImpl
// locals declared ahead of the wrapped scope are in the lists yet, so every slot released
// here has had its store - or, hoisted in front of a switch whose case did not run, is
// null, which a release does nothing with (VariableOpLowering).
void mlirGenReleaseOwnedOnUnwind(mlir::Location location, const GenContext *genContext)
void mlirGenReleaseOwnedOnUnwind(mlir::Location location, const GenContext *genContext, bool unwindOnly = false)
{
for (; genContext != nullptr && !genContext->isUnwindCleanupScope; genContext = genContext->parentBlockContext)
{
if (genContext->ownedVars != nullptr)
{
releaseScopeOwnedVars(location, *genContext->ownedVars);
releaseScopeOwnedVars(location, *genContext->ownedVars, unwindOnly);
}
}
}

// Whether an exception out of a scope opened in this context lands on a TryOp's landing pad
// in this function: some scope between here and the function is a TryOp's body.
static bool isInsideUnwindCleanupScope(const GenContext &genContext)
{
for (auto *context = &genContext; context != nullptr; context = context->parentBlockContext)
{
if (context->isUnwindCleanupScope)
{
return true;
}
}

return false;
}

// Does this reference address a captured variable's cell - a heap block the frame shares
// with the closures that captured it - rather than ordinary frame storage?
//
Expand Down
38 changes: 36 additions & 2 deletions tslang/lib/TypeScript/MLIRGenStatements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ namespace mlirgen
// that is an exit the scope's owned locals have to be released before - see
// blockHasTypedCatch. Same wrapping, same cleanup region, different reason to want it.
//
// So are a block's own locals, once the block sits inside a TryOp's body: the TryOp's
// cleanup releases its body's scope and no other, so an exception out of a nested block
// - `try { { let s = a + b; h(); } } catch {}` - left `s` behind. The wrap's cleanup
// gives back the block's scope and the scopes between it and that body
// (mlirGenReleaseOwnedOnUnwind). Outside any TryOp nothing would run it.
//
// The two remaining conditions each guard against a real, still-open bug the wrapping
// would otherwise hit (see their comments). blockUsingInitializersAreAllNewExpr is
// about disposing a `using`, so it only has a say when there is one - but it keeps its
Expand All @@ -167,7 +173,8 @@ namespace mlirgen
auto declaresUsing = blockDeclaresUsing(blockAST, skipStatements);
auto usingIsDisposable = !declaresUsing || blockUsingInitializersAreAllNewExpr(blockAST, skipStatements);
auto rethrowNeedsCleanup = !compileOptions.isWindows && blockHasTypedCatch(blockAST, skipStatements);
if (usingIsDisposable && (declaresUsing || rethrowNeedsCleanup) && !blockIsInsideCatchOrFinally())
auto localsNeedCleanup = isInsideUnwindCleanupScope(genContext) && blockDeclaresLocals(blockAST, skipStatements);
if (usingIsDisposable && (declaresUsing || rethrowNeedsCleanup || localsNeedCleanup) && !blockIsInsideCatchOrFinally())
{
return mlirGenBlockWithUnwindCleanup(blockAST, genContext, skipStatements);
}
Expand Down Expand Up @@ -1395,6 +1402,16 @@ namespace mlirgen
EXIT_IF_FAILED_OR_NO_VALUE(result)
auto exception = V(result);

// The exception leaves every scope from here out to the nearest TryOp body, and those
// scopes have no landing pad of their own to release their locals from - the function's
// body, a `catch` clause's enclosing scopes. So the throw gives them back itself, as a
// `return` would, and first takes a reference for the exception, as a `return` does for
// its caller: `throw s` must not be freed by the release of `s`. Nothing gives that
// reference back - a caught value is not owned (takeOwnershipOfLocal) - so a thrown heap
// value leaks rather than dangles.
mlirGenRetainCaptured(location, mlir::ValueRange{exception}, /*atLastUse=*/true);
mlirGenReleaseOwnedOnUnwind(location, &genContext);

auto throwOp = builder.create<mlir_ts::ThrowOp>(location, exception);

if (!genContext.allowPartialResolve)
Expand Down Expand Up @@ -1527,7 +1544,16 @@ namespace mlirgen
}
}

auto result = mlirGen(tryStatementAST->catchClause->block, tryGenContext);
// With a finally, an exception out of the catch clause unwinds into the finally's
// unwind copy, which releases the scopes around the `try` (below) - so a `throw` in
// the clause must stop short of them, as it does at a TryOp's body
GenContext catchGenContext(tryGenContext);
if (tryStatementAST->finallyBlock)
{
catchGenContext.isUnwindCleanupScope = true;
}

auto result = mlirGen(tryStatementAST->catchClause->block, catchGenContext);
EXIT_IF_FAILED(result)
}

Expand All @@ -1540,6 +1566,14 @@ namespace mlirgen
{
auto result = mlirGen(tryStatementAST->finallyBlock, tryGenContext);
EXIT_IF_FAILED(result)

// The copy of the finally that runs on the unwind leg resumes the exception past the
// `try`, so it is the last chance the scopes around it get, up to the nearest TryOp
// body, as in a catch-less TryOp's cleanup. It cannot be the cleanup region itself:
// that runs before the finally, which may still read them. Marked unwind-only, so
// TryOpLowering drops them from the copies normal completion, `return` and `break`
// run, where those scopes are still live.
mlirGenReleaseOwnedOnUnwind(location, &tryGenContext, /*unwindOnly=*/true);
}

// terminator
Expand Down
3 changes: 3 additions & 0 deletions tslang/test/tester/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ tslang_add_test(NAME test-compile-00-labelled-scope-exit COMMAND test-runner "${
tslang_add_test(NAME test-compile-00-switch-case-scope-exit COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00switch_case_scope_exit.ts")
tslang_add_test(NAME test-compile-00-using-dispose-order COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_dispose_order.ts")
tslang_add_test(NAME test-compile-00-using-unwind-outer-locals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_unwind_outer_locals.ts")
tslang_add_test(NAME test-compile-00-try-unwind-outer-locals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_unwind_outer_locals.ts")
tslang_add_test(NAME test-compile-00-vars COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00vars.ts")
tslang_add_test(NAME test-compile-00-var-bindings COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00var_bindings.ts")
tslang_add_test(NAME test-compile-00-globals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00globals.ts")
Expand Down Expand Up @@ -880,6 +881,7 @@ tslang_add_test(NAME test-jit-00-labelled-scope-exit COMMAND test-runner -jit "$
tslang_add_test(NAME test-jit-00-switch-case-scope-exit COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00switch_case_scope_exit.ts")
tslang_add_test(NAME test-jit-00-using-dispose-order COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_dispose_order.ts")
tslang_add_test(NAME test-jit-00-using-unwind-outer-locals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_unwind_outer_locals.ts")
tslang_add_test(NAME test-jit-00-try-unwind-outer-locals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_unwind_outer_locals.ts")
tslang_add_test(NAME test-jit-00-vars COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00vars.ts")
tslang_add_test(NAME test-jit-00-var-bindings COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00var_bindings.ts")
tslang_add_test(NAME test-jit-00-globals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00globals.ts")
Expand Down Expand Up @@ -1834,6 +1836,7 @@ set(TSLANG_CORPUS
00switch_case_scope_exit.ts
00using_dispose_order.ts
00using_unwind_outer_locals.ts
00try_unwind_outer_locals.ts
00capture_in_new_arguments.ts
00cast_to_f32.ts
00catch_untyped_value.ts
Expand Down
Loading
Loading