From b764685e6b74da439523f455550c4661f025d616 Mon Sep 17 00:00:00 2001 From: Oliver Hunt Date: Thu, 10 Jul 2025 16:40:12 -0700 Subject: [PATCH] [clang] Fix #137518 The verification pass correctly identifies that the phi node produced for an exact dynamic_cast that always fails. Rather than trying to construct pretend that we have pass and fail paths through an invalid exact dynamic_cast we handle the failure path explicitly. --- clang/lib/CodeGen/CGCXXABI.h | 11 +++++------ clang/lib/CodeGen/CGExprCXX.cpp | 13 ++++++++++--- clang/lib/CodeGen/ItaniumCXXABI.cpp | 15 +++++++-------- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 10 +++++----- .../CodeGenCXX/dynamic-cast-exact-disabled.cpp | 16 ++++++++++++++++ clang/test/CodeGenCXX/dynamic-cast-exact.cpp | 18 ++++++++++++++++++ 6 files changed, 61 insertions(+), 22 deletions(-) diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h index 96fe04661944d..80aaacb56d436 100644 --- a/clang/lib/CodeGen/CGCXXABI.h +++ b/clang/lib/CodeGen/CGCXXABI.h @@ -296,12 +296,11 @@ class CGCXXABI { /// 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 std::optional + emitExactDynamicCast(CodeGenFunction &CGF, Address Value, + QualType SrcRecordTy, QualType DestTy, + QualType DestRecordTy, 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 359e30cb8f5cd..6ac3e27299e51 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -2341,8 +2341,15 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr, } else if (IsExact) { // 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); + std::optional ExactCast = + CGM.getCXXABI().emitExactDynamicCast(*this, ThisAddr, SrcRecordTy, + DestTy, DestRecordTy, CastEnd, + CastNull); + if (!ExactCast) { + Value = llvm::Constant::getNullValue(VoidPtrTy); + EmitBranch(CastNull); + } else + Value = *ExactCast; } else { assert(DestRecordTy->isRecordType() && "destination type must be a record type!"); @@ -2364,7 +2371,7 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr, EmitBlock(CastEnd); - if (CastNull) { + if (CastNull && CastNotNull) { llvm::PHINode *PHI = Builder.CreatePHI(Value->getType(), 2); PHI->addIncoming(Value, CastNotNull); PHI->addIncoming(NullValue, CastNull); diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index f4a99467010af..73d188f6ab998 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -231,11 +231,11 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { QualType DestRecordTy, llvm::BasicBlock *CastEnd) override; - llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address ThisAddr, - QualType SrcRecordTy, QualType DestTy, - QualType DestRecordTy, - llvm::BasicBlock *CastSuccess, - llvm::BasicBlock *CastFail) override; + std::optional + emitExactDynamicCast(CodeGenFunction &CGF, Address ThisAddr, + QualType SrcRecordTy, QualType DestTy, + QualType DestRecordTy, llvm::BasicBlock *CastSuccess, + llvm::BasicBlock *CastFail) override; llvm::Value *emitDynamicCastToVoid(CodeGenFunction &CGF, Address Value, QualType SrcRecordTy) override; @@ -1681,7 +1681,7 @@ llvm::Value *ItaniumCXXABI::emitDynamicCastCall( return Value; } -llvm::Value *ItaniumCXXABI::emitExactDynamicCast( +std::optional ItaniumCXXABI::emitExactDynamicCast( CodeGenFunction &CGF, Address ThisAddr, QualType SrcRecordTy, QualType DestTy, QualType DestRecordTy, llvm::BasicBlock *CastSuccess, llvm::BasicBlock *CastFail) { @@ -1736,8 +1736,7 @@ llvm::Value *ItaniumCXXABI::emitExactDynamicCast( if (!Offset) { // If there are no public inheritance paths, the cast always fails. - CGF.EmitBranch(CastFail); - return llvm::PoisonValue::get(CGF.VoidPtrTy); + return std::nullopt; } // Compare the vptr against the expected vptr for the destination type at diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index a181559834296..133a2ee1e2433 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -158,11 +158,11 @@ class MicrosoftCXXABI : public CGCXXABI { // TODO: Add support for exact dynamic_casts. return false; } - llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address Value, - QualType SrcRecordTy, QualType DestTy, - QualType DestRecordTy, - llvm::BasicBlock *CastSuccess, - llvm::BasicBlock *CastFail) override { + std::optional + emitExactDynamicCast(CodeGenFunction &CGF, Address Value, + QualType SrcRecordTy, QualType DestTy, + QualType DestRecordTy, 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 9a8ce1997a7f9..bf202d14c3398 100644 --- a/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp +++ b/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp @@ -13,3 +13,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..5fdb2f7eb3d75 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,6 +78,23 @@ H *exact_multi(A *a) { return dynamic_cast(a); } +// CHECK-LABEL: @_Z19exact_invalid_multiP1D +H1 *exact_invalid_multi(D* d) { + // CHECK: dynamic_cast.end: + // CHECK-NEXT: ret ptr null + return dynamic_cast(d); +} + +// CHECK-LABEL: @_Z19exact_invalid_multiR1D +H1 &exact_invalid_multi(D& d) { + // CHECK: dynamic_cast.notnull: + // CHECK-NEXT: br label %dynamic_cast.null + // CHECK: dynamic_cast.null: + // CHECK-NEXT: call void @__cxa_bad_cast() + // CHECK-NEXT: unreachable + return dynamic_cast(d); +} + namespace GH64088 { // Ensure we mark the B vtable as used here, because we're going to emit a // reference to it.