Index: lib/AST/VTableBuilder.cpp =================================================================== --- lib/AST/VTableBuilder.cpp +++ lib/AST/VTableBuilder.cpp @@ -2464,11 +2464,18 @@ /// 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 MethodInfoMapTy; @@ -2525,8 +2532,6 @@ } } - 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 @@ 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 @@ 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 @@ 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. + // Interestingly, a new vftable slot is also created if OverriddenMD + // itself required an extra slot due to return adjustment. + 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 @@ // 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 @@ 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 @@ ComputeReturnAdjustmentBaseOffset(Context, OverriderMD, MD); } if (!ReturnAdjustmentOffset.isEmpty()) { - ReturnAdjustingThunk = true; + ForceReturnAdjustmentMangling = true; ReturnAdjustment.NonVirtual = ReturnAdjustmentOffset.NonVirtualOffset.getQuantity(); if (ReturnAdjustmentOffset.VirtualBase) { @@ -2955,8 +2952,9 @@ } } - AddMethod(OverriderMD, ThunkInfo(ThisAdjustmentOffset, ReturnAdjustment, - ReturnAdjustingThunk ? MD : nullptr)); + AddMethod(OverriderMD, + ThunkInfo(ThisAdjustmentOffset, ReturnAdjustment, + ForceReturnAdjustmentMangling ? MD : nullptr)); } } Index: test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp =================================================================== --- test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp +++ test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp @@ -295,3 +295,46 @@ 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(); } +}