diff --git a/tslang/lib/TypeScript/LowerToAffineLoops.cpp b/tslang/lib/TypeScript/LowerToAffineLoops.cpp index 598f6e858..6430fe0e7 100644 --- a/tslang/lib/TypeScript/LowerToAffineLoops.cpp +++ b/tslang/lib/TypeScript/LowerToAffineLoops.cpp @@ -1295,17 +1295,35 @@ struct TryOpLowering : public TsPattern auto module = tryOp->getParentOfType(); auto parentTryOp = tsContext->parentTryOp.lookup(tryOp.getOperation()); - mlir::Block *parentTryOpLandingPad = parentTryOp ? tsContext->landingBlockOf.lookup(parentTryOp) : nullptr; - auto parentCatchesThis = parentTryOpLandingPad && tsContext->inParentTryBody.lookup(tryOp.getOperation()); // Where an exception this try does not catch has to go next within the function: the // parent's landing pad for a try in the parent's body, the parent's finally for one in // the parent's catch clause (whose exceptions the parent does not catch, but whose - // finally still has to run), and nowhere - the caller - otherwise. - mlir::Block *enclosingHandler = parentCatchesThis ? parentTryOpLandingPad : nullptr; - if (!enclosingHandler && parentTryOp && tsContext->inParentTryCatch.lookup(tryOp.getOperation())) - { - enclosingHandler = tsContext->finallyBlockOf.lookup(parentTryOp); + // finally still has to run), and nowhere - the caller - when no try around it takes it. + // + // Not just the parent: a try in the parent's catch clause (with no parent finally) or in + // the parent's finally hands its exception on to wherever the *parent's* would go, and + // so on up. Looking one level only, such a try sent it to the parent's own landing pad - + // the handler already running, which on Windows caught it again, forever - or, finding + // no finally, to the caller, past a try around the parent that was there to catch it. + // Every try around this one has been lowered by now, so their blocks are known. + mlir::Block *enclosingHandler = nullptr; + for (Operation *child = tryOp.getOperation(), *parent = parentTryOp; parent; + child = parent, parent = tsContext->parentTryOp.lookup(parent)) + { + if (tsContext->inParentTryBody.lookup(child)) + { + enclosingHandler = tsContext->landingBlockOf.lookup(parent); + } + else if (tsContext->inParentTryCatch.lookup(child)) + { + enclosingHandler = tsContext->finallyBlockOf.lookup(parent); + } + + if (enclosingHandler) + { + break; + } } MLIRRTTIHelperVC rttih(rewriter, module, tsContext->compileOptions); @@ -1675,7 +1693,7 @@ struct TryOpLowering : public TsPattern // linuxHasCleanups block at the end of this method. That rethrow needs a catch-all pad, // same as the nested-finally case beside it. auto linuxCleanupOnlyChainsToParent = - linuxHasCleanups && !catchHasOps && !finallyHasOps && parentTryOpLandingPad; + linuxHasCleanups && !catchHasOps && !finallyHasOps && enclosingHandler; // A typed catch whose exception may belong to someone else in this same function - the // try's own finally, or the try around it - has to be entered for every exception, not @@ -1689,7 +1707,7 @@ struct TryOpLowering : public TsPattern !linuxHasCleanups && (enclosingHandler || finallyHasOps); mlir::Value catchAll; - if (parentTryOpLandingPad && finallyHasOps || linuxHasCleanups && rttih.hasType() || + if (enclosingHandler && finallyHasOps || linuxHasCleanups && rttih.hasType() || linuxCleanupOnlyChainsToParent || linuxTypedCatchChains) { catchAll = (mlir::Value)rewriter.create(loc, mth.getNullType()); @@ -1731,7 +1749,7 @@ struct TryOpLowering : public TsPattern // own to hand the unwind to. Leaving unwindDests empty here is what tells // EndCleanupOp to resume unwinding instead of branching to a block that does not // exist - the Linux cleanup-only case below already relies on the same fallback - // chain, catchesBlock -> finallyBlock -> parentTryOpLandingPad -> resume. + // chain, catchesBlock -> finallyBlock -> enclosingHandler -> resume. if (catchesBlock) { unwindDests.push_back(catchesBlock); @@ -1740,9 +1758,9 @@ struct TryOpLowering : public TsPattern { unwindDests.push_back(finallyBlock); } - else if (parentTryOpLandingPad) + else if (enclosingHandler) { - unwindDests.push_back(parentTryOpLandingPad); + unwindDests.push_back(enclosingHandler); } auto resultOpCleanup = cast(cleanupBlockLast->getTerminator()); @@ -1781,8 +1799,8 @@ struct TryOpLowering : public TsPattern // The same goes for this try's own `finally`, which a mismatched exception has to // run on its way out: without the edge the catchswitch unwinds straight past it. // - // Only a try in its parent's body has an edge to add (parentCatchesThis): one in the - // parent's catch or finally is already inside the handler that pad leads to. + // The enclosing pad is enclosingHandler: never the parent's own pad for a try in the + // parent's catch or finally, which is already inside the handler that pad leads to. auto windowsNeedsParentEdge = tsContext->compileOptions.isWindows && (enclosingHandler || finallyHasOps); if (rttih.hasType() && (!tsContext->compileOptions.isWindows || windowsNeedsParentEdge)) { @@ -1858,9 +1876,9 @@ struct TryOpLowering : public TsPattern rewriter.setInsertionPoint(finallyBlockLast->getTerminator()); mlir::SmallVector unwindDests; - if (parentTryOpLandingPad) + if (enclosingHandler) { - unwindDests.push_back(parentTryOpLandingPad); + unwindDests.push_back(enclosingHandler); } auto resultOpFinally = cast(finallyBlockLast->getTerminator()); @@ -1868,7 +1886,7 @@ struct TryOpLowering : public TsPattern } else { - if (!parentTryOpLandingPad) + if (!enclosingHandler) { rewriter.setInsertionPointToStart(linuxHasCleanupsForFinally ? cleanupBlock : finallyBlock); @@ -1884,10 +1902,6 @@ struct TryOpLowering : public TsPattern rewriter.setInsertionPoint(finallyBlockLast->getTerminator()); mlir::SmallVector unwindDests; - if (parentTryOpLandingPad) - { - unwindDests.push_back(parentTryOpLandingPad); - } if (catchHasOps) { @@ -1920,7 +1934,7 @@ struct TryOpLowering : public TsPattern auto resultOpOfFinally = cast(finallyBlockLast->getTerminator()); auto throwOp = rewriter.replaceOpWithNewOp(resultOpOfFinally, nullVal); - tsContext->unwind[throwOp] = parentTryOpLandingPad; + tsContext->unwind[throwOp] = enclosingHandler; } // cleanup end } @@ -1948,30 +1962,26 @@ struct TryOpLowering : public TsPattern // instead of falling through to normal post-try control flow. auto *rethrowBlock = rewriter.createBlock(continuationBrCmp); rewriter.setInsertionPointToStart(rethrowBlock); - if (linuxTypedCatchChains) + if (!tsContext->compileOptions.isWindows) { - // __cxa_rethrow needs the exception to have been caught first + // __cxa_rethrow needs the exception to have been caught first. With a `using` in + // the try (linuxHasCleanups) the pad is a catch-all too, and its rethrow used to + // run with nothing caught: "terminate called without an active exception". rewriter.create(loc, mth.getOpaqueType(), cmpValue.getDefiningOp()->getOperand(0)); } auto rethrowVal = rewriter.create(loc, mth.getNullType()); auto rethrowOp = rewriter.create(loc, rethrowVal); - if (tsContext->compileOptions.isWindows || linuxTypedCatchChains) + // see windowsNeedsParentEdge / linuxTypedCatchChains: the finally's pad comes before + // any enclosing try's. It went to the parent's pad even for a try in the parent's + // catch clause, which is the handler already running. + if (finallyHasOps) { - // see windowsNeedsParentEdge / linuxTypedCatchChains: the finally's pad comes - // before any enclosing try's - if (finallyHasOps) - { - tsContext->unwind[rethrowOp] = finallyBlock; - } - else if (enclosingHandler) - { - tsContext->unwind[rethrowOp] = enclosingHandler; - } + tsContext->unwind[rethrowOp] = finallyBlock; } - else if (parentTryOpLandingPad) + else if (enclosingHandler) { - tsContext->unwind[rethrowOp] = parentTryOpLandingPad; + tsContext->unwind[rethrowOp] = enclosingHandler; } rewriter.setInsertionPointAfterValue(lastCmpValue); @@ -2044,7 +2054,7 @@ struct TryOpLowering : public TsPattern auto resultOpCleanup = cast(cleanupBlockLast->getTerminator()); auto throwOp = rewriter.replaceOpWithNewOp(resultOpCleanup, nullVal); - tsContext->unwind[throwOp] = parentTryOpLandingPad; + tsContext->unwind[throwOp] = enclosingHandler; } else { diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 2e1288c73..4f5367b8e 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -1454,7 +1454,9 @@ tslang_add_test(NAME test-jit-none-alloc-in-catch COMMAND test-runner -jit -mm=n # A try/catch inside a catch clause - crashed in every model at every optimisation level. tslang_add_test(NAME test-compile-00-nested-catch COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00nested_catch.ts") +tslang_add_test(NAME test-compile-00-try-mismatch-in-catch-clause COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_mismatch_in_catch_clause.ts") tslang_add_test(NAME test-jit-00-nested-catch COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00nested_catch.ts") +tslang_add_test(NAME test-jit-00-try-mismatch-in-catch-clause COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_mismatch_in_catch_clause.ts") tslang_add_test(NAME test-jit-rc-nested-catch COMMAND test-runner -jit -mm=rc "${PROJECT_SOURCE_DIR}/test/tester/tests/00nested_catch.ts") tslang_add_test(NAME test-jit-none-nested-catch COMMAND test-runner -jit -mm=none "${PROJECT_SOURCE_DIR}/test/tester/tests/00nested_catch.ts") @@ -1657,6 +1659,7 @@ set(TSLANG_CORPUS 00mixed_type_ops.ts 00names_conflict.ts 00nested_catch.ts + 00try_mismatch_in_catch_clause.ts 00new_delete.ts 00ns_bug.ts 00ns.ts @@ -2464,6 +2467,12 @@ if (TSLANG_TEST_X86) "${x86_jit_skipped_count} -jit registrations without a twin") endif() +# A mismatched typed catch in a try with a `using`, inside another try's catch clause. Linux +# only: on Windows a `using` inside a catch clause is broken already (see the test's header). +if (NOT WIN32) + tslang_add_test(NAME test-compile-00-try-mismatch-in-catch-clause-using COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_mismatch_in_catch_clause_using.ts") + tslang_add_test(NAME test-jit-00-try-mismatch-in-catch-clause-using COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_mismatch_in_catch_clause_using.ts") +endif() # An untyped catch of a C++ exception tslang did not throw (Itanium only: the box thunk field of # a __pointer_type_info is a Linux mechanism). foreign_throw.cpp throws a `const char*`, a # pointer to a C++ struct and an int; the catch must box each without calling into the type_info. diff --git a/tslang/test/tester/tests/00try_mismatch_in_catch_clause.ts b/tslang/test/tester/tests/00try_mismatch_in_catch_clause.ts new file mode 100644 index 000000000..e2f6ad32f --- /dev/null +++ b/tslang/test/tester/tests/00try_mismatch_in_catch_clause.ts @@ -0,0 +1,141 @@ +// A try inside another try's catch clause (or finally) whose typed catch does not match. Its +// exception belongs to whatever handles the *parent's* exceptions - a try around the parent - +// but the lowering sent it to the parent's own landing pad, the handler already running. On +// Windows that caught it again forever (a hang), or it left the function uncaught; on Linux it +// reached std::terminate. A try in a parent's finally with a `using` crashed the compiler on +// Windows. With a `using` in the inner try, Linux also rethrew with nothing caught - that shape +// is in 00try_mismatch_in_catch_clause_using.ts. + +let disposed = 0; +let finals = 0; +let wrong = 0; +let outer = 0; + +class Res { + [Symbol.dispose]() { + disposed++; + } +} + +function reset() { + disposed = 0; + finals = 0; + wrong = 0; + outer = 0; +} + +function inUntypedCatch() { + try { + try { + throw 1; + } catch (o) { + try { + throw "x"; + } catch (e: number) { + wrong++; + } + } + } catch (e2) { + outer++; + } +} + +function inTypedCatch() { + try { + try { + throw 1; + } catch (o: number) { + try { + throw "x"; + } catch (e: number) { + wrong++; + } + } + } catch (e2) { + outer++; + } +} + +function withFinally() { + try { + try { + throw 1; + } catch (o) { + try { + throw "x"; + } catch (e: number) { + wrong++; + } finally { + finals++; + } + } + } catch (e2) { + outer++; + } +} + +function inParentFinally() { + try { + try { + finals += 10; + } finally { + try { + using r = new Res(); + throw "x"; + } catch (e: number) { + wrong++; + } finally { + finals++; + } + } + } catch (e2) { + outer++; + } +} + +// two levels up: the parent is itself in a catch clause, whose try has a finally +function twoLevelsUp() { + try { + try { + throw 1; + } catch (a) { + try { + throw 2; + } catch (b) { + try { + throw "x"; + } catch (e: number) { + wrong++; + } + } + } finally { + finals++; + } + } catch (e2) { + outer++; + } +} + +function main() { + reset(); + inUntypedCatch(); + assert(wrong == 0 && outer == 1, "in an untyped catch clause"); + + reset(); + inTypedCatch(); + assert(wrong == 0 && outer == 1, "in a typed catch clause"); + + reset(); + withFinally(); + assert(wrong == 0 && finals == 1 && outer == 1, "with its own finally"); + + reset(); + inParentFinally(); + assert(disposed == 1 && finals == 11 && wrong == 0 && outer == 1, "in the parent's finally"); + + reset(); + twoLevelsUp(); + assert(wrong == 0 && finals == 1 && outer == 1, "two levels up, through a finally"); + + print("done."); +} diff --git a/tslang/test/tester/tests/00try_mismatch_in_catch_clause_using.ts b/tslang/test/tester/tests/00try_mismatch_in_catch_clause_using.ts new file mode 100644 index 000000000..a318cd5ef --- /dev/null +++ b/tslang/test/tester/tests/00try_mismatch_in_catch_clause_using.ts @@ -0,0 +1,72 @@ +// 00try_mismatch_in_catch_clause.ts, with a `using` in the inner try. Linux only: on Windows a +// `using` inside a catch clause is broken on main already - its cleanup pad is not nested in the +// catch funclet, which fails LLVM's verifier ahead of time, and with a typed catch and no try +// around it the JIT crashes in Win32ExceptionPass (see 00try_using_catch.ts, section 9.11). +// On Linux the mismatch rethrow after the cleanup ran with nothing caught: "terminate called +// without an active exception". + +let disposed = 0; +let finals = 0; +let wrong = 0; +let outer = 0; + +class Res { + [Symbol.dispose]() { + disposed++; + } +} + +function reset() { + disposed = 0; + finals = 0; + wrong = 0; + outer = 0; +} + +function withUsing() { + try { + try { + throw 1; + } catch (o) { + try { + using r = new Res(); + throw "x"; + } catch (e: number) { + wrong++; + } + } + } catch (e2) { + outer++; + } +} + +function withUsingAndFinally() { + try { + try { + throw 1; + } catch (o) { + try { + using r = new Res(); + throw "x"; + } catch (e: number) { + wrong++; + } finally { + finals++; + } + } + } catch (e2) { + outer++; + } +} + +function main() { + reset(); + withUsing(); + assert(disposed == 1 && wrong == 0 && outer == 1, "with a using"); + + reset(); + withUsingAndFinally(); + assert(disposed == 1 && finals == 1 && wrong == 0 && outer == 1, "with a using and a finally"); + + print("done."); +}