From b11c39f887e49e1501e7ffe84df6a3eb34655d67 Mon Sep 17 00:00:00 2001 From: Reid Kleckner <rnk@google.com> Date: Tue, 29 Aug 2017 17:40:04 +0000 Subject: [PATCH] [ms] Fix vbtable index for covariant overrides of vbase methods Overriding a method from a virtual base with a covariant return type consumes a slot from the vftable in the virtual base. This can make it impossible to implement certain diamond inheritance hierarchies, but we have to follow along for compatibility in the simple cases. This patch only affects our vtable dumper and member pointer function mangling, since all other callers of getMethodVFTableLocation seem to recompute VBTableIndex instead of using the one in the method location. Patch by David Majnemer git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@312017 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/VTableBuilder.cpp | 10 ++++++---- .../microsoft-abi-vtables-return-thunks.cpp | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp index e60ae33f2e5..ae8f6309fc6 100644 --- a/lib/AST/VTableBuilder.cpp +++ b/lib/AST/VTableBuilder.cpp @@ -2963,6 +2963,9 @@ void VFTableBuilder::AddMethods(BaseSubobject Base, unsigned BaseDepth, CalculateVtordispAdjustment(FinalOverrider, ThisOffset, ThisAdjustmentOffset); + unsigned VBIndex = + LastVBase ? VTables.getVBTableIndex(MostDerivedClass, LastVBase) : 0; + if (OverriddenMD) { // If MD overrides anything in this vftable, we need to update the // entries. @@ -2975,6 +2978,8 @@ void VFTableBuilder::AddMethods(BaseSubobject Base, unsigned BaseDepth, MethodInfo &OverriddenMethodInfo = OverriddenMDIterator->second; + VBIndex = OverriddenMethodInfo.VBTableIndex; + // Let's check if the overrider requires any return adjustments. // We must create a new slot if the MD's return type is not trivially // convertible to the OverriddenMD's one. @@ -2987,8 +2992,7 @@ void VFTableBuilder::AddMethods(BaseSubobject Base, unsigned BaseDepth, if (!ReturnAdjustingThunk) { // No return adjustment needed - just replace the overridden method info // with the current info. - MethodInfo MI(OverriddenMethodInfo.VBTableIndex, - OverriddenMethodInfo.VFTableIndex); + MethodInfo MI(VBIndex, OverriddenMethodInfo.VFTableIndex); MethodInfoMap.erase(OverriddenMDIterator); assert(!MethodInfoMap.count(MD) && @@ -3015,8 +3019,6 @@ void VFTableBuilder::AddMethods(BaseSubobject Base, unsigned BaseDepth, // If we got here, MD is a method not seen in any of the sub-bases or // it requires return adjustment. Insert the method info for this method. - unsigned VBIndex = - LastVBase ? VTables.getVBTableIndex(MostDerivedClass, LastVBase) : 0; MethodInfo MI(VBIndex, HasRTTIComponent ? Components.size() - 1 : Components.size(), ReturnAdjustingThunk); diff --git a/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp b/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp index 2bf7da58ea9..feba91c5055 100644 --- a/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp +++ b/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp @@ -201,3 +201,18 @@ D::D() {} // GLOBALS: @"\01?fn@D@test3@@$4PPPPPPPM@A@AEPAUB@2@XZ" // GLOBALS: @"\01?fn@D@test3@@$4PPPPPPPM@A@AEPAU12@XZ" } + +namespace pr34302 { +// C::f is lives in the vftable inside its virtual B subobject. In the MS ABI, +// covariant return type virtual methods extend vftables from virtual bases, +// even though that can make it impossible to implement certain diamond +// hierarchies correctly. +struct A { virtual ~A(); }; +struct B : A { virtual B *f(); }; +struct C : virtual B { C *f(); }; +C c; +// VFTABLES-LABEL: VFTable indices for 'pr34302::C' (2 entries). +// VFTABLES-NEXT: -- accessible via vbtable index 1, vfptr at offset 0 -- +// VFTABLES-NEXT: 0 | pr34302::C::~C() [scalar deleting] +// VFTABLES-NEXT: 2 | pr34302::C *pr34302::C::f() +} -- GitLab