Author: Nikita Popov Date: 2026-09-22T09:26:08+02:00 New Revision: 3a4d8a9e6acaed193477208328be0eb8f24fd253
URL: https://github.com/llvm/llvm-project/commit/3a4d8a9e6acaed193477208328be0eb8f24fd253 DIFF: https://github.com/llvm/llvm-project/commit/3a4d8a9e6acaed193477208328be0eb8f24fd253.diff LOG: [Clang] Stop using strip.invariant.group (#225072) Clang currently emits calls to llvm.strip.invariant.group under `-fstrict-vtable-pointers` whenever a pointer to a dynamic object is used in a comparison or might be used in a comparison through operations we cannot track. The purpose of the llvm.strip.invariant.group intrinsic is to make sure that if we have a dominating condition like `a == launder.invariant.group(a)`, we don't end up replacing the latter with the former, as that would allow the optimizer to assume that the memory stays invariant, despite going through a launder operation (e.g. on placement new). However, since this was introduced, we've come to the understanding that replacing pointers based on equality comparison is generally only legal if they have the same provenance, and stopped doing such replacements. The specific strip.invariant.group/launder.invariant.group case was still buggy due to an implementation bug, but this was fixed in https://github.com/llvm/llvm-project/pull/224281. As such, I believe that we no longer need to emit llvm.strip.invariant.group in Clang. I plan to also remove the LLVM intrinsic in a followup. Added: Modified: clang/lib/CodeGen/CGBuilder.h clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/test/CodeGenCXX/strict-vtable-pointers.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h index 4a51f6be8578f..4519f7160b341 100644 --- a/clang/lib/CodeGen/CGBuilder.h +++ b/clang/lib/CodeGen/CGBuilder.h @@ -470,12 +470,6 @@ class CGBuilderTy : public CGBuilderBaseTy { Addr.replaceBasePointer(CreateLaunderInvariantGroup(Addr.getBasePointer())); return Addr; } - - using CGBuilderBaseTy::CreateStripInvariantGroup; - Address CreateStripInvariantGroup(Address Addr) { - Addr.replaceBasePointer(CreateStripInvariantGroup(Addr.getBasePointer())); - return Addr; - } }; } // end namespace CodeGen diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index eba802e187beb..c93d3fd8c562e 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5974,18 +5974,6 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, const FieldDecl *field, Address addr = base.getAddress(); if (hasBPFPreserveStaticOffset(rec)) addr = wrapWithBPFPreserveStaticOffset(*this, addr); - if (auto *ClassDef = dyn_cast<CXXRecordDecl>(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.emitRawPointer(*this)); - addr = Address(stripped, addr.getElementType(), addr.getAlignment()); - } - } unsigned RecordCVR = base.getVRQualifiers(); if (rec->isUnion()) { diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 2a88466ad564a..7b90fac46be0a 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -2717,14 +2717,6 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { // Casting to pointer that could carry dynamic information (provided by // invariant.group) requires launder. Src = Builder.CreateLaunderInvariantGroup(Src); - } else if (SrcType.mayBeDynamicClass() && DestTy.mayBeNotDynamicClass()) { - // 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); } } @@ -3007,18 +2999,8 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { } case CK_PointerToIntegral: { assert(!DestTy->isBooleanType() && "bool should use PointerToBool"); - auto *PtrExpr = Visit(E); - - if (CGF.CGM.getCodeGenOpts().StrictVTablePointers) { - const QualType SrcType = E->getType(); - - // Casting to integer requires stripping dynamic information as it does - // not carries it. - if (SrcType.mayBeDynamicClass()) - PtrExpr = Builder.CreateStripInvariantGroup(PtrExpr); - } - - PtrExpr = CGF.authPointerToPointerCast(PtrExpr, E->getType(), DestTy); + auto *PtrExpr = + CGF.authPointerToPointerCast(Visit(E), E->getType(), DestTy); return Builder.CreatePtrToInt(PtrExpr, ConvertType(DestTy)); } case CK_ToVoid: { @@ -5380,23 +5362,6 @@ Value *ScalarExprEmitter::EmitCompare(const BinaryOperator *E, Result = Builder.CreateICmp(SICmpOpc, LHS, RHS, "cmp"); } else { // Unsigned integers and pointers. - - if (CGF.CGM.getCodeGenOpts().StrictVTablePointers && - !isa<llvm::ConstantPointerNull>(LHS) && - !isa<llvm::ConstantPointerNull>(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 (LHSTy.mayBeDynamicClass()) - LHS = Builder.CreateStripInvariantGroup(LHS); - if (RHSTy.mayBeDynamicClass()) - RHS = Builder.CreateStripInvariantGroup(RHS); - } - Result = Builder.CreateICmp(UICmpOpc, LHS, RHS, "cmp"); } diff --git a/clang/test/CodeGenCXX/strict-vtable-pointers.cpp b/clang/test/CodeGenCXX/strict-vtable-pointers.cpp index ade81725487c9..85a16f571e8d0 100644 --- a/clang/test/CodeGenCXX/strict-vtable-pointers.cpp +++ b/clang/test/CodeGenCXX/strict-vtable-pointers.cpp @@ -287,18 +287,14 @@ void compare() { // CHECK-NEW: call ptr @llvm.launder.invariant.group.p0(ptr A *b = new (a) B; - // CHECK-NEW: %[[a:.*]] = call ptr @llvm.strip.invariant.group.p0(ptr - // CHECK-NEW: %[[b:.*]] = call ptr @llvm.strip.invariant.group.p0(ptr - // CHECK-NEW: %cmp = icmp eq ptr %[[a]], %[[b]] + // CHECK-NEW-NOT: call ptr @llvm.strip.invariant.group if (a == b) b->foo(); } // CHECK-NEW-LABEL: compare2 bool compare2(A *a, A *a2) { - // CHECK-NEW: %[[a:.*]] = call ptr @llvm.strip.invariant.group.p0(ptr - // CHECK-NEW: %[[b:.*]] = call ptr @llvm.strip.invariant.group.p0(ptr - // CHECK-NEW: %cmp = icmp ult ptr %[[a]], %[[b]] + // CHECK-NEW-NOT: call ptr @llvm.strip.invariant.group return a < a2; } // CHECK-NEW-LABEL: compareIntPointers @@ -330,13 +326,9 @@ bool compareNull(A *a) { } 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 ptr @llvm.strip.invariant.group.p0(ptr - // CHECK-NEW: %[[x2:.*]] = call ptr @llvm.strip.invariant.group.p0(ptr - // CHECK-NEW: %cmp = icmp eq ptr %[[x]], %[[x2]] + // CHECK-NEW-NOT: call ptr @llvm.strip.invariant.group return x == x2; } @@ -349,7 +341,7 @@ void member1(HoldingOtherVirtuals *p) { // CHECK-NEW-LABEL: member2 void member2(A *a) { - // CHECK-NEW: call ptr @llvm.strip.invariant.group.p0 + // CHECK-NEW-NOT: call ptr @llvm.strip.invariant.group.p0 (void)a->m; } @@ -366,16 +358,14 @@ void testCompareMembers() { // CHECK-NEW: call void %{{.*}}(ptr {{[^,]*}} %{{.*}}) ap->foo(); // CHECK-NEW: [[TMP7:%.*]] = load ptr, ptr [[AP]] - // CHECK-NEW: [[TMP9:%.*]] = call ptr @llvm.strip.invariant.group.p0(ptr [[TMP7]]) - // CHECK-NEW: [[M:%.*]] = getelementptr inbounds nuw [[STRUCT_A:%.*]], ptr [[TMP9]], i32 0, i32 1 + // CHECK-NEW: [[M:%.*]] = getelementptr inbounds nuw [[STRUCT_A:%.*]], ptr [[TMP7]], i32 0, i32 1 // CHECK-NEW: store ptr [[M]], ptr [[APM]] int *const apm = &ap->m; B *bp = new (ap) B; // CHECK-NEW: [[TMP20:%.*]] = load ptr, ptr [[BP]] - // CHECK-NEW: [[TMP23:%.*]] = call ptr @llvm.strip.invariant.group.p0(ptr [[TMP20]]) - // CHECK-NEW: [[M4:%.*]] = getelementptr inbounds nuw [[STRUCT_A]], ptr [[TMP23]], i32 0, i32 1 + // CHECK-NEW: [[M4:%.*]] = getelementptr inbounds nuw [[STRUCT_A]], ptr [[TMP20]], i32 0, i32 1 // CHECK-NEW: store ptr [[M4]], ptr [[BPM]] int *const bpm = &bp->m; @@ -391,36 +381,26 @@ void testCompareMembers() { // CHECK-NEW-LABEL: define{{.*}} void @_Z9testCast1P1A(ptr void testCast1(A *a) { - // Here we get rid of dynamic info - // CHECK-NEW: call ptr @llvm.strip.invariant.group + // CHECK-NEW-NOT: call ptr @llvm.strip.invariant.group + // CHECK-NEW-NOT: @llvm.launder.invariant.group auto *v = (void *)a; - // CHECK-NEW: call ptr @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(ptr void testCast2(Incomplete *I) { - // Here we get rid of potential dynamic info - // CHECK-NEW: call ptr @llvm.strip.invariant.group + // CHECK-NEW-NOT: @llvm.strip.invariant.group + // CHECK-NEW-NOT: @llvm.launder.invariant.group auto *v = (void *)I; - // CHECK-NEW: call ptr @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; } @@ -470,14 +450,14 @@ void testCast5(B *b) { // CHECK-NEW-LABEL: define{{.*}} void @_Z9testCast6P1A( void testCast6(A *a) { - // CHECK-NEW: @llvm.strip.invariant.group + // CHECK-NEW-NOT: @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 + // CHECK-NEW-NOT: @llvm.strip.invariant.group auto *E = (Empty *)a; (void)E; @@ -531,7 +511,7 @@ void testCast8(Incomplete *I) { // CHECK-NEW-LABEL: define{{.*}} void @_Z9testCast9 void testCast9(PossiblyDerivingFromDynamicBase<Incomplete> *P) { - // CHECK-NEW: @llvm.strip.invariant.group + // CHECK-NEW-NOT: @llvm.strip.invariant.group auto *V = (void *)P; // CHECK-NEW-LABEL: ret void _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
