llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Adam Smith (adams381) <details> <summary>Changes</summary> When a handler catches a reference to a pointer, __cxa_begin_catch returns the caught pointer by value instead of the address of the exception object, and how the reference is bound depends on whether the pointer points to a class. The CIR type of the catch parameter cannot always tell whether it is a reference to a pointer, or whether the pointee is a class. CIRGen now records both facts from the AST in two new InitCatchKind values, reference_to_pointer and reference_to_record_pointer. The Itanium EH lowering switches on the kind alone, the way classic codegen does. Assisted-by: Claude Code / Claude Opus 5.5 --- Patch is 63.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/229579.diff 10 Files Affected: - (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+20-3) - (modified) clang/lib/CIR/CodeGen/CIRGenException.cpp (+8-8) - (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+107) - (modified) clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp (+45-20) - (modified) clang/test/CIR/CodeGen/try-catch.cpp (+198-8) - (modified) clang/test/CIR/IR/construct-catch-param.cir (+173) - (modified) clang/test/CIR/IR/invalid-construct-catch-param.cir (+62) - (added) clang/test/CIR/IR/invalid-init-catch-param.cir (+258) - (added) clang/test/CIR/Transforms/eh-abi-lowering-catch-param-ref.cir (+216) - (modified) clang/test/CIR/Transforms/eh-abi-lowering-construct-catch-invalid.cir (+86-2) ``````````diff diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 985cc3712c581..e14ffdfdac392 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -8700,6 +8700,7 @@ def CIR_TryOp : CIR_RegionBranchOpBase<"try", [ ]; let hasVerifier = 1; + let hasRegionVerifier = 1; let hasLLVMLowering = false; } @@ -9286,6 +9287,8 @@ def CIR_InitCatchKind : CIR_I32Enum< I32EnumCase<"Objc", 3, "objc">, I32EnumCase<"TrivialCopy", 4, "trivial_copy">, I32EnumCase<"NonTrivialCopy", 5, "non_trivial_copy">, + I32EnumCase<"ReferenceToPointer", 6, "reference_to_pointer">, + I32EnumCase<"ReferenceToRecordPointer", 7, "reference_to_record_pointer">, ]>; def CIR_InitCatchKindAttr : CIR_EnumAttr<CIR_InitCatchKind, "init_catch">; @@ -9304,6 +9307,16 @@ def CIR_InitCatchParamOp : CIR_Op<"init_catch_param"> { types, an aggregate copy for record types, or a call to a copy constructor when one is required). + A catch parameter that is a reference to a pointer uses the + `reference_to_record_pointer` kind when the pointer's source-level pointee + type is a record (class, struct, or union), and `reference_to_pointer` + otherwise. A reference to any other type uses `reference`. + + A `reference_to_pointer` or `non_trivial_copy` operation requires its catch + handler to start with a `cir.construct_catch_param` of the same kind on the + same parameter. When the handler starts with a `cir.construct_catch_param` + on the parameter of the operation, the two must have the same kind. + Example: ``` @@ -9326,6 +9339,7 @@ def CIR_InitCatchParamOp : CIR_Op<"init_catch_param"> { qualified(type($exn_ptr)) `,` qualified(type($param_addr)) attr-dict }]; + let hasVerifier = 1; let hasLLVMLowering = false; } @@ -9349,9 +9363,11 @@ def CIR_ConstructCatchParamOp : CIR_Op<"construct_catch_param", [ copy of the object before `__cxa_begin_catch` is invoked. This operation takes a `!cir.eh_token` that represents the in-flight - exception and the alloca value that is used for the local copy of the - exception object. The `copy_fn` attribute is a flat symbol reference to a - `cir.func` thunk that copies the exception object to a local alloca value. + exception and the alloca value for the catch parameter. The optional + `copy_fn` attribute, present for `non_trivial_copy`, is a flat symbol + reference to a `cir.func` thunk that copies the exception object to a + local alloca value. For `reference_to_pointer`, the operation binds the + reference to the exception object itself. This operation is replaced with a target-specific representation during the EHABI lowering pass. For some targets, such as the Microsoft ABI, @@ -9377,6 +9393,7 @@ def CIR_ConstructCatchParamOp : CIR_Op<"construct_catch_param", [ qualified(type($param_addr)) attr-dict }]; + let hasVerifier = 1; let hasLLVMLowering = false; } diff --git a/clang/lib/CIR/CodeGen/CIRGenException.cpp b/clang/lib/CIR/CodeGen/CIRGenException.cpp index bd3e2014756e6..a2665798a37fd 100644 --- a/clang/lib/CIR/CodeGen/CIRGenException.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenException.cpp @@ -512,16 +512,16 @@ static void initCatchParam(CIRGenFunction &cgf, CIRGenBuilderTy &builder, CanQualType catchType = cgf.cgm.getASTContext().getCanonicalType(catchParam.getType()); cir::InitCatchKind kind; - bool shouldInitFromExnDirectly = false; - // If we're catching by reference, we can just cast the object - // pointer to the appropriate pointer. if (isa<ReferenceType>(catchType)) { QualType caughtType = cast<ReferenceType>(catchType)->getPointeeType(); - if (const PointerType *ptr = dyn_cast<PointerType>(caughtType)) { - shouldInitFromExnDirectly = !ptr->getPointeeType()->isRecordType(); - } - kind = cir::InitCatchKind::Reference; + const PointerType *ptr = dyn_cast<PointerType>(caughtType); + if (!ptr) + kind = cir::InitCatchKind::Reference; + else if (ptr->getPointeeType()->isRecordType()) + kind = cir::InitCatchKind::ReferenceToRecordPointer; + else + kind = cir::InitCatchKind::ReferenceToPointer; } else { cir::TypeEvaluationKind tek = cgf.getEvaluationKind(catchType); if (tek == cir::TEK_Aggregate) { @@ -555,7 +555,7 @@ static void initCatchParam(CIRGenFunction &cgf, CIRGenBuilderTy &builder, mlir::Location mloc = cgf.getLoc(loc); if (kind == cir::InitCatchKind::NonTrivialCopy || - (kind == cir::InitCatchKind::Reference && shouldInitFromExnDirectly)) { + kind == cir::InitCatchKind::ReferenceToPointer) { // Sanitizer-checked construction (UBSan vptr/derived-class checks, etc.) // would require additional adornments that cir.construct_catch_param does // not yet carry. diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 51b5bd1c58f4b..520565476cf7a 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -4995,6 +4995,62 @@ void cir::TryOp::getSuccessorRegions( regions.push_back(mlir::RegionSuccessor(&handlerRegion)); } +/// Return the cir.construct_catch_param that starts the catch handler with +/// entry block \p entryBlock, or null if there is none. The handler must +/// already have passed cir::TryOp::verify. +static cir::ConstructCatchParamOp +getLeadingConstructCatchParam(mlir::Block &entryBlock) { + mlir::Operation *firstOp = &entryBlock.front(); + if (mlir::isa<cir::LifetimeStartOp>(firstOp)) { + auto lifetimeScope = + mlir::cast<cir::CleanupScopeOp>(firstOp->getNextNode()); + firstOp = &lifetimeScope.getBodyRegion().front().front(); + } + return mlir::dyn_cast<cir::ConstructCatchParamOp>(firstOp); +} + +/// Verify that each cir.init_catch_param in \p handlerRegion, outside any +/// nested cir.try, agrees with the handler's leading cir.construct_catch_param +/// \p constructOp, which may be null. A cir.init_catch_param of kind +/// reference_to_pointer or non_trivial_copy needs \p constructOp on its +/// parameter, and a cir.init_catch_param on the parameter of \p constructOp +/// must have the kind of \p constructOp. +static LogicalResult +verifyCatchParamPairing(mlir::Region &handlerRegion, + cir::ConstructCatchParamOp constructOp) { + mlir::WalkResult result = + handlerRegion.walk<mlir::WalkOrder::PreOrder>([&](mlir::Operation *op) { + if (mlir::isa<cir::TryOp>(op)) + return mlir::WalkResult::skip(); + auto initOp = mlir::dyn_cast<cir::InitCatchParamOp>(op); + if (!initOp) + return mlir::WalkResult::advance(); + + cir::InitCatchKind kind = initOp.getKind(); + bool sameParam = + constructOp && constructOp.getParamAddr() == initOp.getParamAddr(); + if (sameParam && constructOp.getKind() != kind) { + initOp.emitOpError("kind '") + << cir::stringifyInitCatchKind(kind) + << "' does not match the kind '" + << cir::stringifyInitCatchKind(constructOp.getKind()) + << "' of the preceding 'cir.construct_catch_param'"; + return mlir::WalkResult::interrupt(); + } + if ((kind == cir::InitCatchKind::ReferenceToPointer || + kind == cir::InitCatchKind::NonTrivialCopy) && + !sameParam) { + initOp.emitOpError("'") + << cir::stringifyInitCatchKind(kind) + << "' requires a preceding 'cir.construct_catch_param' of the " + "same kind on the same parameter"; + return mlir::WalkResult::interrupt(); + } + return mlir::WalkResult::advance(); + }); + return failure(result.wasInterrupted()); +} + LogicalResult cir::TryOp::verify() { mlir::ArrayAttr handlerTypes = getHandlerTypes(); if (!handlerTypes) { @@ -5098,6 +5154,26 @@ LogicalResult cir::TryOp::verify() { return success(); } +LogicalResult cir::TryOp::verifyRegions() { + mlir::ArrayAttr handlerTypes = getHandlerTypes(); + if (!handlerTypes) + return success(); + + for (const auto &[typeAttr, handlerRegion] : + llvm::zip(handlerTypes, getHandlerRegions())) { + if (mlir::isa<cir::UnwindAttr, cir::EhFilterAttr, cir::EhUnexpectedAttr>( + typeAttr)) + continue; + mlir::Block &entryBlock = handlerRegion.front(); + if (mlir::isa<cir::EhTerminateOp>(entryBlock.front())) + continue; + if (failed(verifyCatchParamPairing( + handlerRegion, getLeadingConstructCatchParam(entryBlock)))) + return failure(); + } + return success(); +} + static void printTryHandlerRegions(mlir::OpAsmPrinter &printer, cir::TryOp op, mlir::MutableArrayRef<mlir::Region> handlerRegions, @@ -5344,10 +5420,41 @@ LogicalResult cir::MemChrOp::verify() { cir::CIRDialect::getSizeTypeWidthAttrName()); } +//===----------------------------------------------------------------------===// +// InitCatchParamOp +//===----------------------------------------------------------------------===// + +/// For the reference_to_pointer and reference_to_record_pointer kinds, emit an +/// error on \p op unless \p paramAddrType is the type of the address of a +/// reference to a pointer. Any other \p kind is accepted. +static LogicalResult verifyCatchParamAddr(mlir::Operation *op, + cir::InitCatchKind kind, + cir::PointerType paramAddrType) { + if (kind != cir::InitCatchKind::ReferenceToPointer && + kind != cir::InitCatchKind::ReferenceToRecordPointer) + return success(); + + auto refType = mlir::dyn_cast<cir::PointerType>(paramAddrType.getPointee()); + if (!refType || !mlir::isa<cir::PointerType>(refType.getPointee())) + return op->emitOpError("'") + << cir::stringifyInitCatchKind(kind) + << "' requires 'param_addr' to be the address of a reference to a " + "pointer"; + return success(); +} + +LogicalResult cir::InitCatchParamOp::verify() { + return verifyCatchParamAddr(*this, getKind(), getParamAddr().getType()); +} + //===----------------------------------------------------------------------===// // ConstructCatchParamOp //===----------------------------------------------------------------------===// +LogicalResult cir::ConstructCatchParamOp::verify() { + return verifyCatchParamAddr(*this, getKind(), getParamAddr().getType()); +} + LogicalResult cir::ConstructCatchParamOp::verifySymbolUses( SymbolTableCollection &symbolTable) { auto copyFnAttr = getCopyFnAttr(); diff --git a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp index 77ce7434c8ed3..72a8cffc3b060 100644 --- a/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp +++ b/clang/lib/CIR/Dialect/Transforms/EHABILowering.cpp @@ -22,7 +22,9 @@ // - cir.resume → cir.resume.flat // - !cir.eh_token values → (!cir.ptr<!void>, !u32i) value pairs // - cir.construct_catch_param → __cxa_get_exception_ptr + inlined -// catch-copy thunk body +// catch-copy thunk body, or a store of the +// exception object address for +// reference_to_pointer // - personality function set on functions requiring EH // //===----------------------------------------------------------------------===// @@ -31,6 +33,7 @@ #include "mlir/IR/Builders.h" #include "mlir/IR/IRMapping.h" #include "mlir/IR/PatternMatch.h" +#include "clang/CIR/Dialect/IR/CIRDataLayout.h" #include "clang/CIR/Dialect/IR/CIRDialect.h" #include "clang/CIR/Dialect/IR/CIROpsEnums.h" #include "clang/CIR/Dialect/IR/CIRTypes.h" @@ -828,7 +831,7 @@ ItaniumEHLowering::lowerConstructCatchParam(cir::ConstructCatchParamOp op, cir::PointerType paramAddrType = mlir::cast<cir::PointerType>(paramAddr.getType()); - if (op.getKind() == cir::InitCatchKind::Reference) { + if (op.getKind() == cir::InitCatchKind::ReferenceToPointer) { assert(!MissingFeatures::sizeOfUnwindException()); constexpr unsigned headerSize = 32; @@ -848,8 +851,8 @@ ItaniumEHLowering::lowerConstructCatchParam(cir::ConstructCatchParamOp op, } if (op.getKind() != cir::InitCatchKind::NonTrivialCopy) - return op.emitError( - "ConstructCatchParam: only non_trivial_copy is supported"); + return op.emitOpError("only reference_to_pointer and non_trivial_copy " + "kinds are supported"); ensureRuntimeDecls(loc); ensureClangCallTerminate(loc); @@ -976,9 +979,14 @@ mlir::LogicalResult ItaniumEHLowering::lowerTryThrow(cir::TryThrowOp op) { /// pointer returned by __cxa_begin_catch. The shape of the lowering /// depends on the init catch kind: /// -/// - Reference: the begin_catch result is -/// the pointer value itself, so just bitcast and store it into the alloca -/// except if it reference of pointer of record. +/// - Reference: the begin_catch result is the adjusted exception object +/// pointer, so just bitcast and store it into the alloca. +/// - ReferenceToPointer: the companion `cir.construct_catch_param` +/// already bound the reference to the exception object, so this lowering +/// is a no-op. +/// - ReferenceToRecordPointer: the begin_catch result is the adjusted +/// pointer value itself, so store it into a temporary and bind the +/// reference to the temporary. /// - Pointer: the begin_catch result is /// the pointer value itself, so just bitcast and store it into the /// alloca. @@ -1002,24 +1010,41 @@ void ItaniumEHLowering::lowerInitCatchParam(cir::InitCatchParamOp op) { switch (kind) { case InitCatchKind::Reference: { - // We have no way to tell the personality function that we're - // catching by reference, so if we're catching a pointer, - // __cxa_begin_catch will actually return that pointer by value. - if (const auto ref = mlir::dyn_cast<cir::PointerType>(elementType)) { - // When catching by reference, generally we should just ignore - // this by-value pointer and use the exception object instead. - if (auto ptr = mlir::dyn_cast<cir::PointerType>(ref.getPointee())) - if (!mlir::isa<cir::RecordType>(ptr.getPointee())) - // Extracting and storing the actual exception object was performed by - // cir.construct_catch_param before cir.begin_catch. - break; - } - mlir::Value casted = cir::CastOp::create(builder, loc, elementType, cir::CastKind::bitcast, exnPtr); cir::StoreOp::create(builder, loc, casted, paramAddr, {}, {}, {}, {}, {}); break; } + case InitCatchKind::ReferenceToPointer: + // The reference was bound to the exception object by the matching + // cir.construct_catch_param before cir.begin_catch. + break; + case InitCatchKind::ReferenceToRecordPointer: { + // __cxa_begin_catch returns the caught pointer by value, and the + // personality function may have adjusted it. Store the adjusted pointer + // into a temporary and bind the reference to it. Assigning to the catch + // parameter then changes the temporary, not the exception object, which + // departs from the language semantics. + auto caughtType = mlir::cast<cir::PointerType>( + mlir::cast<cir::PointerType>(elementType).getPointee()); + cir::AllocaOp byRefTmp; + { + mlir::OpBuilder::InsertionGuard guard(builder); + builder.setInsertionPointToStart( + &op->getParentOfType<cir::FuncOp>().getBody().front()); + cir::CIRDataLayout dataLayout(mod); + uint64_t alignment = + dataLayout.getAlignment(caughtType, /*useABIAlign=*/true).value(); + byRefTmp = + cir::AllocaOp::create(builder, loc, elementType, "exn.byref.tmp", + builder.getI64IntegerAttr(alignment)); + } + mlir::Value casted = cir::CastOp::create(builder, loc, caughtType, + cir::CastKind::bitcast, exnPtr); + cir::StoreOp::create(builder, loc, casted, byRefTmp, {}, {}, {}, {}, {}); + cir::StoreOp::create(builder, loc, byRefTmp, paramAddr, {}, {}, {}, {}, {}); + break; + } case InitCatchKind::TrivialCopy: { mlir::Value srcPtr = cir::CastOp::create(builder, loc, paramAddrType, cir::CastKind::bitcast, exnPtr); diff --git a/clang/test/CIR/CodeGen/try-catch.cpp b/clang/test/CIR/CodeGen/try-catch.cpp index 2b4108ca0d270..8f34a2987c216 100644 --- a/clang/test/CIR/CodeGen/try-catch.cpp +++ b/clang/test/CIR/CodeGen/try-catch.cpp @@ -1,9 +1,9 @@ // 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: FileCheck --input-file=%t-cir.ll %s -check-prefixes=LLVM,SHARED // 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-prefixes=OGCG,SHARED void empty_try_block_with_catch_all() { try {} catch (...) {} @@ -1121,9 +1121,9 @@ void call_function_inside_try_catch_with_ref_ptr_of_record_exception_type() { // CIR: %[[CALL:.*]] = cir.call @_Z8divisionv() : () -> (!s32i {llvm.noundef}) // CIR: cir.yield // CIR: } catch [type #cir.global_view<@_ZTIP6Record> : !cir.ptr<!u8i>] (%[[EH_TOKEN:.*]]: !cir.eh_token {{.*}}) { -// CIR: %[[CATCH_TOKEN:.*]], %[[EXN_PTR:.*]] = cir.begin_catch %[[EH_TOKEN]] : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) +// CIR-NEXT: %[[CATCH_TOKEN:.*]], %[[EXN_PTR:.*]] = cir.begin_catch %[[EH_TOKEN]] : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) // CIR: cir.cleanup.scope { -// CIR: cir.init_catch_param reference %[[EXN_PTR]] to %{{.*}} : !cir.ptr<!void>, !cir.ptr<!cir.ptr<!cir.ptr<!rec_Record>>> +// CIR: cir.init_catch_param reference_to_record_pointer %[[EXN_PTR]] to %[[E_ADDR]] : !cir.ptr<!void>, !cir.ptr<!cir.ptr<!cir.ptr<!rec_Record>>> // CIR: cir.yield // CIR: } cleanup all { // CIR: cir.end_catch %[[CATCH_TOKEN]] : !cir.catch_token @@ -1136,6 +1136,8 @@ void call_function_inside_try_catch_with_ref_ptr_of_record_exception_type() { // CIR: } // LLVM: define {{.*}} void @_Z68call_function_inside_try_catch_with_ref_ptr_of_record_exception_typev() {{.*}} personality ptr @__gxx_personality_v0 +// LLVM: %[[EXN_BYREF_TMP:.*]] = alloca ptr, align 8 +// LLVM: %[[E_ADDR:.*]] = alloca ptr, align 8 // LLVM: br label %[[TRY_SCOPE:.*]] // LLVM: [[TRY_SCOPE]]: // LLVM: br label %[[TRY_BEGIN:.*]] @@ -1163,10 +1165,12 @@ void call_function_inside_try_catch_with_ref_ptr_of_record_exception_type() { // LLVM: [[BEGIN_CATCH]]: // LLVM: %[[EXN_OBJ_PHI2:.*]] = phi ptr [ %[[EXN_OBJ_PHI1:.*]], %[[DISPATCH:.*]] ] // LLVM: %[[EH_SELECTOR_PHI2:.*]] = phi i32 [ %[[EH_SELECTOR_PHI1:.*]], %[[DISPATCH:.*]] ] -// LLVM: %[[TOKEN:.*]] = call ptr @__cxa_begin_catch(ptr %[[EXN_OBJ_PHI2]]) +// LLVM-NEXT: %[[TOKEN:.*]] = call ptr @__cxa_begin_catch(ptr %[[EXN_OBJ_PHI2]]) // LLVM: br label %[[CATCH_BODY:.*]] // LLVM: [[CATCH_BODY]]: -// LLVM: br label %[[END_CATCH:.*]] +// LLVM-NEXT: store ptr %[[TOKEN]], ptr %[[EXN_BYREF_TMP]], align 8 +// LLVM-NEXT: store ptr %[[EXN_BYREF_TMP]], ptr %[[E_ADDR]], align 8 +// LLVM-NEXT: br label %[[END_CATCH:.*]] // LLVM: [[END_CATCH]]: // LLVM: call void @__cxa_end_catch() // LLVM: br label %[[END_DISPATCH:.*]] @@ -1638,10 +1642,10 @@ int init_catch_param_with_ref_to_ptr_to_non_record() { // CIR: %[[CALL:.*]] = cir.call @_Z8divisionv() : () -> (!s32i {llvm.noundef}) // CIR: cir.yield // CIR: } catch [type #cir.global_view<@_ZTIPi> : !cir.ptr<!u8i>] (%[[TOKEN:.*]]: !cir.eh_token {{.*}}) { -// CIR: cir.construct_catch_param reference %[[TOKEN]] to %[[P_ADD... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/229579 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
