r236353 - [MS ABI] NV bases may indirectly contain covariant thunks from V Bases
David Majnemer
david.majnemer at gmail.com
Fri May 1 14:35:41 PDT 2015
Author: majnemer
Date: Fri May 1 16:35:41 2015
New Revision: 236353
URL: http://llvm.org/viewvc/llvm-project?rev=236353&view=rev
Log:
[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.
Modified:
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=236353&r1=236352&r2=236353&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Fri May 1 16:35:41 2015
@@ -3443,10 +3443,10 @@ MicrosoftVTableContext::~MicrosoftVTable
}
/// 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 &
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 &
// 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 &
!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 &
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(
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();
}
}
Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp?rev=236353&r1=236352&r2=236353&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp Fri May 1 16:35:41 2015
@@ -155,3 +155,20 @@ C::C() {}
// GLOBALS: @"\01?f at B@pr21073@@WPPPPPPPI at AEPAUA@2 at XZ"
// GLOBALS: @"\01?f at B@pr21073@@WPPPPPPPI at 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 at pr21073_2@@6B@" = {{.*}} constant [2 x i8*]
+// GLOBALS: @"\01?foo at C@pr21073_2@@QAEPAUA at 2@XZ"
+// GLOBALS: @"\01?foo at C@pr21073_2@@UAEPAU12 at XZ"
+}
Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp?rev=236353&r1=236352&r2=236353&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp Fri May 1 16:35:41 2015
@@ -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(vo
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)
More information about the cfe-commits
mailing list