https://github.com/adams381 updated https://github.com/llvm/llvm-project/pull/205638
>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 1/2] [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 >From 959cc5aaf074d57e2962ff003eb72e00639ddf80 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Thu, 25 Jun 2026 07:57:21 -0700 Subject: [PATCH 2/2] [CIR][NFC] Simplify EHABI dispatch bookkeeping Fold the per-initiate `reachedDispatches` and `reachesCleanup` maps into a single `InitiateEHInfo` map keyed by the initiate op; nothing reads one without the other. Use `SmallSetVector` for the reachable-dispatch and dispatch-to-lower sets so the `is_contained` dedup checks go away. Drop the `deadOps` deferred-removal list: each dispatch is lowered once after every initiate is processed, so `lowerDispatch` erases it directly. Assert that a dispatch's eh_token is registered before lowering it, and that a catch-all is the last reachable dispatch. Also run cir-opt's EHABI pipeline in the test with `CIR-EHABI` checks on the in-flight exception's `cleanup` + catch clauses, and fold the duplicate `OGCG` checks into the shared `LLVM` prefix. --- .../CIR/Dialect/Transforms/EHABILowering.cpp | 99 ++++++++++--------- .../CodeGen/cleanup-scope-throw-caught.cpp | 48 +++++---- 2 files changed, 73 insertions(+), 74 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp index 7e0ee2bf195db..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" @@ -144,11 +145,9 @@ class ItaniumEHLowering : public EHABILowering { mlir::LogicalResult lowerEhInitiate(cir::EhInitiateOp initiateOp, llvm::ArrayRef<cir::EhDispatchOp> reachedDispatches, - bool reachesCleanup, EhTokenMap &ehTokenMap, - SmallVectorImpl<mlir::Operation *> &deadOps); + 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); @@ -316,10 +315,10 @@ mlir::Block *ItaniumEHLowering::buildTerminateBlock(cir::FuncOp funcOp, /// 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) { +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); @@ -339,8 +338,7 @@ collectReachableDispatches(mlir::Value rootToken, 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); + 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()) @@ -382,42 +380,44 @@ mlir::LogicalResult ItaniumEHLowering::lowerFunc(cir::FuncOp funcOp) { // 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; + // 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) { - 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; + 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, reachedDispatches[initiateOp.getOperation()], - reachesCleanup[initiateOp.getOperation()], 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(); + } // 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. + // 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()); - lowerDispatch(dispatch, exnPtr, typeId, deadOps); + assert(exnPtr && typeId && + "dispatch eh_token must be registered in ehTokenMap"); + lowerDispatch(dispatch, exnPtr, typeId); } - // Erase operations that were deferred during per-initiate processing - // (the dispatch ops, lowered above). - for (mlir::Operation *op : deadOps) - op->erase(); - // Remove the !cir.eh_token block arguments that were replaced by (ptr, u32) // pairs. Iterate in reverse to preserve argument indices during removal. for (mlir::Block &block : funcOp.getBody()) { @@ -466,17 +466,17 @@ 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, llvm::ArrayRef<cir::EhDispatchOp> reachedDispatches, bool reachesCleanup, - EhTokenMap &ehTokenMap, SmallVectorImpl<mlir::Operation *> &deadOps) { + EhTokenMap &ehTokenMap) { mlir::Value rootToken = initiateOp.getEhToken(); // The catch clauses for this landing pad come from the dispatches its @@ -497,6 +497,11 @@ mlir::LogicalResult ItaniumEHLowering::lowerEhInitiate( 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; } } @@ -648,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(); @@ -660,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); @@ -704,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 index 3337834e02ddf..ff313033e9a85 100644 --- a/clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp +++ b/clang/test/CIR/CodeGen/cleanup-scope-throw-caught.cpp @@ -1,9 +1,11 @@ // 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=OGCG +// RUN: FileCheck --input-file=%t.ll %s -check-prefix=LLVM struct C { C(); @@ -37,30 +39,26 @@ int testCaughtThrowSharedDispatch() { // 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 (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. +// 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:[0-9]+]] +// LLVM-NEXT: to label %{{.*}} unwind label %[[LPAD:.*]] // 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(); @@ -87,20 +85,18 @@ void testNestedTry() { // 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:[0-9]+]] +// 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 - -// 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
