[clang] ebc2f79 - [CIR] Fix lost catch clauses on EH landing pads (#205638)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 25 10:51:17 PDT 2026
Author: adams381
Date: 2026-06-25T12:51:10-05:00
New Revision: ebc2f7946ad76daa1c4db18fc9196d94845e6946
URL: https://github.com/llvm/llvm-project/commit/ebc2f7946ad76daa1c4db18fc9196d94845e6946
DIFF: https://github.com/llvm/llvm-project/commit/ebc2f7946ad76daa1c4db18fc9196d94845e6946.diff
LOG: [CIR] Fix lost catch clauses on EH landing pads (#205638)
A throw caught by an enclosing handler could call std::terminate instead of
entering the handler. The smallest trigger is a try that destroys a local and
throws a new-expression whose type has a throwing constructor: two cleanups
then sit on the path to the handler and produce two cir.eh.initiate operations
that funnel into one cir.eh.dispatch. The same bug also asserted on ordinary
nested try/catch and dropped the outer handler's catch clause from the inner
landing pad on a nested throw.
ItaniumEHLowering::lowerEhInitiate derived each landing pad's catch clauses
while destructively walking the eh_token graph, tying a correctness property
to lowering order: whichever initiate lowered first tore down the shared cir.br
edges, so the other reached no dispatch and kept an empty catch_type_list -- a
cleanup-only landing pad that resumes past the handler.
lowerFunc now does a read-only walk first. For each initiate it collects every
dispatch the exception can reach (innermost first, preserving nested catch
order) and records the catch_type_list, plus the cleanup clause when a cleanup
lies on the unwind path. Each dispatch is lowered once, after every initiate has
been processed.
Follow-up to #199121, which added cleanup-scope throw handling for the
single-dispatch case.
Added:
clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp
Modified:
clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
Removed:
################################################################################
diff --git a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
index 0e39fa15d377b..71609356141a4 100644
--- a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
@@ -37,6 +37,7 @@
#include "clang/CIR/Dialect/Transforms/CIRTransformUtils.h"
#include "clang/CIR/MissingFeatures.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/TargetParser/Triple.h"
@@ -142,11 +143,11 @@ class ItaniumEHLowering : public EHABILowering {
resolveCatchCopyThunk(cir::ConstructCatchParamOp op);
mlir::LogicalResult lowerFunc(cir::FuncOp funcOp);
mlir::LogicalResult
- lowerEhInitiate(cir::EhInitiateOp initiateOp, EhTokenMap &ehTokenMap,
- SmallVectorImpl<mlir::Operation *> &deadOps);
+ lowerEhInitiate(cir::EhInitiateOp initiateOp,
+ llvm::ArrayRef<cir::EhDispatchOp> reachedDispatches,
+ bool reachesCleanup, EhTokenMap &ehTokenMap);
void lowerDispatch(cir::EhDispatchOp dispatch, mlir::Value exnPtr,
- mlir::Value typeId,
- SmallVectorImpl<mlir::Operation *> &deadOps);
+ mlir::Value typeId);
mlir::LogicalResult lowerConstructCatchParam(cir::ConstructCatchParamOp op,
mlir::Value exnPtr);
void lowerInitCatchParam(cir::InitCatchParamOp op);
@@ -301,6 +302,52 @@ mlir::Block *ItaniumEHLowering::buildTerminateBlock(cir::FuncOp funcOp,
return terminateBlock;
}
+/// Read-only walk of the eh_token graph from an initiate's root token,
+/// collecting every cir.eh.dispatch its exception can reach, innermost first.
+/// The token flows through cleanups to the innermost dispatch and then, via
+/// that dispatch's continue-unwind edge, on to each enclosing dispatch (nested
+/// try/catch). The walk follows the token into catch-handler blocks too, but
+/// it dead-ends there because cir.begin_catch consumes the eh_token (producing
+/// a catch_token), so only the unwind chain yields further dispatches.
+///
+/// This is computed before any destructive lowering so a landing pad's catch
+/// types -- which are a property of the EH graph -- do not depend on the order
+/// in which the destructive per-initiate traversal tears down shared
+/// token-graph edges. \p dispatches is left empty for a cleanup-only initiate
+/// that reaches no dispatch (e.g. a path that only resumes).
+static void collectReachableDispatches(
+ mlir::Value rootToken,
+ llvm::SmallSetVector<cir::EhDispatchOp, 4> &dispatches,
+ bool &reachesCleanup) {
+ llvm::SmallVector<mlir::Value> worklist;
+ llvm::SmallPtrSet<mlir::Value, 8> visited;
+ worklist.push_back(rootToken);
+ // Breadth-first (process in insertion order) so dispatches are discovered
+ // innermost first, matching the order catch clauses must appear in the
+ // landing pad.
+ for (unsigned i = 0; i < worklist.size(); ++i) {
+ mlir::Value current = worklist[i];
+ if (!visited.insert(current).second)
+ continue;
+ for (mlir::OpOperand &use : current.getUses()) {
+ mlir::Operation *user = use.getOwner();
+ // A cleanup anywhere on the unwind path (this initiate's own cleanup or
+ // an enclosing scope's) means the landing pad must carry the cleanup
+ // clause so destructors still run when a foreign exception unwinds
+ // through this frame.
+ if (mlir::isa<cir::BeginCleanupOp>(user))
+ reachesCleanup = true;
+ if (auto dispatch = mlir::dyn_cast<cir::EhDispatchOp>(user))
+ dispatches.insert(dispatch);
+ // Follow the token into eh_token block arguments of successor blocks.
+ for (unsigned s = 0, e = user->getNumSuccessors(); s < e; ++s)
+ for (mlir::BlockArgument arg : user->getSuccessor(s)->getArguments())
+ if (mlir::isa<cir::EhTokenType>(arg.getType()))
+ worklist.push_back(arg);
+ }
+ }
+}
+
/// Lower all EH operations in a single function.
mlir::LogicalResult ItaniumEHLowering::lowerFunc(cir::FuncOp funcOp) {
if (funcOp.isDeclaration())
@@ -325,21 +372,51 @@ mlir::LogicalResult ItaniumEHLowering::lowerFunc(cir::FuncOp funcOp) {
if (!funcOp.getPersonality())
funcOp.setPersonality(kGxxPersonality);
- // Lower each initiate and all EH ops connected to it. The token map is
- // shared across all initiate operations. Multiple initiates may flow into the
- // same dispatch block, and the map ensures the arguments are registered
- // only once. Dispatch ops are scheduled for deferred removal so that sibling
- // initiates can still read catch types from a shared dispatch.
+ // Compute, read-only and before any destructive lowering, the dispatches each
+ // initiate's exception can reach (innermost first; more than one for nested
+ // try/catch). A landing pad's catch types are a property of the EH graph, so
+ // deriving them here keeps them independent of the order in which the
+ // destructive per-initiate traversal in lowerEhInitiate tears down shared
+ // token-graph edges. Otherwise a sibling or outer dispatch could be missed,
+ // leaving a landing pad without its catch clause (it would resume past the
+ // handler to std::terminate) or an un-lowered leftover dispatch.
+ // Per initiate: the dispatches its exception can reach (innermost first) and
+ // whether a cleanup lies on its unwind path. Both are properties of the EH
+ // graph and are always consumed together for the same initiate, so they live
+ // in one map keyed by the initiate op.
+ struct InitiateEHInfo {
+ llvm::SmallSetVector<cir::EhDispatchOp, 4> reachedDispatches;
+ bool reachesCleanup = false;
+ };
+ llvm::DenseMap<mlir::Operation *, InitiateEHInfo> initiateInfo;
+ llvm::SmallSetVector<cir::EhDispatchOp, 4> dispatchesToLower;
+ for (cir::EhInitiateOp initiateOp : initiateOps) {
+ InitiateEHInfo &info = initiateInfo[initiateOp.getOperation()];
+ collectReachableDispatches(initiateOp.getEhToken(), info.reachedDispatches,
+ info.reachesCleanup);
+ dispatchesToLower.insert(info.reachedDispatches.begin(),
+ info.reachedDispatches.end());
+ }
+
EhTokenMap ehTokenMap;
- SmallVector<mlir::Operation *> deadOps;
- for (cir::EhInitiateOp initiateOp : initiateOps)
- if (mlir::failed(lowerEhInitiate(initiateOp, ehTokenMap, deadOps)))
+ for (cir::EhInitiateOp initiateOp : initiateOps) {
+ const InitiateEHInfo &info = initiateInfo[initiateOp.getOperation()];
+ if (mlir::failed(lowerEhInitiate(initiateOp,
+ info.reachedDispatches.getArrayRef(),
+ info.reachesCleanup, ehTokenMap)))
return mlir::failure();
+ }
- // Erase operations that were deferred during per-initiate processing
- // (dispatch ops whose catch types were read by multiple initiates).
- for (mlir::Operation *op : deadOps)
- op->erase();
+ // Lower each dispatch exactly once. Every initiate's token block-argument
+ // (ptr, u32) replacements are registered in ehTokenMap by now, so each
+ // dispatch's own (exnPtr, typeId) pair is available. lowerDispatch erases
+ // the dispatch after building its comparison chain.
+ for (cir::EhDispatchOp dispatch : dispatchesToLower) {
+ auto [exnPtr, typeId] = ehTokenMap.lookup(dispatch.getEhToken());
+ assert(exnPtr && typeId &&
+ "dispatch eh_token must be registered in ehTokenMap");
+ lowerDispatch(dispatch, exnPtr, typeId);
+ }
// Remove the !cir.eh_token block arguments that were replaced by (ptr, u32)
// pairs. Iterate in reverse to preserve argument indices during removal.
@@ -389,25 +466,53 @@ mlir::LogicalResult ItaniumEHLowering::lowerFunc(cir::FuncOp funcOp) {
/// a catch_type_list; when the dispatch is encountered during traversal,
/// the catch types are read and set on the inflight op.
///
-/// Dispatch ops are not erased during per-initiate processing because they may
-/// be used by other initiate ops that haven't yet been lowered. Instead they
-/// are added to \p deadOps and erased by the caller after all initiates have
-/// been lowered.
+/// Dispatch ops are not lowered here; they are lowered once by the caller after
+/// every initiate has been processed (a dispatch can be shared by sibling
+/// initiates), so this traversal only registers the catch-handler block
+/// arguments reachable through them.
///
/// \p ehTokenMap is shared across all initiates in the function so that block
/// arguments reachable from multiple sibling initiates are registered once.
mlir::LogicalResult ItaniumEHLowering::lowerEhInitiate(
- cir::EhInitiateOp initiateOp, EhTokenMap &ehTokenMap,
- SmallVectorImpl<mlir::Operation *> &deadOps) {
+ cir::EhInitiateOp initiateOp,
+ llvm::ArrayRef<cir::EhDispatchOp> reachedDispatches, bool reachesCleanup,
+ EhTokenMap &ehTokenMap) {
mlir::Value rootToken = initiateOp.getEhToken();
- // Create the inflight_exception without a catch_type_list. The catch types
- // will be set once we encounter the dispatch during the traversal below.
+ // The catch clauses for this landing pad come from the dispatches its
+ // exception reaches (computed read-only by the caller before any destructive
+ // lowering). For nested try/catch the exception can reach several dispatches
+ // innermost first, so the landing pad lists their catch types in that order
+ // (matching classic CodeGen), stopping at a catch-all since nothing escapes
+ // it. Deriving this from the original EH graph -- rather than during the
+ // destructive token-graph traversal below -- keeps it correct regardless of
+ // the order in which sibling/nested initiates are lowered.
+ mlir::ArrayAttr catchTypeList;
+ bool catchAll = false;
+ SmallVector<mlir::Attribute> typeSymbols;
+ for (cir::EhDispatchOp dispatch : reachedDispatches) {
+ if (mlir::ArrayAttr catchTypes = dispatch.getCatchTypesAttr())
+ for (mlir::Attribute attr : catchTypes)
+ typeSymbols.push_back(
+ mlir::cast<cir::GlobalViewAttr>(attr).getSymbol());
+ if (dispatch.getDefaultIsCatchAll()) {
+ catchAll = true;
+ // A catch-all handles every exception, so it stops the unwind: no
+ // enclosing dispatch is reachable past it, and the collector therefore
+ // never records one after it. Drop the rest of the clauses here.
+ assert(dispatch == reachedDispatches.back() &&
+ "catch-all must be the last reachable dispatch");
+ break;
+ }
+ }
+ if (!typeSymbols.empty())
+ catchTypeList = builder.getArrayAttr(typeSymbols);
+
builder.setInsertionPoint(initiateOp);
auto inflightOp = cir::EhInflightOp::create(
- builder, initiateOp.getLoc(), /*cleanup=*/initiateOp.getCleanup(),
- /*catch_all=*/false,
- /*catch_type_list=*/mlir::ArrayAttr{});
+ builder, initiateOp.getLoc(),
+ /*cleanup=*/initiateOp.getCleanup() || reachesCleanup,
+ /*catch_all=*/catchAll, catchTypeList);
ehTokenMap[rootToken] = {inflightOp.getExceptionPtr(),
inflightOp.getTypeId()};
@@ -495,25 +600,12 @@ mlir::LogicalResult ItaniumEHLowering::lowerEhInitiate(
auto [exnPtr, typeId] = ehTokenMap.lookup(op.getEhToken());
if (mlir::failed(lowerConstructCatchParam(op, exnPtr)))
return mlir::failure();
- } else if (auto op = mlir::dyn_cast<cir::EhDispatchOp>(user)) {
- // Read catch types from the dispatch and set them on the inflight op.
- mlir::ArrayAttr catchTypes = op.getCatchTypesAttr();
- if (catchTypes && catchTypes.size() > 0) {
- SmallVector<mlir::Attribute> typeSymbols;
- for (mlir::Attribute attr : catchTypes)
- typeSymbols.push_back(
- mlir::cast<cir::GlobalViewAttr>(attr).getSymbol());
- inflightOp.setCatchTypeListAttr(builder.getArrayAttr(typeSymbols));
- }
- if (op.getDefaultIsCatchAll())
- inflightOp.setCatchAllAttr(builder.getUnitAttr());
- // Only lower the dispatch once. A sibling initiate sharing the same
- // dispatch will still read its catch types (above), but the comparison
- // chain and branch replacement are only created the first time.
- if (!llvm::is_contained(deadOps, op.getOperation())) {
- auto [exnPtr, typeId] = ehTokenMap.lookup(op.getEhToken());
- lowerDispatch(op, exnPtr, typeId, deadOps);
- }
+ } else if (mlir::isa<cir::EhDispatchOp>(user)) {
+ // The dispatch's catch types were already read into every reaching
+ // landing pad (read-only, before this traversal). The dispatch op
+ // itself is lowered once by the caller after all initiates are
+ // processed, so nothing is done here; the successor catch-handler
+ // blocks are still reached via the block-argument registration above.
} else if (auto op = mlir::dyn_cast<cir::EhTerminateOp>(user)) {
auto [exnPtr, typeId] = ehTokenMap.lookup(op.getEhToken());
ensureClangCallTerminate(op.getLoc());
@@ -561,10 +653,9 @@ mlir::LogicalResult ItaniumEHLowering::lowerEhInitiate(
/// Lower a cir.eh.dispatch by creating a comparison chain in new blocks.
/// The dispatch itself is replaced with a branch to the first comparison
-/// block and added to deadOps for deferred removal.
-void ItaniumEHLowering::lowerDispatch(
- cir::EhDispatchOp dispatch, mlir::Value exnPtr, mlir::Value typeId,
- SmallVectorImpl<mlir::Operation *> &deadOps) {
+/// block and then erased.
+void ItaniumEHLowering::lowerDispatch(cir::EhDispatchOp dispatch,
+ mlir::Value exnPtr, mlir::Value typeId) {
mlir::Location dispLoc = dispatch.getLoc();
mlir::Block *defaultDest = dispatch.getDefaultDestination();
mlir::ArrayAttr catchTypes = dispatch.getCatchTypesAttr();
@@ -573,7 +664,7 @@ void ItaniumEHLowering::lowerDispatch(
// Build the comparison chain in new blocks inserted after the dispatch's
// block. The dispatch itself is replaced with a branch to the first
- // comparison block and scheduled for deferred removal.
+ // comparison block and erased below.
if (!catchTypes || catchTypes.empty()) {
// No typed catches: replace dispatch with a direct branch.
builder.setInsertionPoint(dispatch);
@@ -617,10 +708,9 @@ void ItaniumEHLowering::lowerDispatch(
mlir::ValueRange{exnPtr, typeId});
}
- // Schedule the dispatch for deferred removal. We cannot erase it now because
- // a sibling initiate that shares this dispatch may still need to read its
- // catch types.
- deadOps.push_back(dispatch);
+ // The caller lowers each dispatch exactly once after every initiate has been
+ // processed, so no sibling still needs it; erase it now.
+ dispatch.erase();
}
mlir::FailureOr<cir::FuncOp>
diff --git a/clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp b/clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp
new file mode 100644
index 0000000000000..ff313033e9a85
--- /dev/null
+++ b/clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp
@@ -0,0 +1,102 @@
+// 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 -cir-hoist-allocas -cir-flatten-cfg -cir-eh-abi-lowering %t.cir -o %t.eh.cir
+// RUN: FileCheck --input-file=%t.eh.cir %s -check-prefix=CIR-EHABI
+// 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=LLVM
+
+struct C {
+ C();
+ ~C();
+};
+
+// Two EH cleanups -- destroying the local `a` and freeing the thrown
+// `new C()` if its constructor throws -- funnel into the same catch dispatch,
+// so the throw's landing pad must carry both the cleanup and the catch clause.
+int testCaughtThrowSharedDispatch() {
+ try {
+ C a;
+ throw new C();
+ } catch (C *p) {
+ delete p;
+ return 0;
+ }
+ return 1;
+}
+
+// CIR: cir.func{{.*}} @_Z29testCaughtThrowSharedDispatchv()
+// CIR: %[[A:.*]] = cir.alloca "a" {{.*}} : !cir.ptr<!rec_C>
+// CIR: cir.try {
+// CIR: cir.call @_ZN1CC1Ev(%[[A]])
+// CIR: cir.cleanup.scope {
+// CIR: cir.throw %{{.*}} : !cir.ptr<!cir.ptr<!rec_C>>, @_ZTIP1C
+// CIR: cir.unreachable
+// CIR: } cleanup all {
+// CIR: cir.call @_ZN1CD1Ev(%[[A]]) nothrow
+// CIR: cir.yield
+// CIR: }
+// CIR: } catch [type #cir.global_view<@_ZTIP1C> : !cir.ptr<!u8i>]
+
+// After EHABI lowering the throw's in-flight exception carries BOTH the
+// `cleanup` clause (to run `a`'s destructor) and the `catch @_ZTIP1C` clause
+// (to reach the handler). The missing catch clause was the bug: the exception
+// would resume past the handler and terminate.
+
+// CIR-EHABI-LABEL: cir.func{{.*}} @_Z29testCaughtThrowSharedDispatchv()
+// CIR-EHABI: cir.eh.inflight_exception cleanup [@_ZTIP1C]
+
+// The throw lowers to an `invoke @__cxa_throw` whose unwind landing pad has
+// BOTH a `cleanup` clause and the `catch ptr @_ZTIP1C` clause; OGCG produces
+// the equivalent landing pad.
+
+// LLVM: define {{.*}}@_Z29testCaughtThrowSharedDispatchv()
+// LLVM: invoke void @__cxa_throw(ptr %{{.*}}, ptr @_ZTIP1C, ptr null)
+// LLVM-NEXT: to label %{{.*}} unwind label %[[LPAD:.*]]
+// LLVM: [[LPAD]]:
+// LLVM: landingpad { ptr, i32 }
+// LLVM-NEXT: cleanup
+// LLVM-NEXT: catch ptr @_ZTIP1C
+
+struct S {
+ S();
+ ~S();
+};
+void may_throw();
+
+// A throw in nested try/catch reaches more than one dispatch; its landing pad
+// lists every reachable catch clause (innermost first) plus the cleanup.
+void testNestedTry() {
+ try {
+ S a;
+ try {
+ may_throw();
+ } catch (int) {
+ }
+ } catch (double) {
+ }
+}
+
+// CIR: cir.func{{.*}} @_Z13testNestedTryv()
+// CIR: cir.try {
+// CIR: cir.try {
+// CIR: cir.call @_Z9may_throwv()
+// CIR: } catch [type #cir.global_view<@_ZTIi> : !cir.ptr<!u8i>]
+// CIR: } catch [type #cir.global_view<@_ZTId> : !cir.ptr<!u8i>]
+
+// The may_throw landing pad reaches both the inner (int) and outer (double)
+// dispatches plus the cleanup for `a`, so its in-flight exception lists both
+// catch clauses innermost first alongside the cleanup.
+
+// CIR-EHABI-LABEL: cir.func{{.*}} @_Z13testNestedTryv()
+// CIR-EHABI: cir.eh.inflight_exception cleanup [@_ZTIi, @_ZTId]
+
+// LLVM: define {{.*}}@_Z13testNestedTryv()
+// LLVM: invoke void @_Z9may_throwv()
+// LLVM-NEXT: to label %{{.*}} unwind label %[[LP:.*]]
+// LLVM: [[LP]]:
+// LLVM: landingpad { ptr, i32 }
+// LLVM-NEXT: cleanup
+// LLVM-NEXT: catch ptr @_ZTIi
+// LLVM-NEXT: catch ptr @_ZTId
More information about the cfe-commits
mailing list