https://github.com/andykaylor created https://github.com/llvm/llvm-project/pull/190233
We had a bug where exceptions caught with catch-all were not properly handling a thrown exception if the catch-all handler enclosed a cleanup handler. The structured CIR was generated correctly, but when we flattened the CFG and introduced cir.eh.initiate operations, the cir.eh.initiate for the cleanup's EH path was incorrectly marked as cleanup-only, even though it chained to the dispatch for the catch-all handler. This resulted in the landing pad generated for the cleanup not being marked as having a catch-all handler, so the exception was not caught. This change fixes the problem in the FlattenCFG pass. Assisted-by: Cursor / claude-4.6-opus-high >From 5d72731c5714ebd32809c98bdf466900bdf347cd Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Thu, 2 Apr 2026 11:09:13 -0700 Subject: [PATCH] [CIR] Fix handling of catch-all with cleanups We had a bug where exceptions caught with catch-all were not properly handling a thrown exception if the catch-all handler enclosed a cleanup handler. The structured CIR was generated correctly, but when we flattened the CFG and introduced cir.eh.initiate operations, the cir.eh.initiate for the cleanup's EH path was incorrectly marked as cleanup-only, even though it chained to the dispatch for the catch-all handler. This resulted in the landing pad generated for the cleanup not being marked as having a catch-all handler, so the exception was not caught. This change fixes the problem in the FlattenCFG pass. Assisted-by: Cursor / claude-4.6-opus-high --- .../lib/CIR/Dialect/Transforms/FlattenCFG.cpp | 54 ++++++- .../CodeGen/try-catch-all-with-cleanup.cpp | 138 ++++++++++++++++++ clang/test/CIR/CodeGen/try-catch.cpp | 2 +- clang/test/CIR/Transforms/flatten-try-op.cir | 2 +- 4 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 clang/test/CIR/CodeGen/try-catch-all-with-cleanup.cpp diff --git a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp index 935d95b01a4ac..0432b092b7467 100644 --- a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp +++ b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp @@ -704,14 +704,14 @@ static void replaceCallWithTryCall(cir::CallOp callOp, mlir::Block *unwindDest, // Create a shared unwind destination block. The block contains a // cir.eh.initiate operation (optionally with the cleanup attribute) and a // branch to the given destination block, passing the eh_token. -static mlir::Block *buildUnwindBlock(mlir::Block *dest, bool hasCleanup, +static mlir::Block *buildUnwindBlock(mlir::Block *dest, bool isCleanupOnly, mlir::Location loc, mlir::Block *insertBefore, mlir::PatternRewriter &rewriter) { mlir::Block *unwindBlock = rewriter.createBlock(insertBefore); rewriter.setInsertionPointToEnd(unwindBlock); auto ehInitiate = - cir::EhInitiateOp::create(rewriter, loc, /*cleanup=*/hasCleanup); + cir::EhInitiateOp::create(rewriter, loc, /*cleanup=*/isCleanupOnly); cir::BrOp::create(rewriter, loc, mlir::ValueRange{ehInitiate.getEhToken()}, dest); return unwindBlock; @@ -1220,8 +1220,8 @@ class CIRCleanupScopeOpFlattening // need a shared unwind destination. Resume ops from inner cleanups // branch directly to the EH cleanup entry. if (!callsToRewrite.empty()) - unwindBlock = buildUnwindBlock(ehCleanupEntry, /*hasCleanup=*/true, loc, - ehCleanupEntry, rewriter); + unwindBlock = buildUnwindBlock(ehCleanupEntry, /*isCleanupOnly=*/true, + loc, ehCleanupEntry, rewriter); } // All normal flow blocks are inserted before this point — either before @@ -1432,6 +1432,27 @@ class CIRCleanupScopeOpFlattening } }; +// Trace an !cir.eh_token value back through block arguments to find the +// cir.eh.initiate operation that defines it. Returns {} if the defining op +// cannot be found (e.g. multiple predecessors). +static cir::EhInitiateOp traceToEhInitiate(mlir::Value ehToken) { + while (ehToken) { + if (auto initiate = ehToken.getDefiningOp<cir::EhInitiateOp>()) + return initiate; + auto blockArg = mlir::dyn_cast<mlir::BlockArgument>(ehToken); + if (!blockArg) + return {}; + mlir::Block *pred = blockArg.getOwner()->getSinglePredecessor(); + if (!pred) + return {}; + auto brOp = mlir::dyn_cast<cir::BrOp>(pred->getTerminator()); + if (!brOp) + return {}; + ehToken = brOp.getDestOperands()[blockArg.getArgNumber()]; + } + return {}; +} + class CIRTryOpFlattening : public mlir::OpRewritePattern<cir::TryOp> { public: using OpRewritePattern<cir::TryOp>::OpRewritePattern; @@ -1654,16 +1675,29 @@ class CIRTryOpFlattening : public mlir::OpRewritePattern<cir::TryOp> { buildCatchDispatchBlock(tryOp, handlerTypes, catchHandlerBlocks, loc, catchHandlerBlocks.front(), rewriter); + // Check whether the try has a catch-all handler. When catch-all is + // present, the personality function will always stop unwinding at this + // frame (because catch-all matches every exception type). The LLVM + // landingpad therefore needs "catch ptr null" rather than "cleanup". + // The downstream pipeline (EHABILowering + LowerToLLVM) emits + // "catch ptr null" when the EhInitiateOp has neither cleanup nor typed + // catch types, so we clear the cleanup flag on every EhInitiateOp that + // feeds into a dispatch with a catch-all handler. + bool hasCatchAll = + handlerTypes && llvm::any_of(handlerTypes, [](mlir::Attribute attr) { + return mlir::isa<cir::CatchAllAttr>(attr); + }); + // Build a block to be the unwind desination for throwing calls and replace // the calls with try_call ops. Note that the unwind block created here is // something different than the unwind handler that we may have created // above. The unwind handler continues unwinding after uncaught exceptions. // This is the block that will eventually become the landing pad for invoke // instructions. - bool hasCleanup = tryOp.getCleanup(); + bool isCleanupOnly = tryOp.getCleanup() && !hasCatchAll; if (!callsToRewrite.empty()) { // Create a shared unwind block for all throwing calls. - mlir::Block *unwindBlock = buildUnwindBlock(dispatchBlock, hasCleanup, + mlir::Block *unwindBlock = buildUnwindBlock(dispatchBlock, isCleanupOnly, loc, dispatchBlock, rewriter); for (cir::CallOp callOp : callsToRewrite) @@ -1674,6 +1708,14 @@ class CIRTryOpFlattening : public mlir::OpRewritePattern<cir::TryOp> { // Resume ops from already-flattened cleanup scopes within the try body // should branch to the catch dispatch block instead of unwinding directly. for (cir::ResumeOp resumeOp : resumeOpsToChain) { + // When there is a catch-all handler, clear the cleanup flag on the + // cir.eh.initiate that produced this token. With catch-all, the LLVM + // landingpad needs "catch ptr null" instead of "cleanup". + if (hasCatchAll) { + if (auto ehInitiate = traceToEhInitiate(resumeOp.getEhToken())) + ehInitiate.removeCleanupAttr(); + } + mlir::Value ehToken = resumeOp.getEhToken(); rewriter.setInsertionPoint(resumeOp); rewriter.replaceOpWithNewOp<cir::BrOp>( diff --git a/clang/test/CIR/CodeGen/try-catch-all-with-cleanup.cpp b/clang/test/CIR/CodeGen/try-catch-all-with-cleanup.cpp new file mode 100644 index 0000000000000..6dcf84872f333 --- /dev/null +++ b/clang/test/CIR/CodeGen/try-catch-all-with-cleanup.cpp @@ -0,0 +1,138 @@ +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR +// RUN: cir-opt %t.cir -cir-flatten-cfg -o %t-flat.cir +// RUN: FileCheck --input-file=%t-flat.cir %s -check-prefix=CIR-FLAT +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -fclangir -emit-llvm %s -o %t-cir.ll +// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -emit-llvm %s -o %t.ll +// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG + +void mayThrow(); + +struct S { + S(); + ~S(); +}; + +void test_catch_all_with_cleanup() { + try { + S s; + mayThrow(); + } catch (...) { + } +} + +// CIR-LABEL: cir.func {{.*}} @_Z27test_catch_all_with_cleanupv() +// CIR: cir.scope { +// CIR: %[[S:.*]] = cir.alloca !rec_S, !cir.ptr<!rec_S>, ["s", init] +// CIR: cir.try { +// CIR: cir.call @_ZN1SC1Ev(%[[S]]) +// CIR: cir.cleanup.scope { +// CIR: cir.call @_Z8mayThrowv() +// CIR: cir.yield +// CIR: } cleanup all { +// CIR: cir.call @_ZN1SD1Ev(%[[S]]) nothrow +// CIR: cir.yield +// CIR: } +// CIR: cir.yield +// CIR: } catch all (%{{.*}}: !cir.eh_token {{.*}}) { +// CIR: %{{.*}}, %{{.*}} = cir.begin_catch +// CIR: cir.cleanup.scope { +// CIR: cir.yield +// CIR: } cleanup all { +// CIR: cir.end_catch +// CIR: cir.yield +// CIR: } +// CIR: cir.yield +// CIR: } +// CIR: } + +// CIR-FLAT-LABEL: cir.func {{.*}} @_Z27test_catch_all_with_cleanupv() +// +// CIR-FLAT: %[[S:.*]] = cir.alloca !rec_S +// +// Ctor may throw; unwinds directly to the dispatch (no cleanup needed yet). +// CIR-FLAT: cir.try_call @_ZN1SC1Ev(%[[S]]) ^[[AFTER_CTOR:bb[0-9]+]], ^[[CTOR_UNWIND:bb[0-9]+]] +// +// After the ctor, enter the cleanup scope where mayThrow may throw. +// CIR-FLAT: ^[[AFTER_CTOR]]: +// CIR-FLAT: cir.try_call @_Z8mayThrowv() ^[[NORMAL:bb[0-9]+]], ^[[INNER_UNWIND:bb[0-9]+]] +// +// Normal path: destroy s and exit the try. +// CIR-FLAT: ^[[NORMAL]]: +// CIR-FLAT: cir.call @_ZN1SD1Ev(%[[S]]) nothrow +// CIR-FLAT: cir.br ^{{.*}} +// +// Inner unwind: NO cleanup flag — catch-all catches everything. +// CIR-FLAT: ^[[INNER_UNWIND]]: +// CIR-FLAT: %[[INNER_ET:.*]] = cir.eh.initiate : !cir.eh_token +// CIR-FLAT: cir.br ^[[EH_CLEANUP:bb[0-9]+]](%[[INNER_ET]] : !cir.eh_token) +// +// EH cleanup: destroy s on the exception path, then go to dispatch. +// CIR-FLAT: ^[[EH_CLEANUP]](%[[EH_ET:.*]]: !cir.eh_token): +// CIR-FLAT: %[[CT:.*]] = cir.begin_cleanup %[[EH_ET]] +// CIR-FLAT: cir.call @_ZN1SD1Ev(%[[S]]) nothrow +// CIR-FLAT: cir.end_cleanup %[[CT]] +// CIR-FLAT: cir.br ^[[DISPATCH:bb[0-9]+]](%[[EH_ET]] : !cir.eh_token) +// +// Ctor unwind: NO cleanup flag — goes directly to dispatch. +// CIR-FLAT: ^[[CTOR_UNWIND]]: +// CIR-FLAT: %[[CTOR_ET:.*]] = cir.eh.initiate : !cir.eh_token +// CIR-FLAT: cir.br ^[[DISPATCH]](%[[CTOR_ET]] : !cir.eh_token) +// +// Dispatch: catch-all handler. +// CIR-FLAT: ^[[DISPATCH]](%[[DISP_ET:.*]]: !cir.eh_token): +// CIR-FLAT: cir.eh.dispatch %[[DISP_ET]] : !cir.eh_token [ +// CIR-FLAT: catch_all : ^[[CATCH_ALL:bb[0-9]+]] +// CIR-FLAT: ] +// +// Catch handler. +// CIR-FLAT: ^[[CATCH_ALL]](%[[CA_ET:.*]]: !cir.eh_token): +// CIR-FLAT: %{{.*}}, %{{.*}} = cir.begin_catch %[[CA_ET]] +// CIR-FLAT: cir.end_catch +// CIR-FLAT: cir.return + +// Both landing pads must use "catch ptr null" (not "cleanup") so that the +// personality function recognises a catch-all handler during the search phase. + +// LLVM-LABEL: define {{.*}} void @_Z27test_catch_all_with_cleanupv() +// LLVM-SAME: personality ptr @__gxx_personality_v0 +// LLVM: %[[S:.*]] = alloca %struct.S +// LLVM: invoke void @_ZN1SC1Ev({{.*}}%[[S]]) +// LLVM: to label %[[AFTER_CTOR:.*]] unwind label %[[CTOR_LP:.*]] +// LLVM: [[AFTER_CTOR]]: +// LLVM: invoke void @_Z8mayThrowv() +// LLVM: to label %[[NORMAL:.*]] unwind label %[[INNER_LP:.*]] +// LLVM: [[NORMAL]]: +// LLVM: call void @_ZN1SD1Ev({{.*}}%[[S]]) +// LLVM: [[INNER_LP]]: +// LLVM: landingpad { ptr, i32 } +// LLVM: catch ptr null +// LLVM: call void @_ZN1SD1Ev({{.*}}%[[S]]) +// LLVM: [[CTOR_LP]]: +// LLVM: landingpad { ptr, i32 } +// LLVM: catch ptr null +// LLVM: call ptr @__cxa_begin_catch +// LLVM: call void @__cxa_end_catch() +// LLVM: ret void + +// OGCG-LABEL: define {{.*}} void @_Z27test_catch_all_with_cleanupv() +// OGCG-SAME: personality ptr @__gxx_personality_v0 +// OGCG: %[[S:.*]] = alloca %struct.S +// OGCG: invoke void @_ZN1SC1Ev({{.*}}%[[S]]) +// OGCG: to label %[[AFTER_CTOR:.*]] unwind label %[[CTOR_LP:.*]] +// OGCG: [[AFTER_CTOR]]: +// OGCG: invoke void @_Z8mayThrowv() +// OGCG: to label %[[NORMAL:.*]] unwind label %[[INNER_LP:.*]] +// OGCG: [[NORMAL]]: +// OGCG: call void @_ZN1SD1Ev({{.*}}%[[S]]) +// OGCG: [[CTOR_LP]]: +// OGCG: landingpad { ptr, i32 } +// OGCG: catch ptr null +// OGCG: [[INNER_LP]]: +// OGCG: landingpad { ptr, i32 } +// OGCG: catch ptr null +// OGCG: call void @_ZN1SD1Ev({{.*}}%[[S]]) +// OGCG: call ptr @__cxa_begin_catch +// OGCG: call void @__cxa_end_catch() +// OGCG: ret void diff --git a/clang/test/CIR/CodeGen/try-catch.cpp b/clang/test/CIR/CodeGen/try-catch.cpp index 899644861b4f0..fd8b9294c9f92 100644 --- a/clang/test/CIR/CodeGen/try-catch.cpp +++ b/clang/test/CIR/CodeGen/try-catch.cpp @@ -941,7 +941,7 @@ void cleanup_inside_try_body() { // LLVM: br label %[[TRY_CONT:.*]] // LLVM: [[LANDING_PAD]]: // LLVM: %[[LP:.*]] = landingpad { ptr, i32 } -// LLVM: cleanup +// LLVM: catch ptr null // LLVM: %[[EXN_OBJ:.*]] = extractvalue { ptr, i32 } %[[LP]], 0 // LLVM: %[[EH_SELECTOR_VAL:.*]] = extractvalue { ptr, i32 } %[[LP]], 1 // LLVM: br label %[[CLEANUP_LANDING:.*]] diff --git a/clang/test/CIR/Transforms/flatten-try-op.cir b/clang/test/CIR/Transforms/flatten-try-op.cir index 7270bd9d5e0fc..d01e409b2731a 100644 --- a/clang/test/CIR/Transforms/flatten-try-op.cir +++ b/clang/test/CIR/Transforms/flatten-try-op.cir @@ -186,7 +186,7 @@ cir.func @test_try_catch_with_cleanup() { // CHECK: cir.br ^[[TRY_EXIT:bb[0-9]+]] // // CHECK: ^[[INNER_UNWIND]]: -// CHECK: %[[INNER_EH:.*]] = cir.eh.initiate cleanup : !cir.eh_token +// CHECK: %[[INNER_EH:.*]] = cir.eh.initiate : !cir.eh_token // CHECK: cir.br ^[[EH_CLEANUP:bb[0-9]+]](%[[INNER_EH]] : !cir.eh_token) // // CHECK: ^[[EH_CLEANUP]](%[[EH_CT:.*]]: !cir.eh_token): _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
