r215312 - Fix PR20444 -- wrong number of vftable slots created when return adjustment thunks are needed
Timur Iskhodzhanov
timurrrr at google.com
Sun Aug 10 04:40:51 PDT 2014
Author: timurrrr
Date: Sun Aug 10 06:40:51 2014
New Revision: 215312
URL: http://llvm.org/viewvc/llvm-project?rev=215312&view=rev
Log:
Fix PR20444 -- wrong number of vftable slots created when return adjustment thunks are needed
Reviewed at http://reviews.llvm.org/D4822
Modified:
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp
Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=215312&r1=215311&r2=215312&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Sun Aug 10 06:40:51 2014
@@ -2464,11 +2464,18 @@ private:
/// or used for vcalls in the most derived class.
bool Shadowed;
- MethodInfo(uint64_t VBTableIndex, uint64_t VFTableIndex)
+ /// UsesExtraSlot - Indicates if this vftable slot was created because
+ /// any of the overridden slots required a return adjusting thunk.
+ bool UsesExtraSlot;
+
+ MethodInfo(uint64_t VBTableIndex, uint64_t VFTableIndex,
+ bool UsesExtraSlot = false)
: VBTableIndex(VBTableIndex), VFTableIndex(VFTableIndex),
- Shadowed(false) {}
+ Shadowed(false), UsesExtraSlot(UsesExtraSlot) {}
- MethodInfo() : VBTableIndex(0), VFTableIndex(0), Shadowed(false) {}
+ MethodInfo()
+ : VBTableIndex(0), VFTableIndex(0), Shadowed(false),
+ UsesExtraSlot(false) {}
};
typedef llvm::DenseMap<const CXXMethodDecl *, MethodInfo> MethodInfoMapTy;
@@ -2525,8 +2532,6 @@ private:
}
}
- bool NeedsReturnAdjustingThunk(const CXXMethodDecl *MD);
-
/// AddMethods - Add the methods of this base subobject and the relevant
/// subbases to the vftable we're currently laying out.
void AddMethods(BaseSubobject Base, unsigned BaseDepth,
@@ -2789,24 +2794,6 @@ static void GroupNewVirtualOverloads(
VirtualMethods.append(Groups[I].rbegin(), Groups[I].rend());
}
-/// We need a return adjusting thunk for this method if its return type is
-/// not trivially convertible to the return type of any of its overridden
-/// methods.
-bool VFTableBuilder::NeedsReturnAdjustingThunk(const CXXMethodDecl *MD) {
- OverriddenMethodsSetTy OverriddenMethods;
- ComputeAllOverriddenMethods(MD, OverriddenMethods);
- for (OverriddenMethodsSetTy::iterator I = OverriddenMethods.begin(),
- E = OverriddenMethods.end();
- I != E; ++I) {
- const CXXMethodDecl *OverriddenMD = *I;
- BaseOffset Adjustment =
- ComputeReturnAdjustmentBaseOffset(Context, MD, OverriddenMD);
- if (!Adjustment.isEmpty())
- return true;
- }
- return false;
-}
-
static bool isDirectVBase(const CXXRecordDecl *Base, const CXXRecordDecl *RD) {
for (const auto &B : RD->bases()) {
if (B.isVirtual() && B.getType()->getAsCXXRecordDecl() == Base)
@@ -2873,7 +2860,7 @@ void VFTableBuilder::AddMethods(BaseSubo
FindNearestOverriddenMethod(MD, VisitedBases);
ThisAdjustment ThisAdjustmentOffset;
- bool ReturnAdjustingThunk = false;
+ bool ReturnAdjustingThunk = false, ForceReturnAdjustmentMangling = false;
CharUnits ThisOffset = ComputeThisOffset(Overrider);
ThisAdjustmentOffset.NonVirtual =
(ThisOffset - WhichVFPtr.FullOffsetInMDC).getQuantity();
@@ -2892,7 +2879,16 @@ void VFTableBuilder::AddMethods(BaseSubo
MethodInfo &OverriddenMethodInfo = OverriddenMDIterator->second;
- if (!NeedsReturnAdjustingThunk(MD)) {
+ // 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.
+ // Once a chain of method overrides adds a return adjusting vftable slot,
+ // all subsequent overrides will also use an extra method slot.
+ ReturnAdjustingThunk = !ComputeReturnAdjustmentBaseOffset(
+ Context, MD, OverriddenMD).isEmpty() ||
+ OverriddenMethodInfo.UsesExtraSlot;
+
+ if (!ReturnAdjustingThunk) {
// No return adjustment needed - just replace the overridden method info
// with the current info.
MethodInfo MI(OverriddenMethodInfo.VBTableIndex,
@@ -2911,7 +2907,7 @@ void VFTableBuilder::AddMethods(BaseSubo
// Force a special name mangling for a return-adjusting thunk
// unless the method is the final overrider without this adjustment.
- ReturnAdjustingThunk =
+ ForceReturnAdjustmentMangling =
!(MD == OverriderMD && ThisAdjustmentOffset.isEmpty());
} else if (Base.getBaseOffset() != WhichVFPtr.FullOffsetInMDC ||
MD->size_overridden_methods()) {
@@ -2926,7 +2922,8 @@ void VFTableBuilder::AddMethods(BaseSubo
unsigned VBIndex =
LastVBase ? VTables.getVBTableIndex(MostDerivedClass, LastVBase) : 0;
MethodInfo MI(VBIndex,
- HasRTTIComponent ? Components.size() - 1 : Components.size());
+ HasRTTIComponent ? Components.size() - 1 : Components.size(),
+ ReturnAdjustingThunk);
assert(!MethodInfoMap.count(MD) &&
"Should not have method info for this method yet!");
@@ -2941,7 +2938,7 @@ void VFTableBuilder::AddMethods(BaseSubo
ComputeReturnAdjustmentBaseOffset(Context, OverriderMD, MD);
}
if (!ReturnAdjustmentOffset.isEmpty()) {
- ReturnAdjustingThunk = true;
+ ForceReturnAdjustmentMangling = true;
ReturnAdjustment.NonVirtual =
ReturnAdjustmentOffset.NonVirtualOffset.getQuantity();
if (ReturnAdjustmentOffset.VirtualBase) {
@@ -2955,8 +2952,9 @@ void VFTableBuilder::AddMethods(BaseSubo
}
}
- AddMethod(OverriderMD, ThunkInfo(ThisAdjustmentOffset, ReturnAdjustment,
- ReturnAdjustingThunk ? MD : nullptr));
+ AddMethod(OverriderMD,
+ ThunkInfo(ThisAdjustmentOffset, ReturnAdjustment,
+ ForceReturnAdjustmentMangling ? MD : nullptr));
}
}
Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp?rev=215312&r1=215311&r2=215312&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp Sun Aug 10 06:40:51 2014
@@ -295,3 +295,46 @@ struct X : E {
void build_vftable(X *obj) { obj->foo(); }
}
+
+namespace pr20444 {
+struct A {
+ virtual A* f();
+};
+struct B {
+ virtual B* f();
+};
+struct C : A, B {
+ virtual C* f();
+ // CHECK-LABEL: VFTable for 'pr20444::A' in 'pr20444::C' (1 entry).
+ // CHECK-NEXT: 0 | pr20444::C *pr20444::C::f()
+
+ // CHECK-LABEL: VFTable for 'pr20444::B' in 'pr20444::C' (2 entries).
+ // CHECK-NEXT: 0 | pr20444::C *pr20444::C::f()
+ // CHECK-NEXT: [return adjustment (to type 'struct pr20444::B *'): 4 non-virtual]
+ // CHECK-NEXT: [this adjustment: -4 non-virtual]
+ // CHECK-NEXT: 1 | pr20444::C *pr20444::C::f()
+ // CHECK-NEXT: [return adjustment (to type 'struct pr20444::C *'): 0 non-virtual]
+ // CHECK-NEXT: [this adjustment: -4 non-virtual]
+};
+
+void build_vftable(C *obj) { obj->f(); }
+
+struct D : C {
+ virtual D* f();
+ // CHECK-LABEL: VFTable for 'pr20444::A' in 'pr20444::C' in 'pr20444::D' (1 entry).
+ // CHECK-NEXT: 0 | pr20444::D *pr20444::D::f()
+
+ // CHECK-LABEL: VFTable for 'pr20444::B' in 'pr20444::C' in 'pr20444::D' (3 entries).
+ // CHECK-NEXT: 0 | pr20444::D *pr20444::D::f()
+ // CHECK-NEXT: [return adjustment (to type 'struct pr20444::B *'): 4 non-virtual]
+ // CHECK-NEXT: [this adjustment: -4 non-virtual]
+ // CHECK-NEXT: 1 | pr20444::D *pr20444::D::f()
+ // CHECK-NEXT: [return adjustment (to type 'struct pr20444::C *'): 0 non-virtual]
+ // CHECK-NEXT: [this adjustment: -4 non-virtual]
+ // CHECK-NEXT: 2 | pr20444::D *pr20444::D::f()
+ // CHECK-NEXT: [return adjustment (to type 'struct pr20444::D *'): 0 non-virtual]
+ // CHECK-NEXT: [this adjustment: -4 non-virtual]
+};
+
+void build_vftable(D *obj) { obj->f(); }
+}
More information about the cfe-commits
mailing list