[PATCH] D31830: Emit invariant.group.barrier when using union field

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek created this revision.

We need to emit barrier if the union field
is CXXRecordDecl because it might have vptrs. The testcode 
was wrongly devirtualized. It also proves that having different
groups for different dynamic types is not sufficient.


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,62 @@
 // 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;
+};
+
+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
@@ -3508,6 +3508,15 @@
     assert(!type->isReferenceType() && "union has reference member");
     // TODO: handle path-aware TBAA for union.
     TBAAPath = false;
+    // Because unions can easily skip invariant.barriers, we need to add
+    // a barrier every time CXXRecord field is referenced. We don't check if
+    // it is dynamic type because it might have dynamic member, so we would
+    // have to check it recursively.
+    if (field->getType().getTypePtr()->getAsCXXRecordDecl() &&
+        CGM.getCodeGenOpts().StrictVTablePointers &&
+        CGM.getCodeGenOpts().OptimizationLevel > 0)
+      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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31830.94569.patch
Type: text/x-patch
Size: 3313 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170410/0b912b9d/attachment.bin>