Skip to content

Commit f04f6c3

Browse files
committed
[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. There is one place where we still lose a fully authenticated path, and that is if there multiple paths from the source type to the destination type. In that case we're forced to perform a dynamic_cast to void* to find the primary base. As we do not know the primary base at this point we do not yet know the dynamic type of the adjusted this object and so cannot authenticate the vtable load. The approach this PR takes to mitigate this gap is to authenticate the vtable of the original object, and then if the stripped vtable pointer matches the expected type we then know the type of the object and so perform a fully authenticated load of the vtable from the resulting object. Fixes #137518
1 parent 9f7f3d6 commit f04f6c3

File tree

8 files changed

+315
-43
lines changed

8 files changed

+315
-43
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ Bug Fixes to C++ Support
160160
- Fix a crash when deleting a pointer to an incomplete array (#GH150359).
161161
- Fix an assertion failure when expression in assumption attribute
162162
(``[[assume(expr)]]``) creates temporary objects.
163+
- Fix the dynamic_cast to final class optimization to correctly handle
164+
casts that are guaranteed to fail, and correctly handle ptrauth protected
165+
vtable pointers (#GH137518).
163166

164167
Bug Fixes to AST Handling
165168
^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/CodeGen/CGCXXABI.h

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,14 +294,22 @@ class CGCXXABI {
294294
Address Value,
295295
QualType SrcRecordTy) = 0;
296296

297+
struct ExactDynamicCastInfo {
298+
bool RequiresCastToPrimaryBase;
299+
CharUnits Offset;
300+
};
301+
302+
virtual std::optional<ExactDynamicCastInfo>
303+
getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy,
304+
QualType DestRecordTy) = 0;
305+
297306
/// Emit a dynamic_cast from SrcRecordTy to DestRecordTy. The cast fails if
298307
/// the dynamic type of Value is not exactly DestRecordTy.
299-
virtual llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address Value,
300-
QualType SrcRecordTy,
301-
QualType DestTy,
302-
QualType DestRecordTy,
303-
llvm::BasicBlock *CastSuccess,
304-
llvm::BasicBlock *CastFail) = 0;
308+
virtual llvm::Value *emitExactDynamicCast(
309+
CodeGenFunction &CGF, Address Value, QualType SrcRecordTy,
310+
QualType DestTy, QualType DestRecordTy,
311+
const ExactDynamicCastInfo &CastInfo, llvm::BasicBlock *CastSuccess,
312+
llvm::BasicBlock *CastFail) = 0;
305313

306314
virtual bool EmitBadCastCall(CodeGenFunction &CGF) = 0;
307315

clang/lib/CodeGen/CGExprCXX.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2294,6 +2294,18 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr,
22942294
DestRecordTy->getAsCXXRecordDecl()->isEffectivelyFinal() &&
22952295
CGM.getCXXABI().shouldEmitExactDynamicCast(DestRecordTy);
22962296

2297+
std::optional<CGCXXABI::ExactDynamicCastInfo> ExactCastInfo;
2298+
if (IsExact) {
2299+
ExactCastInfo = CGM.getCXXABI().getExactDynamicCastInfo(SrcRecordTy, DestTy,
2300+
DestRecordTy);
2301+
if (!ExactCastInfo) {
2302+
llvm::Value *NullValue = EmitDynamicCastToNull(*this, DestTy);
2303+
if (!Builder.GetInsertBlock())
2304+
EmitBlock(createBasicBlock("dynamic_cast.always_fails"));
2305+
return NullValue;
2306+
}
2307+
}
2308+
22972309
// C++ [expr.dynamic.cast]p4:
22982310
// If the value of v is a null pointer value in the pointer case, the result
22992311
// is the null pointer value of type T.
@@ -2321,7 +2333,8 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr,
23212333
// If the destination type is effectively final, this pointer points to the
23222334
// right type if and only if its vptr has the right value.
23232335
Value = CGM.getCXXABI().emitExactDynamicCast(
2324-
*this, ThisAddr, SrcRecordTy, DestTy, DestRecordTy, CastEnd, CastNull);
2336+
*this, ThisAddr, SrcRecordTy, DestTy, DestRecordTy, *ExactCastInfo,
2337+
CastEnd, CastNull);
23252338
} else {
23262339
assert(DestRecordTy->isRecordType() &&
23272340
"destination type must be a record type!");
@@ -2342,12 +2355,10 @@ llvm::Value *CodeGenFunction::EmitDynamicCast(Address ThisAddr,
23422355
}
23432356

23442357
EmitBlock(CastEnd);
2345-
23462358
if (CastNull) {
23472359
llvm::PHINode *PHI = Builder.CreatePHI(Value->getType(), 2);
23482360
PHI->addIncoming(Value, CastNotNull);
23492361
PHI->addIncoming(NullValue, CastNull);
2350-
23512362
Value = PHI;
23522363
}
23532364

clang/lib/CodeGen/ItaniumCXXABI.cpp

Lines changed: 93 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI {
226226
return hasUniqueVTablePointer(DestRecordTy);
227227
}
228228

229+
std::optional<ExactDynamicCastInfo>
230+
getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy,
231+
QualType DestRecordTy) override;
232+
229233
llvm::Value *emitDynamicCastCall(CodeGenFunction &CGF, Address Value,
230234
QualType SrcRecordTy, QualType DestTy,
231235
QualType DestRecordTy,
@@ -234,6 +238,7 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI {
234238
llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address ThisAddr,
235239
QualType SrcRecordTy, QualType DestTy,
236240
QualType DestRecordTy,
241+
const ExactDynamicCastInfo &CastInfo,
237242
llvm::BasicBlock *CastSuccess,
238243
llvm::BasicBlock *CastFail) override;
239244

@@ -1681,10 +1686,11 @@ llvm::Value *ItaniumCXXABI::emitDynamicCastCall(
16811686
return Value;
16821687
}
16831688

1684-
llvm::Value *ItaniumCXXABI::emitExactDynamicCast(
1685-
CodeGenFunction &CGF, Address ThisAddr, QualType SrcRecordTy,
1686-
QualType DestTy, QualType DestRecordTy, llvm::BasicBlock *CastSuccess,
1687-
llvm::BasicBlock *CastFail) {
1689+
std::optional<CGCXXABI::ExactDynamicCastInfo>
1690+
ItaniumCXXABI::getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy,
1691+
QualType DestRecordTy) {
1692+
assert(shouldEmitExactDynamicCast(DestRecordTy));
1693+
16881694
ASTContext &Context = getContext();
16891695

16901696
// Find all the inheritance paths.
@@ -1722,41 +1728,95 @@ llvm::Value *ItaniumCXXABI::emitExactDynamicCast(
17221728
if (!Offset)
17231729
Offset = PathOffset;
17241730
else if (Offset != PathOffset) {
1725-
// Base appears in at least two different places. Find the most-derived
1726-
// object and see if it's a DestDecl. Note that the most-derived object
1727-
// must be at least as aligned as this base class subobject, and must
1728-
// have a vptr at offset 0.
1729-
ThisAddr = Address(emitDynamicCastToVoid(CGF, ThisAddr, SrcRecordTy),
1730-
CGF.VoidPtrTy, ThisAddr.getAlignment());
1731-
SrcDecl = DestDecl;
1732-
Offset = CharUnits::Zero();
1733-
break;
1731+
// Base appears in at least two different places.
1732+
return ExactDynamicCastInfo{/*RequiresCastToPrimaryBase=*/true,
1733+
CharUnits::Zero()};
17341734
}
17351735
}
1736+
if (!Offset)
1737+
return std::nullopt;
1738+
return ExactDynamicCastInfo{/*RequiresCastToPrimaryBase=*/false, *Offset};
1739+
}
17361740

1737-
if (!Offset) {
1738-
// If there are no public inheritance paths, the cast always fails.
1739-
CGF.EmitBranch(CastFail);
1740-
return llvm::PoisonValue::get(CGF.VoidPtrTy);
1741-
}
1741+
llvm::Value *ItaniumCXXABI::emitExactDynamicCast(
1742+
CodeGenFunction &CGF, Address ThisAddr, QualType SrcRecordTy,
1743+
QualType DestTy, QualType DestRecordTy,
1744+
const ExactDynamicCastInfo &ExactCastInfo, llvm::BasicBlock *CastSuccess,
1745+
llvm::BasicBlock *CastFail) {
1746+
const CXXRecordDecl *SrcDecl = SrcRecordTy->getAsCXXRecordDecl();
1747+
const CXXRecordDecl *DestDecl = DestRecordTy->getAsCXXRecordDecl();
1748+
auto AuthenticateVTable = [&](Address ThisAddr, const CXXRecordDecl *Decl) {
1749+
if (!CGF.getLangOpts().PointerAuthCalls)
1750+
return;
1751+
(void)CGF.GetVTablePtr(ThisAddr, CGF.UnqualPtrTy, Decl,
1752+
CodeGenFunction::VTableAuthMode::MustTrap);
1753+
};
1754+
1755+
bool PerformPostCastAuthentication = false;
1756+
llvm::Value *VTable = nullptr;
1757+
if (ExactCastInfo.RequiresCastToPrimaryBase) {
1758+
// Base appears in at least two different places. Find the most-derived
1759+
// object and see if it's a DestDecl. Note that the most-derived object
1760+
// must be at least as aligned as this base class subobject, and must
1761+
// have a vptr at offset 0.
1762+
llvm::Value *PrimaryBase =
1763+
emitDynamicCastToVoid(CGF, ThisAddr, SrcRecordTy);
1764+
ThisAddr = Address(PrimaryBase, CGF.VoidPtrTy, ThisAddr.getAlignment());
1765+
SrcDecl = DestDecl;
1766+
// This unauthenticated load is unavoidable, so we're relying on the
1767+
// authenticated load in the dynamic cast to void, and we'll manually
1768+
// authenticate the resulting v-table at the end of the cast check.
1769+
PerformPostCastAuthentication = CGF.getLangOpts().PointerAuthCalls;
1770+
CGPointerAuthInfo StrippingAuthInfo(0, PointerAuthenticationMode::Strip,
1771+
false, false, nullptr);
1772+
Address VTablePtrPtr = ThisAddr.withElementType(CGF.VoidPtrPtrTy);
1773+
VTable = CGF.Builder.CreateLoad(VTablePtrPtr, "vtable");
1774+
if (PerformPostCastAuthentication)
1775+
VTable = CGF.EmitPointerAuthAuth(StrippingAuthInfo, VTable);
1776+
} else
1777+
VTable = CGF.GetVTablePtr(ThisAddr, CGF.UnqualPtrTy, SrcDecl);
17421778

17431779
// Compare the vptr against the expected vptr for the destination type at
1744-
// this offset. Note that we do not know what type ThisAddr points to in
1745-
// the case where the derived class multiply inherits from the base class
1746-
// so we can't use GetVTablePtr, so we load the vptr directly instead.
1747-
llvm::Instruction *VPtr = CGF.Builder.CreateLoad(
1748-
ThisAddr.withElementType(CGF.VoidPtrPtrTy), "vtable");
1749-
CGM.DecorateInstructionWithTBAA(
1750-
VPtr, CGM.getTBAAVTablePtrAccessInfo(CGF.VoidPtrPtrTy));
1751-
llvm::Value *Success = CGF.Builder.CreateICmpEQ(
1752-
VPtr, getVTableAddressPoint(BaseSubobject(SrcDecl, *Offset), DestDecl));
1753-
llvm::Value *Result = ThisAddr.emitRawPointer(CGF);
1754-
if (!Offset->isZero())
1755-
Result = CGF.Builder.CreateInBoundsGEP(
1756-
CGF.CharTy, Result,
1757-
{llvm::ConstantInt::get(CGF.PtrDiffTy, -Offset->getQuantity())});
1780+
// this offset.
1781+
llvm::Constant *ExpectedVTable = getVTableAddressPoint(
1782+
BaseSubobject(SrcDecl, ExactCastInfo.Offset), DestDecl);
1783+
llvm::Value *Success = CGF.Builder.CreateICmpEQ(VTable, ExpectedVTable);
1784+
llvm::Value *AdjustedThisPtr = ThisAddr.emitRawPointer(CGF);
1785+
1786+
if (!ExactCastInfo.Offset.isZero()) {
1787+
CharUnits::QuantityType Offset = ExactCastInfo.Offset.getQuantity();
1788+
llvm::Constant *OffsetConstant =
1789+
llvm::ConstantInt::get(CGF.PtrDiffTy, -Offset);
1790+
AdjustedThisPtr = CGF.Builder.CreateInBoundsGEP(CGF.CharTy, AdjustedThisPtr,
1791+
OffsetConstant);
1792+
PerformPostCastAuthentication = CGF.getLangOpts().PointerAuthCalls;
1793+
}
1794+
1795+
if (PerformPostCastAuthentication) {
1796+
// If we've changed the object pointer we authenticate the vtable pointer
1797+
// of the resulting object.
1798+
llvm::BasicBlock *NonNullBlock = CGF.Builder.GetInsertBlock();
1799+
llvm::BasicBlock *PostCastAuthSuccess =
1800+
CGF.createBasicBlock("dynamic_cast.postauth.success");
1801+
llvm::BasicBlock *PostCastAuthComplete =
1802+
CGF.createBasicBlock("dynamic_cast.postauth.complete");
1803+
CGF.Builder.CreateCondBr(Success, PostCastAuthSuccess,
1804+
PostCastAuthComplete);
1805+
CGF.EmitBlock(PostCastAuthSuccess);
1806+
Address AdjustedThisAddr =
1807+
Address(AdjustedThisPtr, CGF.IntPtrTy, CGF.getPointerAlign());
1808+
AuthenticateVTable(AdjustedThisAddr, DestDecl);
1809+
CGF.EmitBranch(PostCastAuthComplete);
1810+
CGF.EmitBlock(PostCastAuthComplete);
1811+
llvm::PHINode *PHI = CGF.Builder.CreatePHI(AdjustedThisPtr->getType(), 2);
1812+
PHI->addIncoming(AdjustedThisPtr, PostCastAuthSuccess);
1813+
llvm::Value *NullValue =
1814+
llvm::Constant::getNullValue(AdjustedThisPtr->getType());
1815+
PHI->addIncoming(NullValue, NonNullBlock);
1816+
AdjustedThisPtr = PHI;
1817+
}
17581818
CGF.Builder.CreateCondBr(Success, CastSuccess, CastFail);
1759-
return Result;
1819+
return AdjustedThisPtr;
17601820
}
17611821

17621822
llvm::Value *ItaniumCXXABI::emitDynamicCastToVoid(CodeGenFunction &CGF,

clang/lib/CodeGen/MicrosoftCXXABI.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,15 @@ class MicrosoftCXXABI : public CGCXXABI {
158158
// TODO: Add support for exact dynamic_casts.
159159
return false;
160160
}
161+
std::optional<ExactDynamicCastInfo>
162+
getExactDynamicCastInfo(QualType SrcRecordTy, QualType DestTy,
163+
QualType DestRecordTy) override {
164+
llvm_unreachable("unsupported");
165+
}
161166
llvm::Value *emitExactDynamicCast(CodeGenFunction &CGF, Address Value,
162167
QualType SrcRecordTy, QualType DestTy,
163168
QualType DestRecordTy,
169+
const ExactDynamicCastInfo &CastInfo,
164170
llvm::BasicBlock *CastSuccess,
165171
llvm::BasicBlock *CastFail) override {
166172
llvm_unreachable("unsupported");

clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,19 @@ B *exact(A *a) {
1313
// EXACT-NOT: call {{.*}} @__dynamic_cast
1414
return dynamic_cast<B*>(a);
1515
}
16+
17+
struct C {
18+
virtual ~C();
19+
};
20+
21+
struct D final : private C {
22+
23+
};
24+
25+
// CHECK-LABEL: @_Z5exactP1C
26+
D *exact(C *a) {
27+
// INEXACT: call {{.*}} @__dynamic_cast
28+
// EXACT: entry:
29+
// EXACT-NEXT: ret ptr null
30+
return dynamic_cast<D*>(a);
31+
}

clang/test/CodeGenCXX/dynamic-cast-exact.cpp

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ struct E : A { int e; };
99
struct F : virtual A { int f; };
1010
struct G : virtual A { int g; };
1111
struct H final : C, D, E, F, G { int h; };
12+
struct H1 final: C, private D { int h1; };
1213

1314
// CHECK-LABEL: @_Z7inexactP1A
1415
C *inexact(A *a) {
@@ -77,10 +78,49 @@ H *exact_multi(A *a) {
7778
return dynamic_cast<H*>(a);
7879
}
7980

81+
// CHECK-LABEL: @_Z19exact_invalid_multiP1D
82+
H1 *exact_invalid_multi(D* d) {
83+
// CHECK: entry:
84+
// CHECK-NEXT: %d.addr = alloca ptr
85+
// CHECK-NEXT: store ptr %d, ptr %d.addr
86+
// CHECK-NEXT: load ptr, ptr %d.addr
87+
// CHECK-NEXT: ret ptr null
88+
return dynamic_cast<H1*>(d);
89+
}
90+
91+
// CHECK-LABEL: @_Z19exact_invalid_multiR1D
92+
H1 &exact_invalid_multi(D& d) {
93+
// CHECK: entry:
94+
// CHECK-NEXT: %d.addr = alloca ptr
95+
// CHECK-NEXT: store ptr %d, ptr %d.addr
96+
// CHECK-NEXT: load ptr, ptr %d.addr
97+
// CHECK-NEXT: call void @__cxa_bad_cast()
98+
// CHECK-NEXT: unreachable
99+
// CHECK: dynamic_cast.always_fails:
100+
// CHECK-NEXT: ret ptr poison
101+
return dynamic_cast<H1&>(d);
102+
}
103+
104+
namespace GH137518 {
105+
class base { virtual void fn() = 0; };
106+
class test final : base { virtual void fn() { } };
107+
test* new_test() { return new test(); }
108+
109+
// CHECK-LABEL: @_ZN8GH1375184castEPNS_4baseE(
110+
test* cast(base* b) {
111+
// CHECK: entry:
112+
// CHECK-NEXT: %b.addr = alloca ptr
113+
// CHECK-NEXT: store ptr %b, ptr %b.addr
114+
// CHECK-NEXT: load ptr, ptr %b.addr
115+
// CHECK-NEXT: ret ptr null
116+
return dynamic_cast<test*>(b);
117+
}
118+
}
119+
80120
namespace GH64088 {
81121
// Ensure we mark the B vtable as used here, because we're going to emit a
82122
// reference to it.
83-
// CHECK: define {{.*}} @_ZN7GH640881BD0
123+
// CHECK: define {{.*}} void @_ZN7GH640881BD0Ev(
84124
struct A { virtual ~A(); };
85125
struct B final : A { virtual ~B() = default; };
86126
B *cast(A *p) { return dynamic_cast<B*>(p); }

0 commit comments

Comments
 (0)