rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added subscribers: kristof.beyls, sanjoy, aemerson.
This patch fixes three issues with how Clang emits code to catch exceptions by
reference:
1. We would sometimes bind the reference directly to the exception object when
a temporary is required (when a pointer-to-member is converted by a
qualification conversion but not adjusted).
2. We would sometimes bind the reference to a temporary when a direct binding
is required (when catching a pointer-to-class-object where no derived-to-base
or qualification conversion is required).
3. We would incorrectly allow a non-const (or volatile) reference to catch an
exception of a different type that can be converted to the right type.
A single strategy is used to fix all three issues: where necessary, we extract
the `type_info` of the exception object from the `__cxa_exception` header, and
compare it directly against the reference's pointee `type_info`. For cases #1
and #2, this tells us whether to bind to a temporary object or to the exception
object; for case #3, if the types don't match and the reference can't bind to a
temporary, we rethrow the exception through the remaining `catch` clauses of
the `try` statement.
This patch is insufficiently tested (no changes to the test suite, and no
testing has been done whatsoever for the ARM64 non-unique `type_info` case),
but I'm posting it now to gauge whether this is a direction we want to pursue
for DR388 (as opposed to continuing to get this wrong until the Itanium C++ ABI
has a chance to take an ABI break to properly handle catch-by-reference).
[Despite the different approach to determining catchability, the MS ABI appears
to have exactly the same bugs (it too does not distinguish between catch by
value and catch by reference), and I would assume the same approach would also
work there, assuming the actual type of the exception object can be discerned,
but I've not looked at all at implementing this in that ABI.]
Repository:
rC Clang
https://reviews.llvm.org/D42092
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGCleanup.h
lib/CodeGen/CGException.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
Index: lib/CodeGen/TargetInfo.h
===================================================================
--- lib/CodeGen/TargetInfo.h
+++ lib/CodeGen/TargetInfo.h
@@ -65,6 +65,11 @@
virtual void emitTargetMD(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const {}
+ /// Determines the size of struct __cxa_exception on this platform,
+ /// in 8-bit units, excluding the trailing _Unwind_Exception member.
+ /// The Itanium ABI defines the layout of this struct.
+ virtual unsigned getSizeOfCxaException() const;
+
/// Determines the size of struct _Unwind_Exception on this platform,
/// in 8-bit units. The Itanium ABI defines this as:
/// struct _Unwind_Exception {
Index: lib/CodeGen/TargetInfo.cpp
===================================================================
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -369,6 +369,16 @@
TargetCodeGenInfo::~TargetCodeGenInfo() { delete Info; }
+unsigned TargetCodeGenInfo::getSizeOfCxaException() const {
+ // __cxa_exception comprises 5 pointers followed by 2 ints followed by 4
+ // more pointers, then the _Unwind_Exception object. We assume that no
+ // padding is introduced between these fields.
+ ASTContext &Ctx = Info->getContext();
+ return (9 * Ctx.getTypeSizeInChars(Ctx.VoidPtrTy) +
+ 2 * Ctx.getTypeSizeInChars(Ctx.IntTy))
+ .getQuantity();
+}
+
// 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 {
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -120,7 +120,8 @@
void emitRethrow(CodeGenFunction &CGF, bool isNoReturn) override;
void emitThrow(CodeGenFunction &CGF, const CXXThrowExpr *E) override;
- void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C) override;
+ void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C,
+ ArrayRef<EHCatchScope::Handler> LaterHandlers) override;
llvm::GlobalVariable *getMSCompleteObjectLocator(const CXXRecordDecl *RD,
const VPtrInfo &Info);
@@ -904,8 +905,9 @@
};
}
-void MicrosoftCXXABI::emitBeginCatch(CodeGenFunction &CGF,
- const CXXCatchStmt *S) {
+void MicrosoftCXXABI::emitBeginCatch(
+ CodeGenFunction &CGF, const CXXCatchStmt *S,
+ ArrayRef<EHCatchScope::Handler> LaterHandlers) {
// In the MS ABI, the runtime handles the copy, and the catch handler is
// responsible for destruction.
VarDecl *CatchParam = S->getExceptionDecl();
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -181,7 +181,11 @@
void emitRethrow(CodeGenFunction &CGF, bool isNoReturn) override;
void emitThrow(CodeGenFunction &CGF, const CXXThrowExpr *E) override;
- void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C) override;
+ void InitCatchParam(CodeGenFunction &CGF, const VarDecl &CatchParam,
+ Address ParamAddr, SourceLocation Loc,
+ ArrayRef<EHCatchScope::Handler> LaterHandlers);
+ void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C,
+ ArrayRef<EHCatchScope::Handler> LaterHandlers) override;
llvm::CallInst *
emitTerminateForUnexpectedException(CodeGenFunction &CGF,
@@ -2580,6 +2584,9 @@
/// \param DLLExport - true to mark the RTTI value as DLLExport
llvm::Constant *BuildTypeInfo(QualType Ty, bool Force = false,
bool DLLExport = false);
+
+ llvm::Value *EmitCompareTypeInfo(CodeGenFunction &CGF, QualType Ty,
+ llvm::Value *TypeInfo);
};
}
@@ -3763,12 +3770,61 @@
return call;
}
+llvm::Value *ItaniumRTTIBuilder::EmitCompareTypeInfo(CodeGenFunction &CGF,
+ QualType Ty,
+ llvm::Value *TypeInfo) {
+ // Start by just comparing the address of the type_info.
+ llvm::Value *Equal = CGF.Builder.CreateICmpEQ(BuildTypeInfo(Ty), TypeInfo);
+
+ // For types with unique type_info, the address comparison is sufficient.
+ llvm::GlobalVariable::LinkageTypes Linkage = getTypeInfoLinkage(CGM, Ty);
+ if (CXXABI.classifyRTTIUniqueness(Ty, Linkage) == ItaniumCXXABI::RUK_Unique)
+ return Equal;
+
+ // Otherwise, we need to compare the type name string.
+ llvm::BasicBlock *CurBlock = CGF.Builder.GetInsertBlock();
+ llvm::BasicBlock *CmpTypenameBlock = CGF.createBasicBlock("exc.typename.cmp");
+ llvm::BasicBlock *CmpDoneBlock = CGF.createBasicBlock("exc.typename.done");
+ CGF.Builder.CreateCondBr(Equal, CmpDoneBlock, CmpTypenameBlock);
+
+ CGF.EmitBlock(CmpTypenameBlock);
+
+ // The type name is the second pointer member, just after the vptr. Remove
+ // the high "requires string comparison" bit.
+ // FIXME: This is ARM64-specific.
+ llvm::Value *TypeNameField = CGF.Builder.CreateConstGEP1_32(
+ CGF.Builder.CreateBitCast(TypeInfo, CGF.CGM.Int8PtrPtrTy), 1);
+ TypeNameField = CGF.Builder.CreatePtrToInt(TypeNameField, CGM.Int64Ty);
+ llvm::Constant *Mask =
+ llvm::ConstantInt::get(CGM.Int64Ty, (uint64_t(1) << 63) - 1);
+ TypeNameField = CGF.Builder.CreateAnd(TypeNameField, Mask);
+ TypeNameField = CGF.Builder.CreateIntToPtr(TypeNameField, CGM.Int8PtrTy);
+
+ // Emit a strcmp between the type_info names.
+ Address TypeInfoStringAddr(TypeNameField, CGF.getPointerAlign());
+ llvm::Value *TypeInfoString = CGF.Builder.CreateLoad(TypeInfoStringAddr);
+ llvm::Value *Strcmp = CGM.CreateRuntimeFunction(
+ llvm::FunctionType::get(CGM.IntTy, {CGM.Int8PtrTy, CGM.Int8PtrTy},
+ /*IsVarArgs=*/false),
+ "strcmp");
+ llvm::CallInst *StrcmpValue = CGF.EmitNounwindRuntimeCall(
+ Strcmp, {TypeInfoString, GetAddrOfTypeName(Ty, Linkage)});
+ llvm::Value *StrcmpResult = CGF.Builder.CreateICmpEQ(
+ StrcmpValue, llvm::ConstantInt::get(CGM.IntTy, 0));
+ CGF.EmitBranch(CmpDoneBlock);
+
+ CGF.EmitBlock(CmpDoneBlock);
+ llvm::PHINode *PHI = CGF.Builder.CreatePHI(Equal->getType(), 2);
+ PHI->addIncoming(Equal, CurBlock);
+ PHI->addIncoming(StrcmpResult, CmpTypenameBlock);
+ return PHI;
+}
+
/// A "special initializer" callback for initializing a catch
/// parameter during catch initialization.
-static void InitCatchParam(CodeGenFunction &CGF,
- const VarDecl &CatchParam,
- Address ParamAddr,
- SourceLocation Loc) {
+void ItaniumCXXABI::InitCatchParam(
+ CodeGenFunction &CGF, const VarDecl &CatchParam, Address ParamAddr,
+ SourceLocation Loc, ArrayRef<EHCatchScope::Handler> LaterHandlers) {
// Load the exception from where the landing pad saved it.
llvm::Value *Exn = CGF.getExceptionFromSlot();
@@ -3778,62 +3834,124 @@
// 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 (auto *RT = dyn_cast<ReferenceType>(CatchType)) {
+ QualType CaughtType = RT->getPointeeType();
bool EndCatchMightThrow = CaughtType->isRecordType();
+ bool CanBindToTemporary =
+ CaughtType.isConstQualified() && !CaughtType.isVolatileQualified();
+
+ // Determine whether a conversion was applied to the exception in
+ // forming the reference binding.
+ llvm::Value *MaybeTypeMatch = nullptr;
+ if (CaughtType->isPointerType() ||
+ (CaughtType->isMemberPointerType() && !CanBindToTemporary)) {
+ // Locate the type_info for the exception.
+ unsigned HeaderSize =
+ CGF.CGM.getTargetCodeGenInfo().getSizeOfCxaException();
+ llvm::Value *TypeInfoPtrPtr = CGF.Builder.CreateBitCast(
+ CGF.Builder.CreateConstGEP1_32(Exn, -HeaderSize),
+ CGF.CGM.VoidPtrPtrTy);
+ llvm::Value *TypeInfoPtr = CGF.Builder.CreateLoad(
+ Address(TypeInfoPtrPtr, CGF.getPointerAlign()), "exn.type_info");
+ MaybeTypeMatch = ItaniumRTTIBuilder(*this).EmitCompareTypeInfo(
+ CGF, CaughtType.getUnqualifiedType(), TypeInfoPtr);
+ }
// __cxa_begin_catch returns the adjusted object pointer.
llvm::Value *AdjustedExn = CallBeginCatch(CGF, Exn, EndCatchMightThrow);
- // 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 PointerType *PT = dyn_cast<PointerType>(CaughtType)) {
- QualType PointeeType = PT->getPointeeType();
-
- // 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 =
+ // If the reference is to a pointer or pointer-to-member type and
+ // is not const (non-volatile) reference, and was type-converted, resume
+ // unwinding rather than re-entering this catch block. Note that we need
+ // subsequent catches in the same try statement to catch such a rethrow.
+ // See C++ DR 388.
+ if (!CanBindToTemporary && MaybeTypeMatch) {
+ llvm::BasicBlock *MismatchBlock = CGF.createBasicBlock("exn.mismatch");
+ llvm::BasicBlock *CatchBlock = CGF.createBasicBlock("exn.match");
+ CGF.Builder.CreateCondBr(MaybeTypeMatch, CatchBlock, MismatchBlock);
+
+ // Pass the exception off to the C++ ABI library to deal with.
+ CGF.EmitBlock(MismatchBlock);
+ if (LaterHandlers.empty()) {
+ emitRethrow(CGF, true);
+ } else {
+ // FIXME: We could filter out definitely-unreachable catch handlers.
+ EHCatchScope *LaterCatchScope =
+ CGF.EHStack.pushCatch(LaterHandlers.size());
+ std::uninitialized_copy(LaterHandlers.begin(), LaterHandlers.end(),
+ LaterCatchScope->begin());
+ emitRethrow(CGF, true);
+ CGF.popCatchScope();
+ }
+
+ CGF.EmitBlock(CatchBlock);
+ MaybeTypeMatch = nullptr;
+ }
+
+ // __cxa_begin_catch returns pointers by value, so we need to find the
+ // actual exception object ourselves. Exn points to the struct
+ // _Unwind_Exception header, which we have to skip past in order to reach
+ // the exception data.
+ // We assume the ABI library returns a pointer to the exception object
+ // for non-pointer exceptions (if no conversion is applied).
+ llvm::Value *ExnObject;
+ if (CaughtType->isPointerType()) {
+ unsigned HeaderSize =
CGF.CGM.getTargetCodeGenInfo().getSizeOfUnwindException();
- AdjustedExn = CGF.Builder.CreateConstGEP1_32(Exn, HeaderSize);
+ llvm::Value *ExnObj = CGF.Builder.CreateConstGEP1_32(Exn, HeaderSize);
+ ExnObject = CGF.Builder.CreateBitCast(ExnObj, LLVMCatchTy, "exn.orig");
+ } else {
+ ExnObject =
+ CGF.Builder.CreateBitCast(AdjustedExn, LLVMCatchTy, "exn.byref");
+ }
- // 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 {
+ // If a conversion may have been performed, we need to materialize a local
+ // temporary to hold the converted value (even if the conversion didn't
+ // actually change the value, it must have a different address from that
+ // of the exception object).
+ if (MaybeTypeMatch) {
+ assert(CaughtType->isPointerType() || CaughtType->isMemberPointerType());
+ llvm::BasicBlock *PreBlock = CGF.Builder.GetInsertBlock();
+ llvm::BasicBlock *ConvBlock = CGF.createBasicBlock("exn.conv");
+ llvm::BasicBlock *ContBlock = CGF.createBasicBlock("exn.cont");
+ CGF.Builder.CreateCondBr(MaybeTypeMatch, ContBlock, ConvBlock);
+
+ // If the pointer was converted, the personality function might have
+ // adjusted it, and we need to materialize a separate temporary
+ // ourselves.
+ CGF.EmitBlock(ConvBlock);
+ llvm::Value *Temporary;
+ {
// Pull the pointer for the reference type off.
llvm::Type *PtrTy =
cast<llvm::PointerType>(LLVMCatchTy)->getElementType();
// 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);
+ CharUnits Align = CGF.getNaturalTypeAlignment(CaughtType);
+ Address ExnPtrTmp = CGF.CreateTempAlloca(PtrTy, Align, "exn.byref.tmp");
+ llvm::Value *Value;
+ if (CaughtType->isPointerType())
+ Value = CGF.Builder.CreateBitCast(AdjustedExn, PtrTy);
+ else {
+ Address ExnAddr(AdjustedExn, Align);
+ ExnAddr = CGF.Builder.CreateElementBitCast(ExnAddr, PtrTy);
+ Value = CGF.EmitLoadOfScalar(ExnAddr, false, CaughtType, Loc);
+ }
+ CGF.EmitStoreOfScalar(Value, ExnPtrTmp, false, CaughtType);
// Bind the reference to the temporary.
- AdjustedExn = ExnPtrTmp.getPointer();
+ Temporary = ExnPtrTmp.getPointer();
+ CGF.EmitBranch(ContBlock);
}
+
+ CGF.EmitBlock(ContBlock);
+ llvm::PHINode *Result = CGF.Builder.CreatePHI(LLVMCatchTy, 2);
+ Result->addIncoming(ExnObject, PreBlock);
+ Result->addIncoming(Temporary, ConvBlock);
+ ExnObject = Result;
}
- llvm::Value *ExnCast =
- CGF.Builder.CreateBitCast(AdjustedExn, LLVMCatchTy, "exn.byref");
- CGF.Builder.CreateStore(ExnCast, ParamAddr);
+ CGF.Builder.CreateStore(ExnObject, ParamAddr);
return;
}
@@ -3943,8 +4061,9 @@
/// Begins a catch statement by initializing the catch variable and
/// calling __cxa_begin_catch.
-void ItaniumCXXABI::emitBeginCatch(CodeGenFunction &CGF,
- const CXXCatchStmt *S) {
+void ItaniumCXXABI::emitBeginCatch(
+ CodeGenFunction &CGF, const CXXCatchStmt *S,
+ ArrayRef<EHCatchScope::Handler> LaterHandlers) {
// We have to be very careful with the ordering of cleanups here:
// C++ [except.throw]p4:
// The destruction [of the exception temporary] occurs
@@ -3977,7 +4096,8 @@
// Emit the local.
CodeGenFunction::AutoVarEmission var = CGF.EmitAutoVarAlloca(*CatchParam);
- InitCatchParam(CGF, *CatchParam, var.getObjectAddress(CGF), S->getLocStart());
+ InitCatchParam(CGF, *CatchParam, var.getObjectAddress(CGF), S->getLocStart(),
+ LaterHandlers);
CGF.EmitAutoVarCleanups(var);
}
Index: lib/CodeGen/CGException.cpp
===================================================================
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -557,12 +557,12 @@
llvm::BasicBlock *Handler = createBasicBlock("catch");
if (C->getExceptionDecl()) {
- // FIXME: Dropping the reference type on the type into makes it
- // impossible to correctly implement catch-by-reference
- // semantics for pointers. Unfortunately, this is what all
- // existing compilers do, and it's not clear that the standard
- // personality routine is capable of doing this right. See C++ DR 388:
- // http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#388
+ // Note that dropping the reference type may result in us catching
+ // exceptions we're not supposed to catch: for instance, a 'T&' handler
+ // cannot catch an exception that requires conversion to 'T', but a
+ // 'const T&' handler can; see C++ DR 388. The ABI-specific emission of
+ // catch handlers is expected to deal with this by rethrowing exceptions
+ // it doesn't want to catch.
Qualifiers CaughtTypeQuals;
QualType CaughtType = CGM.getContext().getUnqualifiedArrayType(
C->getCaughtType().getNonReferenceType(), CaughtTypeQuals);
@@ -1070,7 +1070,8 @@
// Initialize the catch variable and set up the cleanups.
SaveAndRestore<llvm::Instruction *> RestoreCurrentFuncletPad(
CurrentFuncletPad);
- CGM.getCXXABI().emitBeginCatch(*this, C);
+ CGM.getCXXABI().emitBeginCatch(
+ *this, C, llvm::makeArrayRef(Handlers.begin() + I, Handlers.end()));
// Emit the PGO counter increment.
incrementProfileCounter(C);
Index: lib/CodeGen/CGCleanup.h
===================================================================
--- lib/CodeGen/CGCleanup.h
+++ lib/CodeGen/CGCleanup.h
@@ -220,9 +220,13 @@
delete getHandler(I).Block;
}
- typedef const Handler *iterator;
- iterator begin() const { return getHandlers(); }
- iterator end() const { return getHandlers() + getNumHandlers(); }
+ typedef const Handler *const_iterator;
+ const_iterator begin() const { return getHandlers(); }
+ const_iterator end() const { return getHandlers() + getNumHandlers(); }
+
+ typedef Handler *iterator;
+ iterator begin() { return getHandlers(); }
+ iterator end() { return getHandlers() + getNumHandlers(); }
static bool classof(const EHScope *Scope) {
return Scope->getKind() == Catch;
Index: lib/CodeGen/CGCXXABI.h
===================================================================
--- lib/CodeGen/CGCXXABI.h
+++ lib/CodeGen/CGCXXABI.h
@@ -16,6 +16,7 @@
#define LLVM_CLANG_LIB_CODEGEN_CGCXXABI_H
#include "CodeGenFunction.h"
+#include "CGCleanup.h"
#include "clang/Basic/LLVM.h"
namespace llvm {
@@ -242,7 +243,9 @@
/// analysis.
virtual bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const = 0;
- virtual void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C) = 0;
+ virtual void
+ emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C,
+ ArrayRef<EHCatchScope::Handler> LaterHandlers) = 0;
virtual llvm::CallInst *
emitTerminateForUnexpectedException(CodeGenFunction &CGF,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
