[MS ABI] Detect and diagnose vftables which cannot be created

The MSVC ABI has a bug introduced by appending to the end of vftables
which come from virtual bases: covariant thunks introduces via
non-overlapping regions of the inheritance lattice both append to the
same slot in the vftable.

It is possible to generate correct vftables in cases where one node in
the lattice completely dominates the other on the way to the base with
the vfptr; in all other cases, we must raise a diagnostic in order to
prevent the illusion that we succeeded in laying out the vftable.

This fixes PR16759.

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

diff --git a/include/clang/Basic/DiagnosticASTKinds.td b/include/clang/Basic/DiagnosticASTKinds.td
index d353b451aaa72fb33b37360d6c02b8b0ec0dd8e1..0b37030d7eeca2cf4331be8f1cbde709cda03245 100644 (file)
--- a/include/clang/Basic/DiagnosticASTKinds.td
+++ b/include/clang/Basic/DiagnosticASTKinds.td
@@ -168,6 +168,14 @@ let CategoryName = "Inline Assembly Issue" in {
     "invalid operand number in inline asm string">;
 }
 
+// vtable related.
+let CategoryName = "VTable ABI Issue" in {
+  def err_vftable_ambiguous_component : Error<
+    "ambiguous vftable component for %0 introduced via covariant thunks; "
+    "this is an inherent limitation of the ABI">;
+  def note_covariant_thunk : Note<
+    "covariant thunk required by %0">;
+}
 
 // Importing ASTs
 def err_odr_variable_type_inconsistent : Error<

diff --git a/include/clang/Basic/DiagnosticIDs.h b/include/clang/Basic/DiagnosticIDs.h
index 99b469d70e7872cfefeb153bc55476f8f94e2aaf..a675dfabe46de0a8a7e5184eb92f6eade349298a 100644 (file)
--- a/include/clang/Basic/DiagnosticIDs.h
+++ b/include/clang/Basic/DiagnosticIDs.h
@@ -34,7 +34,7 @@ namespace clang {
       DIAG_START_LEX           = DIAG_START_SERIALIZATION   +  120,
       DIAG_START_PARSE         = DIAG_START_LEX             +  300,
       DIAG_START_AST           = DIAG_START_PARSE           +  500,
-      DIAG_START_COMMENT       = DIAG_START_AST             +  100,
+      DIAG_START_COMMENT       = DIAG_START_AST             +  110,
       DIAG_START_SEMA          = DIAG_START_COMMENT         +  100,
       DIAG_START_ANALYSIS      = DIAG_START_SEMA            + 3000,
       DIAG_UPPER_LIMIT         = DIAG_START_ANALYSIS        +  100

diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp
index de0c11a249fc0066a8beb2453192e446030910cf..5c7b9d2a6820fc60ad783d3f179fd374e876b110 100644 (file)
--- a/lib/AST/VTableBuilder.cpp
+++ b/lib/AST/VTableBuilder.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/AST/VTableBuilder.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/TargetInfo.h"
@@ -3482,9 +3483,24 @@ static bool findPathForVPtr(ASTContext &Context,
 
   CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/false,
                      /*DetectVirtual=*/true);
+#if 0
+  std::vector<const CXXRecordDecl *> BaseLists;
+  for (const auto &B : RD->bases()) {
+    const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl();
+    Paths.clear();
+    if (!Base->isDerivedFrom(Info->BaseWithVPtr, Paths))
+      continue;
+    for (const auto &Chain : BaseLists) {
+      const CXXRecordDecl *MostDerived = Chain.front();
+    }
+  }
+#endif
   // All virtual bases which are on the path to the BaseWithVPtr are not equal.
   // Specifically, virtual paths which introduce additional covariant thunks
   // must be preferred over paths which do not introduce such thunks.
+  const CXXRecordDecl *Base = nullptr;
+  CharUnits NewOffset;
+  const CXXMethodDecl *CovariantMD = nullptr;
   for (const auto *MD : Info->BaseWithVPtr->methods()) {
     if (!MD->isVirtual())
       continue;
@@ -3501,10 +3517,10 @@ static bool findPathForVPtr(ASTContext &Context,
 
     // Ok, let's iterate through our virtual bases looking for a base which
     // provides a return adjusting overrider for this method.
-    const CXXRecordDecl *Base = nullptr;
-    CharUnits NewOffset;
     for (const auto &B : RD->bases()) {
       const CXXRecordDecl *VBase = B.getType()->getAsCXXRecordDecl();
+      if (Base == VBase)
+        continue;
       // There might be a vbase which derives from a vbase which provides a
       // covariant override for the method *and* provides its own covariant
       // override.
@@ -3512,7 +3528,7 @@ static bool findPathForVPtr(ASTContext &Context,
       // looking for the most derived virtual base which provides a covariant
       // override for the method.
       Paths.clear();
-      if (!VBase->isDerivedFrom(Base ? Base : Info->BaseWithVPtr, Paths) ||
+      if (!VBase->isDerivedFrom(Info->BaseWithVPtr, Paths) ||
           !Paths.getDetectedVirtual())
         continue;
       const CXXMethodDecl *VBaseMD = MD->getCorrespondingMethodInClass(VBase);
@@ -3525,17 +3541,33 @@ static bool findPathForVPtr(ASTContext &Context,
       // Skip any overriders which are not return adjusting.
       if (BO.isEmpty() || !BO.VirtualBase)
         continue;
-      Base = VBase;
-      NewOffset = VBaseNewOffset;
+      Paths.clear();
+      if (!Base || VBase->isDerivedFrom(Base, Paths)) {
+        assert(!Base || Paths.getDetectedVirtual());
+        Base = VBase;
+        NewOffset = VBaseNewOffset;
+        CovariantMD = VBaseMD;
+      } else {
+        Paths.clear();
+        if (!Base->isDerivedFrom(VBase, Paths)) {
+          DiagnosticsEngine &Diags = Context.getDiagnostics();
+          Diags.Report(RD->getLocation(), diag::err_vftable_ambiguous_component)
+              << RD;
+          Diags.Report(CovariantMD->getLocation(), diag::note_covariant_thunk)
+              << CovariantMD;
+          Diags.Report(VBaseMD->getLocation(), diag::note_covariant_thunk)
+              << VBaseMD;
+        }
+      }
     }
-
-    if (Base && Recurse(Base, NewOffset))
-      return true;
   }
 
+  if (Base && Recurse(Base, NewOffset))
+    return true;
+
   for (const auto &B : RD->bases()) {
-    const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl();
-    CharUnits NewOffset = GetBaseOffset(B);
+    Base = B.getType()->getAsCXXRecordDecl();
+    NewOffset = GetBaseOffset(B);
     if (Recurse(Base, NewOffset))
       return true;
   }