https://github.com/adams381 created
https://github.com/llvm/llvm-project/pull/205638
A throw caught by an enclosing handler can 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:
```c++
struct C { C(); ~C(); };
try { C a; throw new C(); } catch (C *p) { delete p; } // calls std::terminate
```
Two cleanups then sit on the path to the handler -- destroying `a`, and freeing
the `new C()` storage if its constructor throws -- and they produce two
`cir.eh.initiate` operations that funnel into one `cir.eh.dispatch`. The same
bug shows up two other ways: it asserts on ordinary nested try/catch, and on a
nested throw it drops the outer handler's catch clause from the inner landing
pad.
`ItaniumEHLowering::lowerEhInitiate` worked out each landing pad's catch
clauses while destructively walking the eh_token graph -- it rewrites and
erases the shared `cir.br` edges and lowers the `cir.eh.dispatch` as it goes.
That tied a correctness property (which clauses a landing pad carries) to
lowering order. Whichever initiate was lowered first tore down the shared
edges, so the other never reached the dispatch and kept an empty
`catch_type_list`: a cleanup-only landing pad that runs its cleanup and resumes
past the handler. The walk also stopped at the first dispatch it found, so for
nested try/catch the outer dispatch was either left un-lowered (its eh_token
block argument was later erased while still in use) or its catch clause never
made it onto the inner landing pad.
A landing pad's clauses are a property of the EH graph, so they shouldn't
depend on lowering order. `lowerFunc` now does a read-only walk first. For each
initiate it collects every dispatch the exception can reach (innermost first,
so nested catch clauses keep their 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.
>From bb9b192067a7206269afc2cb9fa048f79fa28c64 Mon Sep 17 00:00:00 2001
From: Adam Smith <[email protected]>
Date: Wed, 24 Jun 2026 11:37:12 -0700
Subject: [PATCH] [CIR] Fix lost catch clauses on EH landing pads
A throw caught by an enclosing handler could reach std::terminate instead of
the handler. It happens when two EH cleanups lie on the path from the throw to
its catch -- for example a try that destroys a local and throws a new-expression
whose constructor can throw, so the new-expression also needs an operator-delete
cleanup. Those two cleanups produce two cir.eh.initiate operations that funnel
into one cir.eh.dispatch. The same root cause also asserts the compiler on
ordinary nested try/catch and drops the outer catch clause from a nested throw's
landing pad.
lowerEhInitiate worked out each landing pad's catch clauses while destructively
walking the eh_token graph -- it rewrites and erases the shared cir.br edges and
lowers the cir.eh.dispatch as it goes. That tied the clauses to lowering order:
whichever initiate was lowered first tore down the shared edges, so a sibling
sharing the dispatch never reached it and kept an empty catch_type_list, a
cleanup-only landing pad that runs its cleanup and resumes past the handler.
The walk also stopped at the first dispatch, so for nested try/catch the outer
dispatch was left un-lowered (its eh_token block argument was later erased while
still in use) or its catch clause never reached the inner landing pad.
A landing pad's clauses are a property of the EH graph, not of the order it is
lowered in. Compute them from a read-only walk first: for each initiate collect
every dispatch its exception can reach (innermost first), set the
catch_type_list to that ordered list and the cleanup clause when a cleanup lies
on the unwind path, then lower each dispatch once after all initiates are
processed.
Follow-up to the cleanup-scope throw handling added in #199121.
---
.../CIR/Dialect/Transforms/EHABILowering.cpp | 155 ++++++++++++++----
.../CodeGen/cleanup-scope-throw-caught.cpp | 106 ++++++++++++
2 files changed, 227 insertions(+), 34 deletions(-)
create mode 100644 clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp
diff --git a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
index 0e39fa15d377b..7e0ee2bf195db 100644
--- a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp
@@ -142,7 +142,9 @@ class ItaniumEHLowering : public EHABILowering {
resolveCatchCopyThunk(cir::ConstructCatchParamOp op);
mlir::LogicalResult lowerFunc(cir::FuncOp funcOp);
mlir::LogicalResult
- lowerEhInitiate(cir::EhInitiateOp initiateOp, EhTokenMap &ehTokenMap,
+ lowerEhInitiate(cir::EhInitiateOp initiateOp,
+ llvm::ArrayRef<cir::EhDispatchOp> reachedDispatches,
+ bool reachesCleanup, EhTokenMap &ehTokenMap,
SmallVectorImpl<mlir::Operation *> &deadOps);
void lowerDispatch(cir::EhDispatchOp dispatch, mlir::Value exnPtr,
mlir::Value typeId,
@@ -301,6 +303,53 @@ 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::SmallVectorImpl<cir::EhDispatchOp>
&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))
+ if (!llvm::is_contained(dispatches, dispatch))
+ dispatches.push_back(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,19 +374,47 @@ 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.
+ llvm::DenseMap<mlir::Operation *, SmallVector<cir::EhDispatchOp>>
+ reachedDispatches;
+ llvm::DenseMap<mlir::Operation *, bool> reachesCleanup;
+ SmallVector<cir::EhDispatchOp> dispatchesToLower;
+ for (cir::EhInitiateOp initiateOp : initiateOps) {
+ SmallVector<cir::EhDispatchOp> reached;
+ bool cleanupOnPath = false;
+ collectReachableDispatches(initiateOp.getEhToken(), reached,
cleanupOnPath);
+ for (cir::EhDispatchOp dispatch : reached)
+ if (!llvm::is_contained(dispatchesToLower, dispatch))
+ dispatchesToLower.push_back(dispatch);
+ reachedDispatches[initiateOp.getOperation()] = std::move(reached);
+ reachesCleanup[initiateOp.getOperation()] = cleanupOnPath;
+ }
+
EhTokenMap ehTokenMap;
SmallVector<mlir::Operation *> deadOps;
for (cir::EhInitiateOp initiateOp : initiateOps)
- if (mlir::failed(lowerEhInitiate(initiateOp, ehTokenMap, deadOps)))
+ if (mlir::failed(lowerEhInitiate(
+ initiateOp, reachedDispatches[initiateOp.getOperation()],
+ reachesCleanup[initiateOp.getOperation()], ehTokenMap, deadOps)))
return mlir::failure();
+ // 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.
+ for (cir::EhDispatchOp dispatch : dispatchesToLower) {
+ auto [exnPtr, typeId] = ehTokenMap.lookup(dispatch.getEhToken());
+ lowerDispatch(dispatch, exnPtr, typeId, deadOps);
+ }
+
// Erase operations that were deferred during per-initiate processing
- // (dispatch ops whose catch types were read by multiple initiates).
+ // (the dispatch ops, lowered above).
for (mlir::Operation *op : deadOps)
op->erase();
@@ -397,17 +474,40 @@ mlir::LogicalResult
ItaniumEHLowering::lowerFunc(cir::FuncOp funcOp) {
/// \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, SmallVectorImpl<mlir::Operation *> &deadOps) {
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;
+ 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 +595,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());
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..3337834e02ddf
--- /dev/null
+++ b/clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp
@@ -0,0 +1,106 @@
+// 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: %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
+
+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>]
+
+// The throw lowers to an `invoke @__cxa_throw` whose unwind landing pad has
+// BOTH a `cleanup` clause (to run `a`'s destructor) and the `catch ptr
+// @_ZTIP1C` clause (to reach the handler). The missing catch clause was the
+// bug: the exception would resume past the handler and terminate.
+
+// LLVM: define {{.*}}@_Z29testCaughtThrowSharedDispatchv()
+// LLVM: invoke void @__cxa_throw(ptr %{{.*}}, ptr @_ZTIP1C, ptr null)
+// LLVM-NEXT: to label %{{.*}} unwind label %[[LPAD:[0-9]+]]
+// LLVM: [[LPAD]]:
+// LLVM: landingpad { ptr, i32 }
+// LLVM-NEXT: cleanup
+// LLVM-NEXT: catch ptr @_ZTIP1C
+
+// OGCG produces the equivalent landing pad: the throw's unwind path is a
+// `cleanup` + `catch ptr @_ZTIP1C` landing pad.
+
+// OGCG: define {{.*}}@_Z29testCaughtThrowSharedDispatchv()
+// OGCG: invoke void @__cxa_throw(ptr %{{.*}}, ptr @_ZTIP1C, ptr null)
+// OGCG-NEXT: to label %{{.*}} unwind label %[[LPAD:.*]]
+// OGCG: [[LPAD]]:
+// OGCG: landingpad { ptr, i32 }
+// OGCG-NEXT: cleanup
+// OGCG-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>]
+
+// LLVM: define {{.*}}@_Z13testNestedTryv()
+// LLVM: invoke void @_Z9may_throwv()
+// LLVM-NEXT: to label %{{.*}} unwind label %[[LP:[0-9]+]]
+// LLVM: [[LP]]:
+// LLVM: landingpad { ptr, i32 }
+// LLVM-NEXT: cleanup
+// LLVM-NEXT: catch ptr @_ZTIi
+// LLVM-NEXT: catch ptr @_ZTId
+
+// OGCG: define {{.*}}@_Z13testNestedTryv()
+// OGCG: invoke void @_Z9may_throwv()
+// OGCG-NEXT: to label %{{.*}} unwind label %[[LP:.*]]
+// OGCG: [[LP]]:
+// OGCG: landingpad { ptr, i32 }
+// OGCG-NEXT: cleanup
+// OGCG-NEXT: catch ptr @_ZTIi
+// OGCG-NEXT: catch ptr @_ZTId
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
