diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 2a95d1e2736a3..0e9fcaa5fac6a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -182,6 +182,8 @@ Bug Fixes to C++ Support - Fix a crash when deleting a pointer to an incomplete array (#GH150359). - Fix an assertion failure when expression in assumption attribute (``[[assume(expr)]]``) creates temporary objects. +- Fix the dynamic_cast to final class optimization to correctly handle + casts that are guaranteed to fail (#GH137518). Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h index 96fe04661944d..2dd320dbda976 100644 --- a/clang/lib/CodeGen/CGCXXABI.h +++ b/clang/lib/CodeGen/CGCXXABI.h @@ -294,14 +294,22 @@ class CGCXXABI { Address Value, QualType SrcRecordTy) = 0; + struct ExactDynamicCastInfo { + bool RequiresCastToPrimaryBase; + CharUnits Offset; + }; + + virtual std::optional + getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy, + QualType DestRecordTy) = 0; + /// Emit a dynamic_cast from SrcRecordTy to DestRecordTy. The cast fails if /// the dynamic type of Value is not exactly DestRecordTy. - virtual llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address Value, - QualType SrcRecordTy, - QualType DestTy, - QualType DestRecordTy, - llvm::BasicBlock *CastSuccess, - llvm::BasicBlock *CastFail) = 0; + virtual llvm::Value *emitExactDynamicCast( + CodeGenFunction &CGF, Address Value, QualType SrcRecordTy, + QualType DestTy, QualType DestRecordTy, + const ExactDynamicCastInfo &CastInfo, llvm::BasicBlock *CastSuccess, + llvm::BasicBlock *CastFail) = 0; virtual bool EmitBadCastCall(CodeGenFunction &CGF) = 0; diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index fef1baf25d1d2..49d5d8acbe331 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -2295,6 +2295,18 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr, CGM.getCXXABI().shouldEmitExactDynamicCast(DestRecordTy) && !getLangOpts().PointerAuthCalls; + std::optional ExactCastInfo; + if (IsExact) { + ExactCastInfo = CGM.getCXXABI().getExactDynamicCastInfo(SrcRecordTy, DestTy, + DestRecordTy); + if (!ExactCastInfo) { + llvm::Value *NullValue = EmitDynamicCastToNull(*this, DestTy); + if (!Builder.GetInsertBlock()) + EmitBlock(createBasicBlock("dynamic_cast.unreachable")); + return NullValue; + } + } + // C++ [expr.dynamic.cast]p4: // If the value of v is a null pointer value in the pointer case, the result // is the null pointer value of type T. @@ -2322,7 +2334,8 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr, // If the destination type is effectively final, this pointer points to the // right type if and only if its vptr has the right value. Value = CGM.getCXXABI().emitExactDynamicCast( - *this, ThisAddr, SrcRecordTy, DestTy, DestRecordTy, CastEnd, CastNull); + *this, ThisAddr, SrcRecordTy, DestTy, DestRecordTy, *ExactCastInfo, + CastEnd, CastNull); } else { assert(DestRecordTy->isRecordType() && "destination type must be a record type!"); diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index aae1481444067..5ffc1edb9986f 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -226,6 +226,10 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { return hasUniqueVTablePointer(DestRecordTy); } + std::optional + getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy, + QualType DestRecordTy) override; + llvm::Value *emitDynamicCastCall(CodeGenFunction &CGF, Address Value, QualType SrcRecordTy, QualType DestTy, QualType DestRecordTy, @@ -234,6 +238,7 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address ThisAddr, QualType SrcRecordTy, QualType DestTy, QualType DestRecordTy, + const ExactDynamicCastInfo &CastInfo, llvm::BasicBlock *CastSuccess, llvm::BasicBlock *CastFail) override; @@ -1681,10 +1686,11 @@ llvm::Value *ItaniumCXXABI::emitDynamicCastCall( return Value; } -llvm::Value *ItaniumCXXABI::emitExactDynamicCast( - CodeGenFunction &CGF, Address ThisAddr, QualType SrcRecordTy, - QualType DestTy, QualType DestRecordTy, llvm::BasicBlock *CastSuccess, - llvm::BasicBlock *CastFail) { +std::optional +ItaniumCXXABI::getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy, + QualType DestRecordTy) { + assert(shouldEmitExactDynamicCast(DestRecordTy)); + ASTContext &Context = getContext(); // Find all the inheritance paths. @@ -1722,41 +1728,56 @@ llvm::Value *ItaniumCXXABI::emitExactDynamicCast( if (!Offset) Offset = PathOffset; else if (Offset != PathOffset) { - // Base appears in at least two different places. Find the most-derived - // object and see if it's a DestDecl. Note that the most-derived object - // must be at least as aligned as this base class subobject, and must - // have a vptr at offset 0. - ThisAddr = Address(emitDynamicCastToVoid(CGF, ThisAddr, SrcRecordTy), - CGF.VoidPtrTy, ThisAddr.getAlignment()); - SrcDecl = DestDecl; - Offset = CharUnits::Zero(); - break; + // Base appears in at least two different places. + return ExactDynamicCastInfo{/*RequiresCastToPrimaryBase=*/true, + CharUnits::Zero()}; } } + if (!Offset) + return std::nullopt; + return ExactDynamicCastInfo{/*RequiresCastToPrimaryBase=*/false, *Offset}; +} - if (!Offset) { - // If there are no public inheritance paths, the cast always fails. - CGF.EmitBranch(CastFail); - return llvm::PoisonValue::get(CGF.VoidPtrTy); - } +llvm::Value *ItaniumCXXABI::emitExactDynamicCast( + CodeGenFunction &CGF, Address ThisAddr, QualType SrcRecordTy, + QualType DestTy, QualType DestRecordTy, + const ExactDynamicCastInfo &ExactCastInfo, llvm::BasicBlock *CastSuccess, + llvm::BasicBlock *CastFail) { + const CXXRecordDecl *SrcDecl = SrcRecordTy->getAsCXXRecordDecl(); + const CXXRecordDecl *DestDecl = DestRecordTy->getAsCXXRecordDecl(); + + llvm::Value *VTable = nullptr; + if (ExactCastInfo.RequiresCastToPrimaryBase) { + // Base appears in at least two different places. Find the most-derived + // object and see if it's a DestDecl. Note that the most-derived object + // must be at least as aligned as this base class subobject, and must + // have a vptr at offset 0. + llvm::Value *PrimaryBase = + emitDynamicCastToVoid(CGF, ThisAddr, SrcRecordTy); + ThisAddr = Address(PrimaryBase, CGF.VoidPtrTy, ThisAddr.getAlignment()); + SrcDecl = DestDecl; + Address VTablePtrPtr = ThisAddr.withElementType(CGF.VoidPtrPtrTy); + VTable = CGF.Builder.CreateLoad(VTablePtrPtr, "vtable"); + } else + VTable = CGF.GetVTablePtr(ThisAddr, CGF.UnqualPtrTy, SrcDecl); // Compare the vptr against the expected vptr for the destination type at - // this offset. Note that we do not know what type ThisAddr points to in - // the case where the derived class multiply inherits from the base class - // so we can't use GetVTablePtr, so we load the vptr directly instead. - llvm::Instruction *VPtr = CGF.Builder.CreateLoad( - ThisAddr.withElementType(CGF.VoidPtrPtrTy), "vtable"); - CGM.DecorateInstructionWithTBAA( - VPtr, CGM.getTBAAVTablePtrAccessInfo(CGF.VoidPtrPtrTy)); - llvm::Value *Success = CGF.Builder.CreateICmpEQ( - VPtr, getVTableAddressPoint(BaseSubobject(SrcDecl, *Offset), DestDecl)); - llvm::Value *Result = ThisAddr.emitRawPointer(CGF); - if (!Offset->isZero()) - Result = CGF.Builder.CreateInBoundsGEP( - CGF.CharTy, Result, - {llvm::ConstantInt::get(CGF.PtrDiffTy, -Offset->getQuantity())}); + // this offset. + llvm::Constant *ExpectedVTable = getVTableAddressPoint( + BaseSubobject(SrcDecl, ExactCastInfo.Offset), DestDecl); + llvm::Value *Success = CGF.Builder.CreateICmpEQ(VTable, ExpectedVTable); + llvm::Value *AdjustedThisPtr = ThisAddr.emitRawPointer(CGF); + + if (!ExactCastInfo.Offset.isZero()) { + CharUnits::QuantityType Offset = ExactCastInfo.Offset.getQuantity(); + llvm::Constant *OffsetConstant = + llvm::ConstantInt::get(CGF.PtrDiffTy, -Offset); + AdjustedThisPtr = CGF.Builder.CreateInBoundsGEP(CGF.CharTy, AdjustedThisPtr, + OffsetConstant); + } + CGF.Builder.CreateCondBr(Success, CastSuccess, CastFail); - return Result; + return AdjustedThisPtr; } llvm::Value *ItaniumCXXABI::emitDynamicCastToVoid(CodeGenFunction &CGF, diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 700ffa4beffc1..e8d2451a7dd5c 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -158,9 +158,15 @@ class MicrosoftCXXABI : public CGCXXABI { // TODO: Add support for exact dynamic_casts. return false; } + std::optional + getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy, + QualType DestRecordTy) override { + llvm_unreachable("unsupported"); + } llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address Value, QualType SrcRecordTy, QualType DestTy, QualType DestRecordTy, + const ExactDynamicCastInfo &CastInfo, llvm::BasicBlock *CastSuccess, llvm::BasicBlock *CastFail) override { llvm_unreachable("unsupported"); diff --git a/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp b/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp index 19c2a9bd0497e..3156e1bb21b1e 100644 --- a/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp +++ b/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp @@ -14,3 +14,19 @@ B *exact(A *a) { // EXACT-NOT: call {{.*}} @__dynamic_cast return dynamic_cast(a); } + +struct C { + virtual ~C(); +}; + +struct D final : private C { + +}; + +// CHECK-LABEL: @_Z5exactP1C +D *exact(C *a) { + // INEXACT: call {{.*}} @__dynamic_cast + // EXACT: entry: + // EXACT-NEXT: ret ptr null + return dynamic_cast(a); +} diff --git a/clang/test/CodeGenCXX/dynamic-cast-exact.cpp b/clang/test/CodeGenCXX/dynamic-cast-exact.cpp index 86e1965b4ba68..588d80844a2fa 100644 --- a/clang/test/CodeGenCXX/dynamic-cast-exact.cpp +++ b/clang/test/CodeGenCXX/dynamic-cast-exact.cpp @@ -9,6 +9,7 @@ struct E : A { int e; }; struct F : virtual A { int f; }; struct G : virtual A { int g; }; struct H final : C, D, E, F, G { int h; }; +struct H1 final: C, private D { int h1; }; // CHECK-LABEL: @_Z7inexactP1A C *inexact(A *a) { @@ -77,10 +78,49 @@ H *exact_multi(A *a) { return dynamic_cast(a); } +// CHECK-LABEL: @_Z19exact_invalid_multiP1D +H1 *exact_invalid_multi(D* d) { + // CHECK: entry: + // CHECK-NEXT: %d.addr = alloca ptr + // CHECK-NEXT: store ptr %d, ptr %d.addr + // CHECK-NEXT: load ptr, ptr %d.addr + // CHECK-NEXT: ret ptr null + return dynamic_cast(d); +} + +// CHECK-LABEL: @_Z19exact_invalid_multiR1D +H1 &exact_invalid_multi(D& d) { + // CHECK: entry: + // CHECK-NEXT: %d.addr = alloca ptr + // CHECK-NEXT: store ptr %d, ptr %d.addr + // CHECK-NEXT: load ptr, ptr %d.addr + // CHECK-NEXT: call void @__cxa_bad_cast() + // CHECK-NEXT: unreachable + // CHECK: dynamic_cast.unreachable: + // CHECK-NEXT: ret ptr poison + return dynamic_cast(d); +} + +namespace GH137518 { + class base { virtual void fn() = 0; }; + class test final : base { virtual void fn() { } }; + test* new_test() { return new test(); } + + // CHECK-LABEL: @_ZN8GH1375184castEPNS_4baseE( + test* cast(base* b) { + // CHECK: entry: + // CHECK-NEXT: %b.addr = alloca ptr + // CHECK-NEXT: store ptr %b, ptr %b.addr + // CHECK-NEXT: load ptr, ptr %b.addr + // CHECK-NEXT: ret ptr null + return dynamic_cast(b); + } +} + namespace GH64088 { // Ensure we mark the B vtable as used here, because we're going to emit a // reference to it. - // CHECK: define {{.*}} @_ZN7GH640881BD0 + // CHECK: define {{.*}} void @_ZN7GH640881BD0Ev( struct A { virtual ~A(); }; struct B final : A { virtual ~B() = default; }; B *cast(A *p) { return dynamic_cast(p); }