[MS] Fix bug in method vfptr location code

We were assuming that vbtable indices were assigned in layout order in
our comparison, which is not the case. When a virtual method, such as
the destructor, appears in multiple vftables, the vftable that appears
first in object layout order is the one that points to the main
implementation of that method. The later vftables use thunks.

In this layout, we adjusted "this" in the main implementation by the
amount that is appropriate for 'B' instead of 'A', even though the main
implementation is found in D's vftable for A:

  struct A {
    virtual ~A() {}
  };
  struct B {
    virtual ~B() {}
  };
  struct C : virtual B {};
  struct D : virtual A, C {};

D's layout looks like:
   0 D subobject (empty)
   0 C base suboject
   8 A base subobject
  16 B base subobject

With this fix, we correctly adjust by -8 in D's deleting destructor
instead of -16.

Fixes PR36921.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328723 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp
index 3e7871e7bd7c9d7a44cbde30a590bf4b3a9d858d..a95791ce884f86a06d1c2de9d517a437819e9250 100644 (file)
--- a/lib/AST/VTableBuilder.cpp
+++ b/lib/AST/VTableBuilder.cpp
@@ -3544,6 +3544,19 @@ static void computeFullPathsForVFTables(ASTContext &Context,
   }
 }
 
+static bool
+vfptrIsEarlierInMDC(const ASTRecordLayout &Layout,
+                    const MicrosoftVTableContext::MethodVFTableLocation &LHS,
+                    const MicrosoftVTableContext::MethodVFTableLocation &RHS) {
+  CharUnits L = LHS.VFPtrOffset;
+  CharUnits R = RHS.VFPtrOffset;
+  if (LHS.VBase)
+    L += Layout.getVBaseClassOffset(LHS.VBase);
+  if (RHS.VBase)
+    R += Layout.getVBaseClassOffset(RHS.VBase);
+  return L < R;
+}
+
 void MicrosoftVTableContext::computeVTableRelatedInformation(
     const CXXRecordDecl *RD) {
   assert(RD->isDynamicClass());
@@ -3574,12 +3587,15 @@ void MicrosoftVTableContext::computeVTableRelatedInformation(
         EmptyAddressPointsMap);
     Thunks.insert(Builder.thunks_begin(), Builder.thunks_end());
 
+    const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
     for (const auto &Loc : Builder.vtable_locations()) {
-      GlobalDecl GD = Loc.first;
-      MethodVFTableLocation NewLoc = Loc.second;
-      auto M = NewMethodLocations.find(GD);
-      if (M == NewMethodLocations.end() || NewLoc < M->second)
-        NewMethodLocations[GD] = NewLoc;
+      auto Insert = NewMethodLocations.insert(Loc);
+      if (!Insert.second) {
+        const MethodVFTableLocation &NewLoc = Loc.second;
+        MethodVFTableLocation &OldLoc = Insert.first->second;
+        if (vfptrIsEarlierInMDC(Layout, NewLoc, OldLoc))
+          OldLoc = NewLoc;
+      }
     }
   }
 

diff --git a/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp b/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
index b34f20d3390d37a38f0e7dd35782a9c3de7757e3..212c8dc84ae3e5c2c4bf5069b6b55d3b402e3fd4 100644 (file)
--- a/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ b/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -538,3 +538,20 @@ D::D() : C() {}
 // CHECK:   %[[FIELD:.*]] = getelementptr inbounds i8, i8* %[[C_i8]], i32 8
 // CHECK:   call void @llvm.memset.p0i8.i32(i8* align 4 %[[FIELD]], i8 0, i32 4, i1 false)
 }
+
+namespace pr36921 {
+struct A {
+  virtual ~A() {}
+};
+struct B {
+  virtual ~B() {}
+};
+struct C : virtual B {};
+struct D : virtual A, C {};
+D d;
+// CHECK-LABEL: define linkonce_odr dso_local x86_thiscallcc i8* @"??_GD@pr36921@@UAEPAXI@Z"(
+// CHECK:   %[[THIS:.*]] = load %"struct.pr36921::D"*, %"struct.pr36921::D"**
+// CHECK:   %[[THIS_UNADJ_i8:.*]] = bitcast %"struct.pr36921::D"* %[[THIS_RELOAD]] to i8*
+// CHECK:   %[[THIS_ADJ_i8:.*]] = getelementptr inbounds i8, i8* %[[THIS_UNADJ_i8]], i32 -4
+// CHECK:   %[[THIS:.*]] = bitcast i8* %[[THIS_ADJ_i8]] to %"struct.pr36921::D"*
+}