Index: clang/include/clang/AST/DeclCXX.h =================================================================== --- clang/include/clang/AST/DeclCXX.h +++ clang/include/clang/AST/DeclCXX.h @@ -775,6 +775,14 @@ return data().Polymorphic || data().NumVBases != 0; } + bool mayBeDynamicClass() const { + return !hasDefinition() || isDynamicClass() || hasAnyDependentBases(); + } + + bool mayBeNotDynamicClass() const { + return !hasDefinition() || !isDynamicClass() || hasAnyDependentBases(); + } + void setIsParsingBaseSpecifiers() { data().IsParsingBaseSpecifiers = true; } bool isParsingBaseSpecifiers() const { Index: clang/lib/CodeGen/CGExpr.cpp =================================================================== --- clang/lib/CodeGen/CGExpr.cpp +++ clang/lib/CodeGen/CGExpr.cpp @@ -3849,6 +3849,18 @@ } Address addr = base.getAddress(); + if (auto *ClassDef = dyn_cast(rec)) { + if (CGM.getCodeGenOpts().StrictVTablePointers && + ClassDef->isDynamicClass()) { + // Getting to any field of dynamic object requires stripping dynamic + // information provided by invariant.group. This is because accessing + // fields may leak the real address of dynamic object, which could result + // in miscompilation when leaked pointer would be compared. + auto *stripped = Builder.CreateStripInvariantGroup(addr.getPointer()); + addr = Address(stripped, addr.getAlignment()); + } + } + unsigned RecordCVR = base.getVRQualifiers(); if (rec->isUnion()) { // For unions, there is no pointer adjustment. Index: clang/lib/CodeGen/CGExprScalar.cpp =================================================================== --- clang/lib/CodeGen/CGExprScalar.cpp +++ clang/lib/CodeGen/CGExprScalar.cpp @@ -1618,6 +1618,34 @@ CE->getLocStart()); } + if (CGF.CGM.getCodeGenOpts().StrictVTablePointers) { + const CXXRecordDecl *SourceClassDecl = + E->getType().getTypePtr()->getPointeeCXXRecordDecl(); + const CXXRecordDecl *DstClassDecl = DestTy->getPointeeCXXRecordDecl(); + + bool SourceMayBeDynamic = + SourceClassDecl && SourceClassDecl->mayBeDynamicClass(); + bool SourceMayBeNotDynamic = + !SourceClassDecl || SourceClassDecl->mayBeNotDynamicClass(); + + bool DstMayBeDynamic = DstClassDecl && DstClassDecl->mayBeDynamicClass(); + bool DstMayNotBeDynamic = + !DstClassDecl || DstClassDecl->mayBeNotDynamicClass(); + if (SourceMayBeNotDynamic && DstMayBeDynamic) { + // Casting to pointer that could carry dynamic information (provided by + // invariant.group) requires launder. + Src = Builder.CreateLaunderInvariantGroup(Src); + } else if (SourceMayBeDynamic && DstMayNotBeDynamic) { + // Casting to pointer that does not carry dynamic information (provided + // by invariant.group) requires stripping it. Note that we don't do it + // if the source could not be dynamic type and destination could be + // dynamic because dynamic information is already laundered. It is + // because launder(strip(src)) == launder(src), so there is no need to + // add extra strip before launder. + Src = Builder.CreateStripInvariantGroup(Src); + } + } + return Builder.CreateBitCast(Src, DstTy); } case CK_AddressSpaceConversion: { @@ -1754,12 +1782,32 @@ llvm::Value* IntResult = Builder.CreateIntCast(Src, MiddleTy, InputSigned, "conv"); - return Builder.CreateIntToPtr(IntResult, DestLLVMTy); + auto *IntToPtr = Builder.CreateIntToPtr(IntResult, DestLLVMTy); + + if (CGF.CGM.getCodeGenOpts().StrictVTablePointers) { + auto *DstClassDecl = DestTy->getPointeeCXXRecordDecl(); + // Going from integer to pointer that could be dynamic requires reloading + // dynamic information from invariant.group. + if (DstClassDecl && DstClassDecl->mayBeDynamicClass()) + IntToPtr = Builder.CreateLaunderInvariantGroup(IntToPtr); + } + return IntToPtr; } - case CK_PointerToIntegral: + case CK_PointerToIntegral: { assert(!DestTy->isBooleanType() && "bool should use PointerToBool"); - return Builder.CreatePtrToInt(Visit(E), ConvertType(DestTy)); + auto *PtrExpr = Visit(E); + + if (CGF.CGM.getCodeGenOpts().StrictVTablePointers) { + const CXXRecordDecl *ClassDecl = + E->getType().getTypePtr()->getPointeeCXXRecordDecl(); + // Casting to integer requires stripping dynamic information as it does + // not carries it. + if (ClassDecl && ClassDecl->mayBeDynamicClass()) + PtrExpr = Builder.CreateStripInvariantGroup(PtrExpr); + } + return Builder.CreatePtrToInt(PtrExpr, ConvertType(DestTy)); + } case CK_ToVoid: { CGF.EmitIgnoredExpr(E); return nullptr; @@ -3238,6 +3286,25 @@ Result = Builder.CreateICmp(SICmpOpc, LHS, RHS, "cmp"); } else { // Unsigned integers and pointers. + + if (CGF.CGM.getCodeGenOpts().StrictVTablePointers && + !isa(LHS) && + !isa(RHS)) { + + // Dynamic information is required to be stripped for comparisons, + // because it could leak the dynamic information. Based on comparisons + // of pointers to dynamic objects, the optimizer can replace one pointer + // with another, which might be incorrect in presence of invariant + // groups. Comparison with null is safe because null does not carry any + // dynamic information. + if (auto *RD = LHSTy->getPointeeCXXRecordDecl()) + if (RD->mayBeDynamicClass()) + LHS = Builder.CreateStripInvariantGroup(LHS); + if (auto *RD = RHSTy->getPointeeCXXRecordDecl()) + if (RD->mayBeDynamicClass()) + RHS = Builder.CreateStripInvariantGroup(RHS); + } + Result = Builder.CreateICmp(UICmpOpc, LHS, RHS, "cmp"); } Index: clang/test/CodeGenCXX/strict-vtable-pointers.cpp =================================================================== --- clang/test/CodeGenCXX/strict-vtable-pointers.cpp +++ clang/test/CodeGenCXX/strict-vtable-pointers.cpp @@ -5,7 +5,8 @@ // RUN: FileCheck --check-prefix=CHECK-LINK-REQ %s < %t.ll typedef __typeof__(sizeof(0)) size_t; -void *operator new(size_t, void*) throw(); +void *operator new(size_t, void *) throw(); +using uintptr_t = unsigned long long; struct NotTrivialDtor { ~NotTrivialDtor(); @@ -17,7 +18,7 @@ }; struct DynamicDerived : DynamicBase1 { - void foo(); + void foo() override; }; struct DynamicBase2 { @@ -28,8 +29,8 @@ }; struct DynamicDerivedMultiple : DynamicBase1, DynamicBase2 { - virtual void foo(); - virtual void bar(); + void foo() override; + void bar() override; }; struct StaticBase { @@ -47,9 +48,8 @@ struct DynamicFromVirtualStatic2 : virtual StaticBase { }; -struct DynamicFrom2Virtuals : - DynamicFromVirtualStatic1, - DynamicFromVirtualStatic2 { +struct DynamicFrom2Virtuals : DynamicFromVirtualStatic1, + DynamicFromVirtualStatic2 { }; // CHECK-NEW-LABEL: define void @_Z12LocalObjectsv() @@ -89,7 +89,6 @@ // CHECK-CTORS: call i8* @llvm.launder.invariant.group.p0i8( // CHECK-CTORS-LABEL: {{^}}} - // CHECK-NEW-LABEL: define void @_Z9Pointers1v() // CHECK-NEW-NOT: @llvm.launder.invariant.group.p0i8( // CHECK-NEW-LABEL: call void @_ZN12DynamicBase1C1Ev( @@ -134,7 +133,6 @@ // CHECK-CTORS-NOT: call i8* @llvm.launder.invariant.group.p0i8( // CHECK-CTORS-LABEL: {{^}}} - struct DynamicDerived; // CHECK-CTORS-LABEL: define linkonce_odr void @_ZN14DynamicDerivedC2Ev( @@ -164,14 +162,12 @@ // CHECK-CTORS: call void @_ZN12DynamicBase2C2Ev( // CHECK-CTORS-NOT: @llvm.launder.invariant.group.p0i8 - // CHECK-CTORS: %[[THIS10:.*]] = bitcast %struct.DynamicDerivedMultiple* %[[THIS0]] to i32 (...)*** // CHECK-CTORS: store {{.*}} @_ZTV22DynamicDerivedMultiple, i32 0, inrange i32 0, i32 2) {{.*}} %[[THIS10]] // CHECK-CTORS: %[[THIS11:.*]] = bitcast %struct.DynamicDerivedMultiple* %[[THIS0]] to i8* // CHECK-CTORS: %[[THIS_ADD:.*]] = getelementptr inbounds i8, i8* %[[THIS11]], i64 16 // CHECK-CTORS: %[[THIS12:.*]] = bitcast i8* %[[THIS_ADD]] to i32 (...)*** - // CHECK-CTORS: store {{.*}} @_ZTV22DynamicDerivedMultiple, i32 0, inrange i32 1, i32 2) {{.*}} %[[THIS12]] // CHECK-CTORS-LABEL: {{^}}} @@ -182,9 +178,10 @@ struct A { virtual void foo(); + int m; }; struct B : A { - virtual void foo(); + void foo() override; }; union U { @@ -209,7 +206,7 @@ // CHECK-NEW: call i8* @llvm.launder.invariant.group.p0i8(i8* // CHECK-NEW: call void @_Z2g2P1A(%struct.A* g2(&u->b); - // CHECK-NEW: call void @_Z9changeToAP1U(%union.U* + // CHECK-NEW: call void @_Z9changeToAP1U(%union.U* changeToA(u); // CHECK-NEW: call i8* @llvm.launder.invariant.group.p0i8(i8* // call void @_Z2g2P1A(%struct.A* %a) @@ -294,12 +291,279 @@ take(u.v3); } +// CHECK-NEW-LABEL: define void @_Z7comparev() +void compare() { + A *a = new A; + a->foo(); + // CHECK-NEW: call i8* @llvm.launder.invariant.group.p0i8(i8* + A *b = new (a) B; + + // CHECK-NEW: %[[a:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* + // CHECK-NEW: %[[a2:.*]] = bitcast i8* %[[a]] to %struct.A* + // CHECK-NEW: %[[b:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* + // CHECK-NEW: %[[b2:.*]] = bitcast i8* %[[b]] to %struct.A* + // CHECK-NEW: %cmp = icmp eq %struct.A* %[[a2]], %[[b2]] + if (a == b) + b->foo(); +} + +// CHECK-NEW-LABEL: compare2 +bool compare2(A *a, A *a2) { + // CHECK-NEW: %[[a:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* + // CHECK-NEW: %[[a2:.*]] = bitcast i8* %[[a]] to %struct.A* + // CHECK-NEW: %[[b:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* + // CHECK-NEW: %[[b2:.*]] = bitcast i8* %[[b]] to %struct.A* + // CHECK-NEW: %cmp = icmp ult %struct.A* %[[a2]], %[[b2]] + return a < a2; +} +// CHECK-NEW-LABEL: compareIntPointers +bool compareIntPointers(int *a, int *b) { + // CHECK-NEW-NOT: call i8* @llvm.strip.invariant.group + return a == b; +} + +struct HoldingOtherVirtuals { + B b; +}; + +// There is no need to add barriers for comparision of pointer to classes +// that are not dynamic. +// CHECK-NEW-LABEL: compare5 +bool compare5(HoldingOtherVirtuals *a, HoldingOtherVirtuals *b) { + // CHECK-NEW-NOT: call i8* @llvm.strip.invariant.group + return a == b; +} +// CHECK-NEW-LABEL: compareNull +bool compareNull(A *a) { + // CHECK-NEW-NOT: call i8* @llvm.strip.invariant.group + + if (a != nullptr) + return false; + if (!a) + return false; + return a == nullptr; +} + +struct X; +// We have to also introduce the barriers if comparing pointers to incomplete +// objects +// CHECK-NEW-LABEL: define zeroext i1 @_Z8compare4P1XS0_ +bool compare4(X *x, X *x2) { + // CHECK-NEW: %[[x:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* + // CHECK-NEW: %[[xp:.*]] = bitcast i8* %[[x]] to %struct.X* + // CHECK-NEW: %[[x2:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* + // CHECK-NEW: %[[x2p:.*]] = bitcast i8* %[[x2]] to %struct.X* + // CHECK-NEW: %cmp = icmp eq %struct.X* %[[xp]], %[[x2p]] + return x == x2; +} + +// CHECK-NEW-LABEL: define void @_Z7member1P20HoldingOtherVirtuals( +void member1(HoldingOtherVirtuals *p) { + + // CHECK-NEW-NOT: call i8* @llvm.strip.invariant.group.p0i8( + (void)p->b; +} + +// CHECK-NEW-LABEL: member2 +void member2(A *a) { + // CHECK-NEW: call i8* @llvm.strip.invariant.group.p0i8 + (void)a->m; +} + +// Check if from comparison of addresses of member we can't infer the equality +// of ap and bp. +// CHECK-NEW-LABEL: @_Z18testCompareMembersv( +void testCompareMembers() { + // CHECK-NEW: [[AP:%.*]] = alloca %struct.A* + // CHECK-NEW: [[APM:%.*]] = alloca i32* + // CHECK-NEW: [[BP:%.*]] = alloca %struct.B* + // CHECK-NEW: [[BPM:%.*]] = alloca i32* + + A *ap = new A; + // CHECK-NEW: call void %{{.*}}(%struct.A* %{{.*}}) + ap->foo(); + // CHECK-NEW: [[TMP7:%.*]] = load %struct.A*, %struct.A** [[AP]] + // CHECK-NEW: [[TMP8:%.*]] = bitcast %struct.A* [[TMP7]] to i8* + // CHECK-NEW: [[TMP9:%.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* [[TMP8]]) + // CHECK-NEW: [[TMP10:%.*]] = bitcast i8* [[TMP9]] to %struct.A* + // CHECK-NEW: [[M:%.*]] = getelementptr inbounds [[STRUCT_A:%.*]], %struct.A* [[TMP10]], i32 0, i32 1 + // CHECK-NEW: store i32* [[M]], i32** [[APM]] + int *const apm = &ap->m; + + B *bp = new (ap) B; + + // CHECK-NEW: [[TMP20:%.*]] = load %struct.B*, %struct.B** [[BP]] + // CHECK-NEW: [[TMP21:%.*]] = bitcast %struct.B* [[TMP20]] to %struct.A* + // CHECK-NEW: [[TMP22:%.*]] = bitcast %struct.A* [[TMP21]] to i8* + // CHECK-NEW: [[TMP23:%.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8* [[TMP22]]) + // CHECK-NEW: [[TMP24:%.*]] = bitcast i8* [[TMP23]] to %struct.A* + // CHECK-NEW: [[M4:%.*]] = getelementptr inbounds [[STRUCT_A]], %struct.A* [[TMP24]], i32 0, i32 1 + // CHECK-NEW: store i32* [[M4]], i32** [[BPM]] + int *const bpm = &bp->m; + + // CHECK-NEW: [[TMP25:%.*]] = load i32*, i32** [[APM]] + // CHECK-NEW: [[TMP26:%.*]] = load i32*, i32** [[BPM]] + // CHECK-NEW-NOT: strip.invariant.group + // CHECK-NEW-NOT: launder.invariant.group + // CHECK-NEW: [[CMP:%.*]] = icmp eq i32* [[TMP25]], [[TMP26]] + if (apm == bpm) { + bp->foo(); + } +} + +// CHECK-NEW-LABEL: define void @_Z9testCast1P1A(%struct.A* +void testCast1(A *a) { + // Here we get rid of dynamic info + // CHECK-NEW: call i8* @llvm.strip.invariant.group + auto *v = (void *)a; + + // CHECK-NEW: call i8* @llvm.strip.invariant.group + auto i2 = (uintptr_t)a; + (void)i2; + + // CHECK-NEW-NOT: @llvm.strip.invariant.group + // CHECK-NEW-NOT: @llvm.launder.invariant.group + + // The information is already stripped + auto i = (uintptr_t)v; +} + +struct Incomplete; +// CHECK-NEW-LABEL: define void @_Z9testCast2P10Incomplete(%struct.Incomplete* +void testCast2(Incomplete *I) { + // Here we get rid of potential dynamic info + // CHECK-NEW: call i8* @llvm.strip.invariant.group + auto *v = (void *)I; + + // CHECK-NEW: call i8* @llvm.strip.invariant.group + auto i2 = (uintptr_t)I; + (void)i2; + + // CHECK-NEW-NOT: @llvm.strip.invariant.group + // CHECK-NEW-NOT: @llvm.launder.invariant.group + + // The information is already stripped + auto i = (uintptr_t)v; +} + +// CHECK-NEW-LABEL: define void @_Z9testCast3y( +void testCast3(uintptr_t i) { + // CHECK-NEW-NOT: @llvm.strip.invariant.group + // CHECK-NEW: @llvm.launder.invariant.group + A *a3 = (A *)i; + (void)a3; + + auto *v2 = (void *)i; + + // CHECK-NEW: @llvm.launder.invariant.group + A *a2 = (A *)v2; + (void)a2; + + // CHECK-NEW-NOT: @llvm.launder.invariant.group + auto *v3 = (void *)i; + (void)v3; +} + +// CHECK-NEW-LABEL: define void @_Z9testCast4y( +void testCast4(uintptr_t i) { + // CHECK-NEW-NOT: @llvm.strip.invariant.group + // CHECK-NEW: @llvm.launder.invariant.group + auto *a3 = (Incomplete *)i; + (void)a3; + + // CHECK-NEW: @llvm.launder.invariant.group + auto *v2 = (void *)i; + // CHECK-NEW-NOT: @llvm.launder.invariant.group + auto *a2 = (Incomplete *)v2; + (void)a2; +} + +// CHECK-NEW-LABEL: define void @_Z9testCast5P1B( +void testCast5(B *b) { + // CHECK-NEW-NOT: @llvm.strip.invariant.group + // CHECK-NEW-NOT: @llvm.launder.invariant.group + A *a = b; + (void)a; + + auto *b2 = (B *)a; + (void)b2; +} + +// CHECK-NEW-LABEL: define void @_Z9testCast6P1A( +void testCast6(A *a) { + + // CHECK-NEW: @llvm.strip.invariant.group + auto *I = (Incomplete *)a; + (void)I; + // CHECK-NEW: @llvm.launder.invariant.group + auto *a2 = (A *)I; + (void)a2; + + // CHECK-NEW: @llvm.strip.invariant.group + auto *E = (Empty *)a; + (void)E; + + // CHECK-NEW: @llvm.launder.invariant.group + auto *a3 = (A *)E; + (void)a3; + + // CHECK-NEW-NOT: @llvm.strip.invariant.group + auto i = (uintptr_t)E; + (void)i; +} + +class Incomplete2; +// CHECK-NEW-LABEL: define void @_Z9testCast7P10Incomplete( +void testCast7(Incomplete *I) { + // CHECK-NEW-NOT: @llvm.strip.invariant.group + + // Incomplete2 could be dynamic where Incomplete may not be dynamic, thus + // launder is needed. We don't strip firstly because launder is sufficient. + + // CHECK-NEW: @llvm.launder.invariant.group + auto *I2 = (Incomplete2 *)I; + (void)I2; + // CHECK-NEW-LABEL: ret void +} + +template +struct PossiblyDerivingFromDynamicBase : Base { +}; + +// CHECK-NEW-LABEL: define void @_Z9testCast8P10Incomplete( +void testCast8(Incomplete *I) { + // CHECK-NEW-NOT: @llvm.strip.invariant.group + // CHECK-NEW: @llvm.launder.invariant.group + auto *P = (PossiblyDerivingFromDynamicBase *)I; + (void)P; + + // CHECK-NEW: @llvm.launder.invariant.group + auto *P2 = (PossiblyDerivingFromDynamicBase *)I; + (void)P2; + + // CHECK-NEW: @llvm.launder.invariant.group + auto *P3 = (PossiblyDerivingFromDynamicBase *)I; + (void)P3; + + // CHECK-NEW-NOT: @llvm.launder.invariant.group + auto *a3 = (A *)P3; + + // CHECK-NEW-LABEL: ret void +} + +// CHECK-NEW-LABEL: define void @_Z9testCast9 +void testCast9(PossiblyDerivingFromDynamicBase *P) { + // CHECK-NEW: @llvm.strip.invariant.group + auto *V = (void *)P; + + // CHECK-NEW-LABEL: ret void +} + /** DTORS **/ // CHECK-DTORS-LABEL: define linkonce_odr void @_ZN10StaticBaseD2Ev( // CHECK-DTORS-NOT: call i8* @llvm.launder.invariant.group.p0i8( // CHECK-DTORS-LABEL: {{^}}} - // CHECK-DTORS-LABEL: define linkonce_odr void @_ZN25DynamicFromVirtualStatic2D2Ev( // CHECK-DTORS-NOT: invariant.barrier // CHECK-DTORS-LABEL: {{^}}} @@ -308,7 +572,6 @@ // CHECK-DTORS-NOT: call i8* @llvm.launder.invariant.group.p0i8( // CHECK-DTORS-LABEL: {{^}}} - // CHECK-DTORS-LABEL: define linkonce_odr void @_ZN22DynamicDerivedMultipleD2Ev( // CHECK-DTORS-LABEL: define linkonce_odr void @_ZN12DynamicBase2D2Ev( @@ -323,10 +586,8 @@ // CHECK-DTORS-NOT: call i8* @llvm.launder.invariant.group.p0i8( // CHECK-DTORS-LABEL: {{^}}} - // CHECK-LINK-REQ: !llvm.module.flags = !{![[FIRST:[0-9]+]], ![[SEC:[0-9]+]]{{.*}}} // CHECK-LINK-REQ: ![[FIRST]] = !{i32 1, !"StrictVTablePointers", i32 1} // CHECK-LINK-REQ: ![[SEC]] = !{i32 3, !"StrictVTablePointersRequirement", ![[META:.*]]} // CHECK-LINK-REQ: ![[META]] = !{!"StrictVTablePointers", i32 1} -