r312017 - [ms] Fix vbtable index for covariant overrides of vbase methods

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Aug 29 10:40:04 2017
New Revision: 312017

URL: http://llvm.org/viewvc/llvm-project?rev=312017&view=rev
Log:
[ms] Fix vbtable index for covariant overrides of vbase methods

Overriding a method from a virtual base with a covariant return type
consumes a slot from the vftable in the virtual base. This can make it
impossible to implement certain diamond inheritance hierarchies, but we
have to follow along for compatibility in the simple cases.

This patch only affects our vtable dumper and member pointer function
mangling, since all other callers of getMethodVFTableLocation seem to
recompute VBTableIndex instead of using the one in the method location.

Patch by David Majnemer

Modified:
    cfe/trunk/lib/AST/VTableBuilder.cpp
    cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp

Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=312017&r1=312016&r2=312017&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Tue Aug 29 10:40:04 2017
@@ -2963,6 +2963,9 @@ void VFTableBuilder::AddMethods(BaseSubo
       CalculateVtordispAdjustment(FinalOverrider, ThisOffset,
                                   ThisAdjustmentOffset);
 
+    unsigned VBIndex =
+        LastVBase ? VTables.getVBTableIndex(MostDerivedClass, LastVBase) : 0;
+
     if (OverriddenMD) {
       // If MD overrides anything in this vftable, we need to update the
       // entries.
@@ -2975,6 +2978,8 @@ void VFTableBuilder::AddMethods(BaseSubo
 
       MethodInfo &OverriddenMethodInfo = OverriddenMDIterator->second;
 
+      VBIndex = OverriddenMethodInfo.VBTableIndex;
+
       // Let's check if the overrider requires any return adjustments.
       // We must create a new slot if the MD's return type is not trivially
       // convertible to the OverriddenMD's one.
@@ -2987,8 +2992,7 @@ void VFTableBuilder::AddMethods(BaseSubo
       if (!ReturnAdjustingThunk) {
         // No return adjustment needed - just replace the overridden method info
         // with the current info.
-        MethodInfo MI(OverriddenMethodInfo.VBTableIndex,
-                      OverriddenMethodInfo.VFTableIndex);
+        MethodInfo MI(VBIndex, OverriddenMethodInfo.VFTableIndex);
         MethodInfoMap.erase(OverriddenMDIterator);
 
         assert(!MethodInfoMap.count(MD) &&
@@ -3015,8 +3019,6 @@ void VFTableBuilder::AddMethods(BaseSubo
 
     // If we got here, MD is a method not seen in any of the sub-bases or
     // it requires return adjustment. Insert the method info for this method.
-    unsigned VBIndex =
-        LastVBase ? VTables.getVBTableIndex(MostDerivedClass, LastVBase) : 0;
     MethodInfo MI(VBIndex,
                   HasRTTIComponent ? Components.size() - 1 : Components.size(),
                   ReturnAdjustingThunk);

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp?rev=312017&r1=312016&r2=312017&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp Tue Aug 29 10:40:04 2017
@@ -201,3 +201,18 @@ D::D() {}
 // GLOBALS: @"\01?fn at D@test3@@$4PPPPPPPM at A@AEPAUB at 2@XZ"
 // GLOBALS: @"\01?fn at D@test3@@$4PPPPPPPM at A@AEPAU12 at XZ"
 }
+
+namespace pr34302 {
+// C::f is lives in the vftable inside its virtual B subobject. In the MS ABI,
+// covariant return type virtual methods extend vftables from virtual bases,
+// even though that can make it impossible to implement certain diamond
+// hierarchies correctly.
+struct A { virtual ~A(); };
+struct B : A { virtual B *f(); };
+struct C : virtual B { C *f(); };
+C c;
+// VFTABLES-LABEL: VFTable indices for 'pr34302::C' (2 entries).
+// VFTABLES-NEXT:  -- accessible via vbtable index 1, vfptr at offset 0 --
+// VFTABLES-NEXT:    0 | pr34302::C::~C() [scalar deleting]
+// VFTABLES-NEXT:    2 | pr34302::C *pr34302::C::f()
+}