From: David Majnemer Date: Fri, 1 May 2015 21:35:41 +0000 (+0000) Subject: [MS ABI] NV bases may indirectly contain covariant thunks from V Bases X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=205632097d35d56fa911461686528ff91d46e259;p=clang [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 903341c5a1..de0c11a249 100644 --- 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> 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 36c6434d54..741f85cb52 100644 --- 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 d600ebbf4c..1a6797c80e 100644 --- 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)