https://github.com/andykaylor updated https://github.com/llvm/llvm-project/pull/219600
>From 67e7505098865854959c19337ff773ee20ad0816 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Fri, 28 Aug 2026 16:31:10 -0700 Subject: [PATCH 1/2] [CIR] Allow lifetime markers before begin_catch in a try handler When exceptions were enabled and lifetime markers were emitted, we were getting CIR verification errors because the cir.lifetime.start op appeared before cir.begin_catch and cir.construct_catch_param in a cir.try operation's catch handler region. These operations were correctly placed, the verifier just needed to be updated to properly check for them. Fixes https://github.com/llvm/llvm-project/issues/219549 Assisted-by: Cursor / claude-opus-5 --- clang/lib/CIR/CodeGen/CIRGenException.cpp | 2 +- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 29 +++++- clang/test/CIR/CodeGen/lifetime-marker.cpp | 65 +++++++++++++- clang/test/CIR/IR/catch-param.cir | 56 ++++++++++++ clang/test/CIR/IR/invalid-try-catch.cir | 100 +++++++++++++++++++++ 5 files changed, 247 insertions(+), 5 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenException.cpp b/clang/lib/CIR/CodeGen/CIRGenException.cpp index 2e3518c2d8bca..90a13087084b1 100644 --- a/clang/lib/CIR/CodeGen/CIRGenException.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenException.cpp @@ -387,7 +387,7 @@ static void initCatchParam(CIRGenFunction &cgf, CIRGenBuilderTy &builder, CanQualType catchType = cgf.cgm.getASTContext().getCanonicalType(catchParam.getType()); cir::InitCatchKind kind; - bool shouldInitFromExnDirectly; + bool shouldInitFromExnDirectly = false; // If we're catching by reference, we can just cast the object // pointer to the appropriate pointer. diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 21864cfa63691..67738cbafbcb2 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -4453,12 +4453,35 @@ LogicalResult cir::TryOp::verify() { if (mlir::isa<cir::UnwindAttr>(typeAttr)) continue; - // A catch handler region must start with cir.begin_catch, optionally - // preceded by a single cir.construct_catch_param that performs any - // pre-begin_catch initialization for the catch parameter. + // Nothing may run in a catch handler before cir.begin_catch, so it has to + // be the handler region's first operation, with two exceptions. + // + // When lifetime markers are enabled, the catch parameter's storage can be + // marked by a cir.lifetime.start. That parameter is the only variable + // whose lifetime begins before the catch is entered, so there is at most + // one such marker. Its lifetime-end cleanup has to run after the catch + // handler is finished (or exited by an exception unwind), so if there is a + // lifetime begin marker, it is followed by a cir.cleanup.scope that + // encloses the the rest of the handler with a cir.lifetime.end in its + // cleanup region. + // + // A cir.construct_catch_param may also precede cir.begin_catch, to + // perform any pre-begin_catch initialization of the catch parameter. if (entryBlock.empty()) return emitOpError("catch handler region must not be empty"); + mlir::Operation *firstOp = &entryBlock.front(); + if (mlir::isa<cir::LifetimeStartOp>(firstOp)) { + mlir::Operation *next = firstOp->getNextNode(); + auto lifetimeScope = mlir::dyn_cast_if_present<cir::CleanupScopeOp>(next); + if (!lifetimeScope) + return emitOpError("'cir.lifetime.start' in a catch handler region " + "must be followed by the 'cir.cleanup.scope' of " + "its lifetime-end cleanup"); + mlir::Block &scopeBody = lifetimeScope.getBodyRegion().front(); + firstOp = scopeBody.empty() ? nullptr : &scopeBody.front(); + } + if (mlir::isa_and_present<cir::ConstructCatchParamOp>(firstOp)) firstOp = firstOp->getNextNode(); if (!firstOp || !mlir::isa<cir::BeginCatchOp>(firstOp)) diff --git a/clang/test/CIR/CodeGen/lifetime-marker.cpp b/clang/test/CIR/CodeGen/lifetime-marker.cpp index ce661aafe4498..9265ab16b1208 100644 --- a/clang/test/CIR/CodeGen/lifetime-marker.cpp +++ b/clang/test/CIR/CodeGen/lifetime-marker.cpp @@ -7,7 +7,7 @@ // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -fcxx-exceptions -fexceptions -fclangir -emit-cir %s -o %t-eh.cir // RUN: FileCheck --input-file=%t-eh.cir %s --check-prefix=CIR-EH // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -fcxx-exceptions -fexceptions -fclangir -emit-llvm -disable-llvm-passes %s -o %t-eh.ll -// RUN: FileCheck --input-file=%t-eh.ll %s --check-prefix=LLVM-EH +// RUN: FileCheck --input-file=%t-eh.ll %s --check-prefixes=LLVM-EH void use(int); @@ -279,3 +279,66 @@ void while_record_condvar() { // LLVM-EH-NEXT: cleanup // LLVM-EH: call void @_ZN8LoopCondD1Ev(ptr {{.*}} %[[C]]) // LLVM-EH: call void @llvm.lifetime.end.p0(ptr %[[C]]) + +#ifdef __EXCEPTIONS + +struct Ex {}; + +void catch_by_ref() { + try { + may_throw(); + } catch (const Ex &e) { + } +} + +// CIR-EH-LABEL: cir.func{{.*}} @_Z12catch_by_refv +// CIR-EH: %[[E:.*]] = cir.alloca "e" {{.*}} : !cir.ptr<!cir.ptr<!rec_Ex>> +// CIR-EH: } catch [type #cir.global_view<@_ZTI2Ex>{{.*}}] (%[[TOK:[^:]*]]: +// CIR-EH-NEXT: cir.lifetime.start %[[E]] : !cir.ptr<!cir.ptr<!rec_Ex>> +// CIR-EH-NEXT: cir.cleanup.scope { +// CIR-EH-NEXT: %[[CATCH_TOK:.*]], %{{.*}} = cir.begin_catch %[[TOK]] +// CIR-EH: } cleanup all { +// CIR-EH: cir.end_catch %[[CATCH_TOK]] +// CIR-EH: } +// CIR-EH: } cleanup all { +// CIR-EH-NEXT: cir.lifetime.end %[[E]] : !cir.ptr<!cir.ptr<!rec_Ex>> + +// LLVM-EH-LABEL: define{{.*}} void @_Z12catch_by_refv() +// LLVM-EH: call void @llvm.lifetime.start.p0(ptr %[[E:.*]]) +// LLVM-EH: call ptr @__cxa_begin_catch +// LLVM-EH: call void @__cxa_end_catch() +// LLVM-EH: call void @llvm.lifetime.end.p0(ptr %[[E]]) + +struct Copy { + Copy(const Copy &); + ~Copy(); +}; + +void catch_by_value() { + try { + may_throw(); + } catch (Copy c) { + } +} + +// CIR-EH-LABEL: cir.func{{.*}} @_Z14catch_by_valuev +// CIR-EH: %[[C:.*]] = cir.alloca "c" {{.*}} : !cir.ptr<!rec_Copy> +// CIR-EH: } catch [type #cir.global_view<@_ZTI4Copy>{{.*}}] (%[[TOK:[^:]*]]: +// CIR-EH-NEXT: cir.lifetime.start %[[C]] : !cir.ptr<!rec_Copy> +// CIR-EH-NEXT: cir.cleanup.scope { +// CIR-EH-NEXT: cir.construct_catch_param non_trivial_copy %[[TOK]] to %[[C]] +// CIR-EH-NEXT: %[[CATCH_TOK:.*]], %{{.*}} = cir.begin_catch %[[TOK]] +// CIR-EH: cir.call @_ZN4CopyD1Ev(%[[C]]) +// CIR-EH: cir.end_catch %[[CATCH_TOK]] +// CIR-EH: } cleanup all { +// CIR-EH-NEXT: cir.lifetime.end %[[C]] : !cir.ptr<!rec_Copy> + +// LLVM-EH-LABEL: define{{.*}} void @_Z14catch_by_valuev() +// LLVM-EH: call void @llvm.lifetime.start.p0(ptr %[[C:.*]]) +// LLVM-EH: call ptr @__cxa_get_exception_ptr +// LLVM-EH: call ptr @__cxa_begin_catch +// LLVM-EH: call void @_ZN4CopyD1Ev(ptr {{.*}} %[[C]]) +// LLVM-EH: call void @__cxa_end_catch() +// LLVM-EH: call void @llvm.lifetime.end.p0(ptr %[[C]]) + +#endif // __EXCEPTIONS diff --git a/clang/test/CIR/IR/catch-param.cir b/clang/test/CIR/IR/catch-param.cir index 3734917c35877..8e5089cf34130 100644 --- a/clang/test/CIR/IR/catch-param.cir +++ b/clang/test/CIR/IR/catch-param.cir @@ -41,4 +41,60 @@ cir.func @begin_catch_inside_catch() { // CHECK: cir.return // CHECK: } +// The catch parameter's lifetime-end cleanup has to run after __cxa_end_catch, +// so it is pushed before cir.begin_catch is emitted and its cir.cleanup.scope +// ends up wrapping the begin_catch. The lifetime.start marker precedes the +// scope. +cir.func @begin_catch_inside_lifetime_cleanup_scope() { + cir.scope { + %0 = cir.alloca "i" align(4) : !cir.ptr<!s32i> + cir.try { + cir.yield + } catch all (%eh_token : !cir.eh_token) { + cir.lifetime.start %0 : !cir.ptr<!s32i> + cir.cleanup.scope { + %catch_token, %exception = cir.begin_catch %eh_token : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) + cir.cleanup.scope { + cir.yield + } cleanup all { + cir.end_catch %catch_token : !cir.catch_token + cir.yield + } + cir.yield + } cleanup all { + cir.lifetime.end %0 : !cir.ptr<!s32i> + cir.yield + } + cir.yield + } + } + cir.return +} + +// CHECK: cir.func @begin_catch_inside_lifetime_cleanup_scope() { +// CHECK: cir.scope { +// CHECK: %[[I:.*]] = cir.alloca "i" align(4) : !cir.ptr<!s32i> +// CHECK: cir.try { +// CHECK: cir.yield +// CHECK: } catch all (%[[EH_TOKEN:.*]]: !cir.eh_token) { +// CHECK: cir.lifetime.start %[[I]] : !cir.ptr<!s32i> +// CHECK: cir.cleanup.scope { +// CHECK: %[[CATCH_TOKEN:.*]], %[[EXCEPTION:.*]] = cir.begin_catch %[[EH_TOKEN]] : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) +// CHECK: cir.cleanup.scope { +// CHECK: cir.yield +// CHECK: } cleanup all { +// CHECK: cir.end_catch %[[CATCH_TOKEN]] : !cir.catch_token +// CHECK: cir.yield +// CHECK: } +// CHECK: cir.yield +// CHECK: } cleanup all { +// CHECK: cir.lifetime.end %[[I]] : !cir.ptr<!s32i> +// CHECK: cir.yield +// CHECK: } +// CHECK: cir.yield +// CHECK: } +// CHECK: } +// CHECK: cir.return +// CHECK: } + } diff --git a/clang/test/CIR/IR/invalid-try-catch.cir b/clang/test/CIR/IR/invalid-try-catch.cir index a40c8efc9acc5..367733a6d7e6f 100644 --- a/clang/test/CIR/IR/invalid-try-catch.cir +++ b/clang/test/CIR/IR/invalid-try-catch.cir @@ -181,6 +181,106 @@ cir.func dso_local @catch_all_handler_missing_begin_catch() { // ----- +!s32i = !cir.int<s, 32> +!void = !cir.void + +module { + +cir.func private @some_call() + +// The lifetime-end cleanup scope may hold cir.begin_catch, but it doesn't let +// arbitrary operations be hoisted above it. +cir.func dso_local @catch_handler_call_before_begin_catch() { + cir.scope { + %0 = cir.alloca "i" align(4) : !cir.ptr<!s32i> + // expected-error @below {{catch handler region must start with 'cir.begin_catch'}} + cir.try { + cir.yield + } catch all (%eh_token : !cir.eh_token) { + cir.lifetime.start %0 : !cir.ptr<!s32i> + cir.cleanup.scope { + cir.call @some_call() : () -> () + %catch_token, %exception = cir.begin_catch %eh_token : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) + cir.end_catch %catch_token : !cir.catch_token + cir.yield + } cleanup all { + cir.lifetime.end %0 : !cir.ptr<!s32i> + cir.yield + } + cir.yield + } + } + cir.return +} + +} + +// ----- + +!s32i = !cir.int<s, 32> +!void = !cir.void + +module { + +// The catch parameter's lifetime marker is always followed by the +// cir.cleanup.scope that runs its lifetime-end marker. +cir.func dso_local @catch_handler_lifetime_start_without_cleanup_scope() { + cir.scope { + %0 = cir.alloca "i" align(4) : !cir.ptr<!s32i> + // expected-error @below {{'cir.lifetime.start' in a catch handler region must be followed by the 'cir.cleanup.scope' of its lifetime-end cleanup}} + cir.try { + cir.yield + } catch all (%eh_token : !cir.eh_token) { + cir.lifetime.start %0 : !cir.ptr<!s32i> + %catch_token, %exception = cir.begin_catch %eh_token : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) + cir.end_catch %catch_token : !cir.catch_token + cir.lifetime.end %0 : !cir.ptr<!s32i> + cir.yield + } + } + cir.return +} + +} + +// ----- + +!s32i = !cir.int<s, 32> +!void = !cir.void + +module { + +// The catch parameter is the only variable whose lifetime starts before the +// catch is entered, so a handler carries at most one lifetime marker. +cir.func dso_local @catch_handler_two_lifetime_starts() { + cir.scope { + %0 = cir.alloca "i" align(4) : !cir.ptr<!s32i> + %1 = cir.alloca "j" align(4) : !cir.ptr<!s32i> + // expected-error @below {{'cir.lifetime.start' in a catch handler region must be followed by the 'cir.cleanup.scope' of its lifetime-end cleanup}} + cir.try { + cir.yield + } catch all (%eh_token : !cir.eh_token) { + cir.lifetime.start %0 : !cir.ptr<!s32i> + cir.lifetime.start %1 : !cir.ptr<!s32i> + cir.cleanup.scope { + %catch_token, %exception = cir.begin_catch %eh_token : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) + cir.end_catch %catch_token : !cir.catch_token + cir.yield + } cleanup all { + cir.lifetime.end %1 : !cir.ptr<!s32i> + cir.lifetime.end %0 : !cir.ptr<!s32i> + cir.yield + } + cir.yield + } + } + cir.return +} + +} + +// ----- + module { cir.func dso_local @invalid_unwind_with_catch_all() { >From 0471ae282798f98b8922d2f985ddd4acac211d19 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Mon, 31 Aug 2026 10:48:33 -0700 Subject: [PATCH 2/2] Address review feedback --- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 7 +++++- clang/test/CIR/CodeGen/lifetime-marker.cpp | 4 +-- clang/test/CIR/IR/invalid-try-catch.cir | 29 ++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 67738cbafbcb2..d429493dabd1b 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -4478,13 +4478,18 @@ LogicalResult cir::TryOp::verify() { return emitOpError("'cir.lifetime.start' in a catch handler region " "must be followed by the 'cir.cleanup.scope' of " "its lifetime-end cleanup"); + if (lifetimeScope.getBodyRegion().empty()) + return emitOpError( + "'cir.lifetime.start' in a catch handler region must be " + "followed by the 'cir.cleanup.scope' of its lifetime-end " + "cleanup"); mlir::Block &scopeBody = lifetimeScope.getBodyRegion().front(); firstOp = scopeBody.empty() ? nullptr : &scopeBody.front(); } if (mlir::isa_and_present<cir::ConstructCatchParamOp>(firstOp)) firstOp = firstOp->getNextNode(); - if (!firstOp || !mlir::isa<cir::BeginCatchOp>(firstOp)) + if (!mlir::isa_and_present<cir::BeginCatchOp>(firstOp)) return emitOpError( "catch handler region must start with 'cir.begin_catch'"); } diff --git a/clang/test/CIR/CodeGen/lifetime-marker.cpp b/clang/test/CIR/CodeGen/lifetime-marker.cpp index 9265ab16b1208..dde66fac13d96 100644 --- a/clang/test/CIR/CodeGen/lifetime-marker.cpp +++ b/clang/test/CIR/CodeGen/lifetime-marker.cpp @@ -4,9 +4,9 @@ // RUN: FileCheck --input-file=%t.ll %s --check-prefix=LLVM // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t-o0.cir // RUN: FileCheck --input-file=%t-o0.cir %s --implicit-check-not "cir.lifetime" -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -fcxx-exceptions -fexceptions -fclangir -emit-cir %s -o %t-eh.cir +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -fcxx-exceptions -fexceptions -fclangir -emit-cir %s -o %t-eh.cir // RUN: FileCheck --input-file=%t-eh.cir %s --check-prefix=CIR-EH -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -fcxx-exceptions -fexceptions -fclangir -emit-llvm -disable-llvm-passes %s -o %t-eh.ll +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -fcxx-exceptions -fexceptions -fclangir -emit-llvm -disable-llvm-passes %s -o %t-eh.ll // RUN: FileCheck --input-file=%t-eh.ll %s --check-prefixes=LLVM-EH void use(int); diff --git a/clang/test/CIR/IR/invalid-try-catch.cir b/clang/test/CIR/IR/invalid-try-catch.cir index 367733a6d7e6f..50da221af2c52 100644 --- a/clang/test/CIR/IR/invalid-try-catch.cir +++ b/clang/test/CIR/IR/invalid-try-catch.cir @@ -250,6 +250,35 @@ cir.func dso_local @catch_handler_lifetime_start_without_cleanup_scope() { module { +// A cleanup.scope following a lifetime start marker must not be empty. +cir.func dso_local @catch_handler_lifetime_start_empty_cleanup_body() { + cir.scope { + %0 = cir.alloca "i" align(4) : !cir.ptr<!s32i> + // expected-error @below {{'cir.lifetime.start' in a catch handler region must be followed by the 'cir.cleanup.scope' of its lifetime-end cleanup}} + cir.try { + cir.yield + } catch all (%eh_token : !cir.eh_token) { + cir.lifetime.start %0 : !cir.ptr<!s32i> + cir.cleanup.scope { + } cleanup all { + cir.lifetime.end %0 : !cir.ptr<!s32i> + cir.yield + } + cir.yield + } + } + cir.return +} + +} + +// ----- + +!s32i = !cir.int<s, 32> +!void = !cir.void + +module { + // The catch parameter is the only variable whose lifetime starts before the // catch is entered, so a handler carries at most one lifetime marker. cir.func dso_local @catch_handler_two_lifetime_starts() { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
