From: David Majnemer Date: Fri, 1 May 2015 21:35:45 +0000 (+0000) Subject: [MS ABI] Detect and diagnose vftables which cannot be created X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9a9c87baad47937aaa1f6ca78fa799ab31e970af;p=clang [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 d353b451aa..0b37030d7e 100644 --- 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 99b469d70e..a675dfabe4 100644 --- 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 de0c11a249..5c7b9d2a68 100644 --- 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 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; }