Prazek updated this revision to Diff 96312.
Prazek added a comment.
- Inserting barrier with -O0
https://reviews.llvm.org/D31830
Files:
lib/CodeGen/CGExpr.cpp
test/CodeGenCXX/strict-vtable-pointers.cpp
Index: test/CodeGenCXX/strict-vtable-pointers.cpp
===================================================================
--- test/CodeGenCXX/strict-vtable-pointers.cpp
+++ test/CodeGenCXX/strict-vtable-pointers.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -fstrict-vtable-pointers -disable-llvm-passes -O2 -emit-llvm -o %t.ll
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -fstrict-vtable-pointers -std=c++11 -disable-llvm-passes -O2 -emit-llvm -o %t.ll
// RUN: FileCheck --check-prefix=CHECK-CTORS %s < %t.ll
// RUN: FileCheck --check-prefix=CHECK-NEW %s < %t.ll
// RUN: FileCheck --check-prefix=CHECK-DTORS %s < %t.ll
@@ -180,6 +180,82 @@
// CHECK-CTORS-NOT: @llvm.invariant.group.barrier(
// CHECK-CTORS-LABEL: {{^}}}
+struct A {
+ virtual void foo();
+};
+struct B : A {
+ virtual void foo();
+};
+
+union U {
+ A a;
+ B b;
+};
+
+void changeToB(U *u);
+void changeToA(U *u);
+
+void g2(A *a) {
+ a->foo();
+}
+// We have to guard access to union fields with invariant.group, because
+// it is very easy to skip the barrier with unions. In this example the inlined
+// g2 will produce loads with the same !invariant.group metadata, and
+// u->a and u->b would use the same pointer.
+// CHECK-NEW-LABEL: define void @_Z14UnionsBarriersP1U
+void UnionsBarriers(U *u) {
+ // CHECK-NEW: call void @_Z9changeToBP1U(
+ changeToB(u);
+ // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8*
+ // CHECK-NEW: call void @_Z2g2P1A(%struct.A*
+ g2(&u->b);
+ // CHECK-NEW: call void @_Z9changeToAP1U(%union.U* %6)
+ changeToA(u);
+ // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8*
+ // call void @_Z2g2P1A(%struct.A* %a)
+ g2(&u->a);
+ // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+}
+
+struct HoldingVirtuals {
+ A a;
+};
+
+struct Empty {};
+struct AnotherEmpty {
+ Empty e;
+};
+union NoVptrs {
+ int a;
+ AnotherEmpty empty;
+};
+void take(AnotherEmpty &);
+
+// CHECK-NEW-LABEL: noBarriers
+void noBarriers(NoVptrs &noVptrs) {
+ // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+ // CHECK-NEW: 42
+ noVptrs.a += 42;
+ // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+ // CHECK-NEW: call void @_Z4takeR12AnotherEmpty(
+ take(noVptrs.empty);
+}
+
+union U2 {
+ HoldingVirtuals h;
+ int z;
+};
+void take(HoldingVirtuals &);
+
+// CHECK-NEW-LABEL: define void @_Z15UnionsBarriers2R2U2
+void UnionsBarriers2(U2 &u) {
+ // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+ // CHECK-NEW: 42
+ u.z += 42;
+ // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8*
+ // CHECK-NEW: call void @_Z4takeR15HoldingVirtuals(
+ take(u.h);
+}
/** DTORS **/
// CHECK-DTORS-LABEL: define linkonce_odr void @_ZN10StaticBaseD2Ev(
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3513,6 +3513,22 @@
return CGF.Builder.CreateStructGEP(base, idx, offset, field->getName());
}
+static bool hasAnyVptr(const CXXRecordDecl *RD, const ASTContext &Context) {
+ if (RD->isDynamicClass())
+ return true;
+
+ for (const FieldDecl *Field : RD->fields()) {
+ QualType FieldType = Context.getBaseElementType(Field->getType());
+ const RecordType *RT = FieldType->getAs<RecordType>();
+ if (!RT)
+ continue;
+ auto *FieldClassDecl = cast<CXXRecordDecl>(RT->getDecl());
+ if (hasAnyVptr(FieldClassDecl, Context))
+ return true;
+ }
+ return false;
+}
+
LValue CodeGenFunction::EmitLValueForField(LValue base,
const FieldDecl *field) {
AlignmentSource fieldAlignSource =
@@ -3552,6 +3568,14 @@
assert(!type->isReferenceType() && "union has reference member");
// TODO: handle path-aware TBAA for union.
TBAAPath = false;
+
+ const auto *RD = field->getType().getTypePtr()->getAsCXXRecordDecl();
+ if (CGM.getCodeGenOpts().StrictVTablePointers &&
+ RD && hasAnyVptr(RD, getContext()))
+ // Because unions can easily skip invariant.barriers, we need to add
+ // a barrier every time CXXRecord field with vptr is referenced.
+ addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),
+ addr.getAlignment());
} else {
// For structs, we GEP to the field that the record layout suggests.
addr = emitAddrOfFieldStorage(*this, addr, field);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
