From 42ee2c4bdf7c217c5d1849ebdf2194fab7bec450 Mon Sep 17 00:00:00 2001 From: Piotr Padlewski Date: Thu, 1 Jun 2017 18:39:34 +0000 Subject: [PATCH] Emit invariant.group.barrier when using union field Summary: 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. Reviewers: rjmccall, rsmith, mehdi_amini Subscribers: amharc, cfe-commits Differential Revision: https://reviews.llvm.org/D31830 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@304448 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGExpr.cpp | 27 +++++ test/CodeGenCXX/strict-vtable-pointers.cpp | 115 ++++++++++++++++++++- 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index b918a663ce..6808c69cd8 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -3530,6 +3530,25 @@ static Address emitAddrOfFieldStorage(CodeGenFunction &CGF, Address base, return CGF.Builder.CreateStructGEP(base, idx, offset, field->getName()); } +static bool hasAnyVptr(const QualType Type, const ASTContext &Context) { + const auto *RD = Type.getTypePtr()->getAsCXXRecordDecl(); + if (!RD) + return false; + + if (RD->isDynamicClass()) + return true; + + for (const auto &Base : RD->bases()) + if (hasAnyVptr(Base.getType(), Context)) + return true; + + for (const FieldDecl *Field : RD->fields()) + if (hasAnyVptr(Field->getType(), Context)) + return true; + + return false; +} + LValue CodeGenFunction::EmitLValueForField(LValue base, const FieldDecl *field) { LValueBaseInfo BaseInfo = base.getBaseInfo(); @@ -3572,6 +3591,14 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, assert(!type->isReferenceType() && "union has reference member"); // TODO: handle path-aware TBAA for union. TBAAPath = false; + + const auto FieldType = field->getType(); + if (CGM.getCodeGenOpts().StrictVTablePointers && + hasAnyVptr(FieldType, 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); diff --git a/test/CodeGenCXX/strict-vtable-pointers.cpp b/test/CodeGenCXX/strict-vtable-pointers.cpp index 928817bfb1..890e53e869 100644 --- a/test/CodeGenCXX/strict-vtable-pointers.cpp +++ b/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,119 @@ struct DynamicFromStatic; // 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); +} + +struct VirtualInBase : HoldingVirtuals, Empty { +}; + +struct VirtualInVBase : virtual Empty, virtual HoldingVirtuals { +}; + +// It has vtable by virtual inheritance. +struct VirtualInheritance : virtual Empty { +}; + +union U3 { + VirtualInBase v1; + VirtualInBase v2; + VirtualInheritance v3; + int z; +}; + +void take(VirtualInBase &); +void take(VirtualInVBase &); +void take(VirtualInheritance &); + +void UnionsBarrier3(U3 &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 @_Z4takeR13VirtualInBase( + take(u.v1); + // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8* + // CHECK-NEW: call void @_Z4takeR13VirtualInBase( + take(u.v2); + + // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8* + // CHECK-NEW: call void @_Z4takeR18VirtualInheritance( + take(u.v3); +} /** DTORS **/ // CHECK-DTORS-LABEL: define linkonce_odr void @_ZN10StaticBaseD2Ev( -- 2.40.0