r236354 - [MS ABI] Detect and diagnose vftables which cannot be created
David Majnemer
david.majnemer at gmail.com
Fri May 1 14:35:45 PDT 2015
Author: majnemer
Date: Fri May 1 16:35:45 2015
New Revision: 236354
URL: http://llvm.org/viewvc/llvm-project?rev=236354&view=rev
Log:
[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.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/DiagnosticIDs.h
cfe/trunk/lib/AST/VTableBuilder.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=236354&r1=236353&r2=236354&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Fri May 1 16:35:45 2015
@@ -168,6 +168,14 @@ let CategoryName = "Inline Assembly Issu
"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<
Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=236354&r1=236353&r2=236354&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Fri May 1 16:35:45 2015
@@ -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
Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=236354&r1=236353&r2=236354&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Fri May 1 16:35:45 2015
@@ -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 &
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 &
// 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 &
// 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 &
// 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;
}
More information about the cfe-commits
mailing list