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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
