From c4c23ff5ee6f6ec165cd2511c061a270d7ff3af7 Mon Sep 17 00:00:00 2001 From: Oliver Hunt Date: Mon, 4 Aug 2025 20:54:46 -0700 Subject: [PATCH] [clang] Fix crash in dynamic_cast final class optimization This corrects the codegen for the final class optimization to correct handle the case where there is no path to perform the cast, and also corrects the codegen to handle ptrauth protected vtable pointers. As part of this fix we separate out the path computation as that makes it easier to reason about the failure code paths and more importantly means we can know what the type of the this object is during the cast. The allows us to use the GetVTablePointer interface which correctly performs the authentication operations required when pointer authentication is enabled. This still leaves incorrect authentication behavior in the multiple inheritance case but currently the optimization is disabled entirely if pointer authentication is enabled. Fixes #137518 --- clang/docs/ReleaseNotes.rst | 2 + clang/lib/CodeGen/CGCXXABI.h | 20 +++-- clang/lib/CodeGen/CGExprCXX.cpp | 15 +++- clang/lib/CodeGen/ItaniumCXXABI.cpp | 87 ++++++++++++------- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 6 ++ .../dynamic-cast-exact-disabled.cpp | 16 ++++ clang/test/CodeGenCXX/dynamic-cast-exact.cpp | 42 ++++++++- 7 files changed, 147 insertions(+), 41 deletions(-) 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); }