Follow-up to r206457 -- fix static adjustments for some subtle virtual inheritance cases

Reviewed at http://reviews.llvm.org/D3410

llvm-svn: 206504

clang/lib/AST/VTableBuilder.cpp

@@ -2649,6 +2649,8 @@ VFTableBuilder::ComputeThisOffset(FinalOverriders::OverriderInfo Overrider) {
   CharUnits Ret;
   bool First = true;
 
+  const ASTRecordLayout &OverriderRDLayout =
+      Context.getASTRecordLayout(Overrider.Method->getParent());
   for (CXXBasePaths::paths_iterator I = Paths.begin(), E = Paths.end();
        I != E; ++I) {
     const CXXBasePath &Path = (*I);
@@ -2665,19 +2667,18 @@ VFTableBuilder::ComputeThisOffset(FinalOverriders::OverriderInfo Overrider) {
       const ASTRecordLayout &Layout = Context.getASTRecordLayout(PrevRD);
 
       if (Element.Base->isVirtual()) {
-        LastVBaseOffset = MostDerivedClassLayout.getVBaseClassOffset(CurRD);
+        // The interesting things begin when you have virtual inheritance.
-        if (Overrider.Method->getParent() == PrevRD) {
+        // The final overrider will use a static adjustment equal to the offset
-          // This one's interesting. If the final overrider is in a vbase B of the
+        // of the vbase in the final overrider class.
-          // most derived class and it overrides a method of the B's own vbase A,
+        // For example, if the final overrider is in a vbase B of the most
-          // it uses A* as "this". In its prologue, it can cast A* to B* with
+        // derived class and it overrides a method of the B's own vbase A,
-          // a static offset. This offset is used regardless of the actual
+        // it uses A* as "this".  In its prologue, it can cast A* to B* with
-          // offset of A from B in the most derived class, requiring an
+        // a static offset.  This offset is used regardless of the actual
-          // this-adjusting thunk in the vftable if A and B are laid out
+        // offset of A from B in the most derived class, requiring an
-          // differently in the most derived class.
+        // this-adjusting thunk in the vftable if A and B are laid out
-          ThisOffset += Layout.getVBaseClassOffset(CurRD);
+        // differently in the most derived class.
-        } else {
+        LastVBaseOffset = ThisOffset =
-          ThisOffset = LastVBaseOffset;
+            Overrider.Offset + OverriderRDLayout.getVBaseClassOffset(CurRD);
-        }
       } else {
         ThisOffset += Layout.getBaseClassOffset(CurRD);
       }
@@ -2739,22 +2740,9 @@ void VFTableBuilder::CalculateVtordispAdjustment(
 
   // A simple vtordisp thunk will suffice if the final overrider is defined
   // in either the most derived class or its non-virtual base.
-  if (OverriderRD == MostDerivedClass)
+  if (OverriderRD == MostDerivedClass || !OverriderVBase)
     return;
 
-  if (!OverriderVBase) {
-    MethodVFTableLocation ML = VTables.getMethodVFTableLocation(Overrider.Method);
-    assert(ML.VBase && "why would we need a vtordisp if we can call the method "
-                       "without a vfptr of a vbase?");
-    // We need to offset the this parameter if the offset of the vbase is
-    // different between the overrider class and the most derived class.
-    const ASTRecordLayout &OverriderRDLayout =
-        Context.getASTRecordLayout(OverriderRD);
-    TA.NonVirtual += (OverriderRDLayout.getVBaseClassOffset(ML.VBase) +
-                     ML.VFPtrOffset - ThisOffset).getQuantity();
-    return;
-  }
-
   // Otherwise, we need to do use the dynamic offset of the final overrider
   // in order to get "this" adjustment right.
   TA.Virtual.Microsoft.VBPtrOffset =

clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp

@@ -455,6 +455,35 @@ struct W : virtual X, A {};
 W w;
 }
 
+namespace Test12 {
+struct X : B, A { };
+
+struct Y : X {
+  virtual void f();  // Overrides A::f.
+};
+
+struct Z : virtual Y {
+  // CHECK-LABEL: VFTable for 'A' in 'Test12::X' in 'Test12::Y' in 'Test12::Z' (2 entries).
+  // CHECK-NEXT:   0 | void Test12::Y::f()
+  // CHECK-NEXT:   1 | void A::z()
+
+  int z;
+  // MANGLING-DAG: @"\01??_7Z@Test12@@6BA@@@" = {{.*}}@"\01?f@Y@Test12@@UAEXXZ"
+};
+
+struct W : Z {
+  // CHECK-LABEL: VFTable for 'A' in 'Test12::X' in 'Test12::Y' in 'Test12::Z' in 'Test12::W' (2 entries).
+  // CHECK-NEXT:   0 | void Test12::Y::f()
+  // CHECK-NEXT:   1 | void A::z()
+  W();
+
+  int w;
+  // MANGLING-DAG: @"\01??_7W@Test12@@6BA@@@" = {{.*}}@"\01?f@Y@Test12@@UAEXXZ"
+};
+
+W::W() {}
+}
+
 namespace vdtors {
 struct X {
   virtual ~X();
@@ -697,3 +726,41 @@ D obj;
 // MANGLING-DAG: @"\01??_7B@pr19240@@6B@"
 
 }
+
+namespace pr19408 {
+// This test is a non-vtordisp version of the reproducer for PR19408.
+struct X : virtual A {
+  int x;
+};
+
+struct Y : X {
+  virtual void f();
+  int y;
+};
+
+struct Z : Y {
+  // CHECK-LABEL: VFTable for 'A' in 'pr19408::X' in 'pr19408::Y' in 'pr19408::Z' (2 entries).
+  // CHECK-NEXT:   0 | void pr19408::Y::f()
+  // CHECK-NEXT:       [this adjustment: -4 non-virtual]
+  // CHECK-NEXT:   1 | void A::z()
+
+  Z();
+  int z;
+  // MANGLING-DAG: @"\01??_7Z@pr19408@@6B@" = {{.*}}@"\01?f@Y@pr19408@@W3AEXXZ"
+};
+
+Z::Z() {}
+
+struct W : B, Y {
+  // CHECK-LABEL: VFTable for 'A' in 'pr19408::X' in 'pr19408::Y' in 'pr19408::W' (2 entries).
+  // CHECK-NEXT:   0 | void pr19408::Y::f()
+  // CHECK-NEXT:       [this adjustment: -4 non-virtual]
+  // CHECK-NEXT:   1 | void A::z()
+
+  W();
+  int w;
+  // MANGLING-DAG: @"\01??_7W@pr19408@@6BY@1@@" = {{.*}}@"\01?f@Y@pr19408@@W3AEXXZ"
+};
+
+W::W() {}
+}