diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -4442,47 +4442,34 @@ // 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. + // + // This violates the language semantics in two ways: + // + // 1. If you have `struct A : public B {};`, and throw a B*, that should not + // be caught by `catch (A*& ptrref)` (though it would via `catch (A* + // ptr)`). But, it will be. + // + // 2. Assigning to the pointer through the reference ought to be visible to + // other catch blocks if we rethrow the exception after the mutation. But it + // won't be. + // + // Unfortunately, there's nothing we can actually do about that within the + // current (long-standing) Itanium EH ABI. GCC has the same issue. + // So, we'll just dump the by-value adjusted pointer into an + // alloca, and have the reference point to that. _shrug_. if (const PointerType *PT = dyn_cast(CaughtType)) { QualType PointeeType = PT->getPointeeType(); + // Pull the pointer for the reference type off. + llvm::Type *PtrTy = CGF.ConvertTypeForMem(CaughtType); - // When catching by reference, generally we should just ignore - // this by-value pointer and use the exception object instead. - if (!PointeeType->isRecordType()) { - - // Exn points to the struct _Unwind_Exception header, which - // we have to skip past in order to reach the exception data. - unsigned HeaderSize = - CGF.CGM.getTargetCodeGenInfo().getSizeOfUnwindException(); - AdjustedExn = - CGF.Builder.CreateConstGEP1_32(CGF.Int8Ty, Exn, HeaderSize); - - // However, if we're catching a pointer-to-record type that won't - // work, because the personality function might have adjusted - // the pointer. There's actually no way for us to fully satisfy - // the language/ABI contract here: we can't use Exn because it - // might have the wrong adjustment, but we can't use the by-value - // pointer because it's off by a level of abstraction. - // - // The current solution is to dump the adjusted pointer into an - // alloca, which breaks language semantics (because changing the - // pointer doesn't change the exception) but at least works. - // The better solution would be to filter out non-exact matches - // and rethrow them, but this is tricky because the rethrow - // really needs to be catchable by other sites at this landing - // pad. The best solution is to fix the personality function. - } else { - // Pull the pointer for the reference type off. - llvm::Type *PtrTy = CGF.ConvertTypeForMem(CaughtType); - - // Create the temporary and write the adjusted pointer into it. - Address ExnPtrTmp = + // Create the temporary and write the adjusted pointer into it. + Address ExnPtrTmp = CGF.CreateTempAlloca(PtrTy, CGF.getPointerAlign(), "exn.byref.tmp"); - llvm::Value *Casted = CGF.Builder.CreateBitCast(AdjustedExn, PtrTy); - CGF.Builder.CreateStore(Casted, ExnPtrTmp); + llvm::Value *Casted = CGF.Builder.CreateBitCast(AdjustedExn, PtrTy); + CGF.Builder.CreateStore(Casted, ExnPtrTmp); - // Bind the reference to the temporary. - AdjustedExn = ExnPtrTmp.getPointer(); - } + // Bind the reference to the temporary. + AdjustedExn = ExnPtrTmp.getPointer(); } llvm::Value *ExnCast = diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h --- a/clang/lib/CodeGen/TargetInfo.h +++ b/clang/lib/CodeGen/TargetInfo.h @@ -70,16 +70,6 @@ const FunctionDecl *Callee, const CallArgList &Args) const {} - /// Determines the size of struct _Unwind_Exception on this platform, - /// in 8-bit units. The Itanium ABI defines this as: - /// struct _Unwind_Exception { - /// uint64 exception_class; - /// _Unwind_Exception_Cleanup_Fn exception_cleanup; - /// uint64 private_1; - /// uint64 private_2; - /// }; - virtual unsigned getSizeOfUnwindException() const; - /// Controls whether __builtin_extend_pointer should sign-extend /// pointers to uint64_t or zero-extend them (the default). Has /// no effect for targets: diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp --- a/clang/lib/CodeGen/TargetInfo.cpp +++ b/clang/lib/CodeGen/TargetInfo.cpp @@ -440,18 +440,6 @@ TargetCodeGenInfo::~TargetCodeGenInfo() = default; -// If someone can figure out a general rule for this, that would be great. -// It's probably just doomed to be platform-dependent, though. -unsigned TargetCodeGenInfo::getSizeOfUnwindException() const { - // Verified for: - // x86-64 FreeBSD, Linux, Darwin - // x86-32 FreeBSD, Linux, Darwin - // PowerPC Linux, Darwin - // ARM Darwin (*not* EABI) - // AArch64 Linux - return 32; -} - bool TargetCodeGenInfo::isNoProtoCallVariadic(const CallArgList &args, const FunctionNoProtoType *fnType) const { // The following conventions are known to require this to be false: @@ -6388,11 +6376,6 @@ return false; } - unsigned getSizeOfUnwindException() const override { - if (getABIInfo().isEABI()) return 88; - return TargetCodeGenInfo::getSizeOfUnwindException(); - } - void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &CGM) const override { if (GV->isDeclaration()) @@ -7859,11 +7842,9 @@ }; class MIPSTargetCodeGenInfo : public TargetCodeGenInfo { - unsigned SizeOfUnwindException; public: MIPSTargetCodeGenInfo(CodeGenTypes &CGT, bool IsO32) - : TargetCodeGenInfo(std::make_unique(CGT, IsO32)), - SizeOfUnwindException(IsO32 ? 24 : 32) {} + : TargetCodeGenInfo(std::make_unique(CGT, IsO32)) {} int getDwarfEHStackPointer(CodeGen::CodeGenModule &CGM) const override { return 29; @@ -7920,9 +7901,6 @@ bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF, llvm::Value *Address) const override; - unsigned getSizeOfUnwindException() const override { - return SizeOfUnwindException; - } }; } diff --git a/clang/test/CodeGenCXX/eh.cpp b/clang/test/CodeGenCXX/eh.cpp --- a/clang/test/CodeGenCXX/eh.cpp +++ b/clang/test/CodeGenCXX/eh.cpp @@ -243,26 +243,11 @@ } } -// __cxa_begin_catch returns pointers by value, even when catching by reference -// +// __cxa_begin_catch returns pointers by value, even when catching by reference. +// This violates the langauge spec, but is not solveable without Itanium EH ABI +// changes. See https://github.com/llvm/llvm-project/issues/55340 namespace test11 { void opaque(); - - // CHECK-LABEL: define{{.*}} void @_ZN6test113fooEv() - void foo() { - try { - // CHECK: invoke void @_ZN6test116opaqueEv() - opaque(); - } catch (int**&p) { - // CHECK: [[EXN:%.*]] = load i8*, i8** - // CHECK-NEXT: call i8* @__cxa_begin_catch(i8* [[EXN]]) [[NUW]] - // CHECK-NEXT: [[ADJ1:%.*]] = getelementptr i8, i8* [[EXN]], i32 32 - // CHECK-NEXT: [[ADJ2:%.*]] = bitcast i8* [[ADJ1]] to i32*** - // CHECK-NEXT: store i32*** [[ADJ2]], i32**** [[P:%.*]] - // CHECK-NEXT: call void @__cxa_end_catch() [[NUW]] - } - } - struct A {}; // CHECK-LABEL: define{{.*}} void @_ZN6test113barEv() diff --git a/clang/test/CodeGenCXX/sizeof-unwind-exception.cpp b/clang/test/CodeGenCXX/sizeof-unwind-exception.cpp deleted file mode 100644 --- a/clang/test/CodeGenCXX/sizeof-unwind-exception.cpp +++ /dev/null @@ -1,33 +0,0 @@ -// RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=X86-64 -// RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=X86-32 -// RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=ARM-DARWIN -// RUN: %clang_cc1 -no-opaque-pointers -triple arm-unknown-gnueabi -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=ARM-EABI -// RUN: %clang_cc1 -no-opaque-pointers -triple mipsel-unknown-unknown -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=MIPS - -void foo(); -void test() { - try { - foo(); - } catch (int *&i) { - *i = 5; - } -} - -// PR10789: different platforms have different sizes for struct UnwindException. - -// X86-64: [[T0:%.*]] = tail call i8* @__cxa_begin_catch(i8* [[EXN:%.*]]) [[NUW:#[0-9]+]] -// X86-64-NEXT: [[T1:%.*]] = getelementptr i8, i8* [[EXN]], i64 32 -// X86-32: [[T0:%.*]] = tail call i8* @__cxa_begin_catch(i8* [[EXN:%.*]]) [[NUW:#[0-9]+]] -// X86-32-NEXT: [[T1:%.*]] = getelementptr i8, i8* [[EXN]], i64 32 -// ARM-DARWIN: [[T0:%.*]] = tail call i8* @__cxa_begin_catch(i8* [[EXN:%.*]]) [[NUW:#[0-9]+]] -// ARM-DARWIN-NEXT: [[T1:%.*]] = getelementptr i8, i8* [[EXN]], i64 32 -// ARM-EABI: [[T0:%.*]] = tail call i8* @__cxa_begin_catch(i8* [[EXN:%.*]]) [[NUW:#[0-9]+]] -// ARM-EABI-NEXT: [[T1:%.*]] = getelementptr i8, i8* [[EXN]], i32 88 -// MIPS: [[T0:%.*]] = tail call i8* @__cxa_begin_catch(i8* [[EXN:%.*]]) [[NUW:#[0-9]+]] -// MIPS-NEXT: [[T1:%.*]] = getelementptr i8, i8* [[EXN]], i32 24 - -// X86-64: attributes [[NUW]] = { nounwind } -// X86-32: attributes [[NUW]] = { nounwind } -// ARM-DARWIN: attributes [[NUW]] = { nounwind } -// ARM-EABI: attributes [[NUW]] = { nounwind } -// MIPS: attributes [[NUW]] = { nounwind }