[MS ABI] NV bases may indirectly contain covariant thunks from V Bases

A class might contain multiple ways of getting to a vbase, some of which
are virtual and other non-virtual.  It may be the case that a
non-virtual base contains an override of a method in a vbase.  This
means that we must carefully pick between a set of nvbases to determine
which is the best.

As a consequence, the findPathForVPtr algorithm is considerably simpler.

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

diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp
index 903341c5a1b288d852ad048970b62cb19a270d55..de0c11a249fc0066a8beb2453192e446030910cf 100644 (file)
--- a/lib/AST/VTableBuilder.cpp
+++ b/lib/AST/VTableBuilder.cpp
@@ -3443,10 +3443,10 @@ MicrosoftVTableContext::~MicrosoftVTableContext() {
 }
 
 /// Find the full path of bases from the most derived class to the base class
-/// containing the vptr described by Info. Use depth-first search for this, but
-/// search non-virtual bases before virtual bases. This is important in cases
-/// like this where we need to find the path to a vbase that goes through an
-/// nvbase:
+/// containing the vptr described by Info. Utilize final overriders to detect
+/// vftable slots gained through covariant overriders on virtual base paths.
+/// This is important in cases like this where we need to find the path to a
+/// vbase that goes through an nvbase:
 ///   struct A { virtual void f(); }
 ///   struct B : virtual A { virtual void f(); };
 ///   struct C : virtual A, B { virtual void f(); };
@@ -3474,29 +3474,11 @@ static bool findPathForVPtr(ASTContext &Context,
     return false;
   };
 
-  // Start with the non-virtual bases so that we correctly create paths to
-  // virtual bases *through* non-virtual bases.
-  for (const auto &B : RD->bases()) {
-    if (B.isVirtual())
-      continue;
-    const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl();
-    CharUnits NewOffset = Offset + Layout.getBaseClassOffset(Base);
-    if (Recurse(Base, NewOffset))
-      return true;
-  }
-  // None of non-virtual bases got us to the BaseWithVPtr, we need to try the
-  // virtual bases.
-  std::set<std::pair<const CXXRecordDecl *, CharUnits>> VBases;
-  for (const auto &B : RD->bases()) {
-    if (!B.isVirtual())
-      continue;
-    const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl();
-    CharUnits NewOffset = MostDerivedLayout.getVBaseClassOffset(Base);
-    VBases.insert(std::make_pair(Base, NewOffset));
-  }
-  // No virtual bases, bail out early.
-  if (VBases.empty())
-    return false;
+  auto GetBaseOffset = [&](const CXXBaseSpecifier &BS) {
+    const CXXRecordDecl *Base = BS.getType()->getAsCXXRecordDecl();
+    return BS.isVirtual() ? MostDerivedLayout.getVBaseClassOffset(Base)
+                          : Offset + Layout.getBaseClassOffset(Base);
+  };
 
   CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/false,
                      /*DetectVirtual=*/true);
@@ -3521,8 +3503,8 @@ static bool findPathForVPtr(ASTContext &Context,
     // provides a return adjusting overrider for this method.
     const CXXRecordDecl *Base = nullptr;
     CharUnits NewOffset;
-    for (auto VBasePair : VBases) {
-      const CXXRecordDecl *VBase = VBasePair.first;
+    for (const auto &B : RD->bases()) {
+      const CXXRecordDecl *VBase = B.getType()->getAsCXXRecordDecl();
       // There might be a vbase which derives from a vbase which provides a
       // covariant override for the method *and* provides its own covariant
       // override.
@@ -3534,7 +3516,10 @@ static bool findPathForVPtr(ASTContext &Context,
           !Paths.getDetectedVirtual())
         continue;
       const CXXMethodDecl *VBaseMD = MD->getCorrespondingMethodInClass(VBase);
-      CharUnits VBaseNewOffset = VBasePair.second;
+      // Skip the base if it does not have an override of this method.
+      if (VBaseMD == MD)
+        continue;
+      CharUnits VBaseNewOffset = GetBaseOffset(B);
       Overrider = Overriders.getOverrider(VBaseMD, VBaseNewOffset);
       BO = ComputeReturnAdjustmentBaseOffset(Context, Overrider.Method, MD);
       // Skip any overriders which are not return adjusting.
@@ -3547,15 +3532,14 @@ static bool findPathForVPtr(ASTContext &Context,
     if (Base && Recurse(Base, NewOffset))
       return true;
   }
-  // There are no virtual bases listed as a base specifier which provides a
-  // covariant override.  However, we may still need to go through the virtual
-  // base to get to BaseWithVPtr.
-  for (const auto &B : VBases) {
-    const CXXRecordDecl *Base = B.first;
-    CharUnits NewOffset = B.second;
+
+  for (const auto &B : RD->bases()) {
+    const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl();
+    CharUnits NewOffset = GetBaseOffset(B);
     if (Recurse(Base, NewOffset))
       return true;
   }
+
   return false;
 }
 
@@ -3564,10 +3548,11 @@ static void computeFullPathsForVFTables(ASTContext &Context,
                                         VPtrInfoVector &Paths) {
   const ASTRecordLayout &MostDerivedLayout = Context.getASTRecordLayout(RD);
   VPtrInfo::BasePath FullPath;
-  FinalOverriders Overriders(RD, CharUnits(), RD);
+  FinalOverriders Overriders(RD, CharUnits::Zero(), RD);
   for (VPtrInfo *Info : Paths) {
-    findPathForVPtr(Context, MostDerivedLayout, RD, CharUnits::Zero(),
-                    Overriders, FullPath, Info);
+    if (!findPathForVPtr(Context, MostDerivedLayout, RD, CharUnits::Zero(),
+                         Overriders, FullPath, Info))
+      llvm_unreachable("no path for vptr!");
     FullPath.clear();
   }
 }

diff --git a/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp b/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
index 36c6434d549a8edee56212fa13b9ec37421ff04c..741f85cb52f4e5e6a02dfef355abeb69585f9b0d 100644 (file)
--- a/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
+++ b/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
@@ -155,3 +155,20 @@ C::C() {}
 // GLOBALS: @"\01?f@B@pr21073@@WPPPPPPPI@AEPAUA@2@XZ"
 // GLOBALS: @"\01?f@B@pr21073@@WPPPPPPPI@AEPAU12@XZ"
 }
+
+namespace pr21073_2 {
+struct A { virtual A *foo(); };
+struct B : virtual A {};
+struct C : virtual A { virtual C *foo(); };
+struct D : B, C { D(); };
+D::D() {}
+
+// VFTABLES-LABEL: VFTable for 'pr21073_2::A' in 'pr21073_2::C' in 'pr21073_2::D' (2 entries)
+// VFTABLES-NEXT:   0 | pr21073_2::C *pr21073_2::C::foo()
+// VFTABLES-NEXT:       [return adjustment (to type 'struct pr21073_2::A *'): vbase #1, 0 non-virtual]
+// VFTABLES-NEXT:   1 | pr21073_2::C *pr21073_2::C::foo()
+
+// GLOBALS-LABEL: @"\01??_7D@pr21073_2@@6B@" = {{.*}} constant [2 x i8*]
+// GLOBALS: @"\01?foo@C@pr21073_2@@QAEPAUA@2@XZ"
+// GLOBALS: @"\01?foo@C@pr21073_2@@UAEPAU12@XZ"
+}

diff --git a/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp b/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
index d600ebbf4c2f6b26cfef878ff9e2d3d00b86bd03..1a6797c80ecc3aafda05cbaba4f373c19d85bc27 100644 (file)
--- a/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
+++ b/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
@@ -423,7 +423,7 @@ void use(T *obj) { obj->f(); }
 
 namespace Test10 {
 struct X : virtual C, virtual A {
-  // CHECK-LABEL: VFTable for 'A' in 'Test10::X' (2 entries).
+  // CHECK-LABEL: VFTable for 'A' in 'C' in 'Test10::X' (2 entries).
   // CHECK-NEXT: 0 | void Test10::X::f()
   // CHECK-NEXT: 1 | void A::z()
 
@@ -782,7 +782,7 @@ struct B : virtual A { virtual void g(void); };
 struct C : virtual A, B { C(); };
 C::C() {}
 
-// CHECK-LABEL: VFTable for 'pr21031_1::A' in 'pr21031_1::B' in 'pr21031_1::C' (1 entry)
+// CHECK-LABEL: VFTable for 'pr21031_1::A' in 'pr21031_1::C' (1 entry)
 // CHECK-NEXT:   0 | void pr21031_1::A::f()
 
 // CHECK-LABEL: VFTable for 'pr21031_1::B' in 'pr21031_1::C' (1 entry)