Emiting invariant.group.barrier for ctors bugfix

[Prazek]

Summary

Please review asap.

This bug caused clang call pure virtual functions when called from constructor body.

[nlewycky]

I can't meaningfully review this, but I see nothing wrong here.

lib/CodeGen/CGClass.cpp

1378

"llvm::Value* const" --> "llvm::Value *const"

[rsmith]

LGTM, but your commit message should be more descriptive than your current summary. Something like:

Ensure that the vptr store in the most-derived constructor is not behind an invariant group barrier. Previously, the base-most vptr store would be the one behind no barrier, and that could result in the creator of the object thinking it had the base-most vtable.

[chapuni]

test/CodeGenCXX/strict-vtable-pointers.cpp

145

You missed "%2" (should be THIS3). Fixed in r249223.

[Prazek]

Thank you!

lib/CodeGen/CGClass.cpp

if (ClassDecl->getNumVBases() &&
       !CGM.getTarget().getCXXABI().hasConstructorVariants()) {
     // The ABIs that don't have constructor variants need to put a branch
     // before the virtual base initialization code.
     BaseCtorContinueBB =
       CGM.getCXXABI().EmitCtorCompleteObjectHandler(*this, ClassDecl);
     assert(BaseCtorContinueBB);
   }
 
-  bool BaseVPtrsInitialized = false;
+  llvm::Value *const OldThis = CXXThisValue;
   // Virtual base initializers first.
   for (; B != E && (*B)->isBaseInitializer() && (*B)->isBaseVirtual(); B++) {
-    CXXCtorInitializer *BaseInit = *B;
+    if (CGM.getCodeGenOpts().StrictVTablePointers &&
+        CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+        isInitializerOfDynamicClass(*B))
+      CXXThisValue = Builder.CreateInvariantGroupBarrier(LoadCXXThis());
     EmitBaseInitializer(*this, ClassDecl, *B, CtorType);
-    BaseVPtrsInitialized |= BaseInitializerUsesThis(getContext(),
-                                                    BaseInit->getInit());
   }
 
   if (BaseCtorContinueBB) {
     // Complete object handler should continue to the remaining initializers.
     Builder.CreateBr(BaseCtorContinueBB);
     EmitBlock(BaseCtorContinueBB);
   }
 
   // Then, non-virtual base initializers.
   for (; B != E && (*B)->isBaseInitializer(); B++) {
     assert(!(*B)->isBaseVirtual());
-    EmitBaseInitializer(*this, ClassDecl, *B, CtorType);
-    BaseVPtrsInitialized |= isInitializerOfDynamicClass(*B);
-  }
 
-  // Pointer to this requires to be passed through invariant.group.barrier
-  // only if we've initialized any base vptrs.
     if (CGM.getCodeGenOpts().StrictVTablePointers &&
-      CGM.getCodeGenOpts().OptimizationLevel > 0 && BaseVPtrsInitialized)
+        CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+        isInitializerOfDynamicClass(*B))
       CXXThisValue = Builder.CreateInvariantGroupBarrier(LoadCXXThis());
+    EmitBaseInitializer(*this, ClassDecl, *B, CtorType);
+  }
+
+  CXXThisValue = OldThis;
 
   InitializeVTablePointers(ClassDecl);
 
   // And finally, initialize class members.
   FieldConstructionScope FCS(*this, LoadCXXThisAddress());
   ConstructorMemcpyizer CM(*this, CD, Args);
   for (; B != E; B++) {
     CXXCtorInitializer *Member = (*B);

test/CodeGenCXX/invariant.group-for-vptrs.cpp

 
 // Checking A::A()
 // CHECK-LABEL: define linkonce_odr void @_ZN1AC2Ev(
 // CHECK: store {{.*}}, !invariant.group ![[A_MD]]
 // CHECK-LABEL: }
 
 // Checking D::D()
 // CHECK-LABEL: define linkonce_odr void @_ZN1DC2Ev(
-
-// CHECK:  call void @_ZN1AC2Ev(%struct.A*
 // CHECK:  = call i8* @llvm.invariant.group.barrier(i8*
+// CHECK:  call void @_ZN1AC2Ev(%struct.A*
 // CHECK: store {{.*}} !invariant.group ![[D_MD]]
 
 // Checking B::B()
 // CHECK-LABEL: define internal void @_ZN12_GLOBAL__N_11BC2Ev(
 // CHECK:  store {{.*}}, !invariant.group ![[B_MD]]
 
 // Checking C::C()
 // CHECK-LABEL: define internal void @_ZN12_GLOBAL__N_11CC2Ev(
 // CHECK:  store {{.*}}, !invariant.group ![[C_MD]]
 
 // CHECK: ![[A_MD]] = !{!"_ZTS1A"}
 // CHECK: ![[D_MD]] = !{!"_ZTS1D"}
 // CHECK: ![[B_MD]] = distinct !{}
 // CHECK: ![[C_MD]] = distinct !{}

test/CodeGenCXX/strict-vtable-pointers.cpp

 /*** Testing Constructors ***/
 struct DynamicBase1;
 // CHECK-CTORS-LABEL: define linkonce_odr void @_ZN12DynamicBase1C2Ev(
 // CHECK-CTORS-NOT: call i8* @llvm.invariant.group.barrier(
 // CHECK-CTORS-LABEL: }
 
 
 struct DynamicDerived;
+
 // CHECK-CTORS-LABEL: define linkonce_odr void @_ZN14DynamicDerivedC2Ev(
-// CHECK-CTORS: call void @_ZN12DynamicBase1C2Ev(
-// CHECK-CTORS: %[[THIS1:.*]] = bitcast %[[DynamicDerived:.*]]* %[[THIS0:.*]] to i8*
+// CHECK-CTORS: %[[THIS0:.*]] = load %[[DynamicDerived:.*]]*, %[[DynamicDerived]]** {{.*}}
+// CHECK-CTORS: %[[THIS1:.*]] = bitcast %[[DynamicDerived:.*]]* %[[THIS0]] to i8*
 // CHECK-CTORS: %[[THIS2:.*]] = call i8* @llvm.invariant.group.barrier(i8* %[[THIS1:.*]])
-// CHECK-CTORS: %[[THIS3:.*]] = bitcast i8* %[[THIS2:.*]] to %[[DynamicDerived]]*
-// CHECK-CTORS: %[[THIS4:.*]] = bitcast %struct.DynamicDerived* %[[THIS3:.*]] to i32 (...)***
-// CHECK-CTORS: store {{.*}} %[[THIS4:.*]]
+// CHECK-CTORS: %[[THIS3:.*]] = bitcast i8* %[[THIS2]] to %[[DynamicDerived]]*
+// CHECK-CTORS: %[[THIS4:.*]] = bitcast %[[DynamicDerived]]* %2 to %[[DynamicBase:.*]]*
+// CHECK-CTORS: call void @_ZN12DynamicBase1C2Ev(%[[DynamicBase]]* %[[THIS4]])
+
+// CHECK-CTORS: %[[THIS5:.*]] = bitcast %struct.DynamicDerived* %[[THIS0]] to i32 (...)***
+// CHECK-CTORS: store {{.*}} %[[THIS5]]
 // CHECK-CTORS-LABEL: }
 
 struct DynamicDerivedMultiple;
-// CHECK-CTORS-LABEL: define linkonce_odr void @_ZN22DynamicDerivedMultipleC2Ev
-// CHECK-CTORS: call void @_ZN12DynamicBase1C2Ev(
+// CHECK-CTORS-LABEL: define linkonce_odr void @_ZN22DynamicDerivedMultipleC2Ev(
+
+// CHECK-CTORS: %[[THIS0:.*]] = load %[[CLASS:.*]]*, %[[CLASS]]** {{.*}}
+// CHECK-CTORS: %[[THIS1:.*]] = bitcast %[[CLASS:.*]]* %[[THIS0]] to i8*
+// CHECK-CTORS: %[[THIS2:.*]] = call i8* @llvm.invariant.group.barrier(i8* %[[THIS1]])
+// CHECK-CTORS: %[[THIS3:.*]] = bitcast i8* %[[THIS2]] to %[[CLASS]]*
+// CHECK-CTORS: %[[THIS4:.*]] = bitcast %[[CLASS]]* %[[THIS3]] to %[[BASE_CLASS:.*]]*
+// CHECK-CTORS: call void @_ZN12DynamicBase1C2Ev(%[[BASE_CLASS]]* %[[THIS4]])
+
+// CHECK-CTORS: call i8* @llvm.invariant.group.barrier(
+
+// CHECK-CTORS: call void @_ZN12DynamicBase2C2Ev(
 // CHECK-CTORS-NOT: @llvm.invariant.group.barrier
-// CHECK-CTORS-LABEL: call void @_ZN12DynamicBase2C2Ev(
-// CHECK-CTORS: %[[THIS1:.*]] = bitcast %[[CLASS:.*]]* %[[THIS0:.*]] to i8*
-// CHECK-CTORS: %[[THIS2:.*]] = call i8* @llvm.invariant.group.barrier(i8* %[[THIS1:.*]])
-// CHECK-CTORS: %[[THIS3:.*]] = bitcast i8* %[[THIS2:.*]] to %[[CLASS]]*
-// CHECK-CTORS-NOT: invariant.group.barrier
-// CHECK-CTORS: store {{.*}} @_ZTV22DynamicDerivedMultiple, i64 0, i64 2)
-// CHECK-CTORS: store {{.*}} @_ZTV22DynamicDerivedMultiple, i64 0, i64 6)
+
+
+// CHECK-CTORS: %[[THIS10:.*]] = bitcast %struct.DynamicDerivedMultiple* %[[THIS0]] to i32 (...)***
+// CHECK-CTORS: store {{.*}} @_ZTV22DynamicDerivedMultiple, i64 0, i64 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, i64 0, i64 6) {{.*}} %[[THIS12]]
 // CHECK-CTORS-LABEL: }
 
 struct DynamicFromStatic;
 // CHECK-CTORS-LABEL: define linkonce_odr void @_ZN17DynamicFromStaticC2Ev(
 // CHECK-CTORS-NOT: @llvm.invariant.group.barrier(
 // CHECK-CTORS-LABEL: }