Index: lib/AST/VTableBuilder.cpp =================================================================== --- lib/AST/VTableBuilder.cpp +++ lib/AST/VTableBuilder.cpp @@ -2715,27 +2715,45 @@ VBaseMap.find(WhichVFPtr.getVBaseWithVPtr()); assert(VBaseMapEntry != VBaseMap.end()); - // Check if we need a vtordisp adjustment at all. + // If there's no vtordisp, we don't need any vtordisp adjustment. if (!VBaseMapEntry->second.hasVtorDisp()) return; - CharUnits VFPtrVBaseOffset = VBaseMapEntry->second.VBaseOffset; + const CXXRecordDecl *OverriderRD = Overrider.Method->getParent(); + const CXXRecordDecl *OverriderVBase = 0; + if (OverriderRD != MostDerivedClass) { + OverriderVBase = + ComputeBaseOffset(Context, OverriderRD, MostDerivedClass).VirtualBase; + } + + // If the final overrider is defined in the same vbase as the initial + // declaration, we don't need a vtordisp thunk at all. + if (OverriderVBase == WhichVFPtr.getVBaseWithVPtr()) + return; + + // OK, now we know we need to use a vtordisp thunk. // The implicit vtordisp field is located right before the vbase. + CharUnits VFPtrVBaseOffset = VBaseMapEntry->second.VBaseOffset; TA.Virtual.Microsoft.VtordispOffset = (VFPtrVBaseOffset - WhichVFPtr.FullOffsetInMDC).getQuantity() - 4; - // If the final overrider is defined in either: - // - the most derived class or its non-virtual base or - // - the same vbase as the initial declaration, - // a simple vtordisp thunk will suffice. - const CXXRecordDecl *OverriderRD = Overrider.Method->getParent(); + // A simple vtordisp thunk will suffice if the final overrider is defined + // in either the most derived class or its non-virtual base. if (OverriderRD == MostDerivedClass) return; - const CXXRecordDecl *OverriderVBase = - ComputeBaseOffset(Context, OverriderRD, MostDerivedClass).VirtualBase; - if (!OverriderVBase || OverriderVBase == WhichVFPtr.getVBaseWithVPtr()) + if (!OverriderVBase) { + MethodVFTableLocation ML = VTables.getMethodVFTableLocation(Overrider.Method); + assert(ML.VBase && "why would we need a vtordisp if we can call the method " + "without a vfptr of a vbase?"); + // We need to offset the this parameter if the offset of the vbase is + // different between the overrider class and the most derived class. + const ASTRecordLayout &OverriderRDLayout = + Context.getASTRecordLayout(OverriderRD); + TA.NonVirtual += (OverriderRDLayout.getVBaseClassOffset(ML.VBase) + + ML.VFPtrOffset - ThisOffset).getQuantity(); return; + } // Otherwise, we need to do use the dynamic offset of the final overrider // in order to get "this" adjustment right. Index: test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp =================================================================== --- test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp +++ test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp @@ -157,6 +157,67 @@ C c; void use(C *obj) { obj->f(); } + +class D : B { + // CHECK-LABEL: VFTable for 'V2' in 'V3' in 'simple::B' in 'simple::D' (2 entries). + // CHECK-NEXT: 0 | void simple::B::f() + // CHECK-NEXT: [this adjustment: vtordisp at -12, -4 non-virtual] + // CHECK-NEXT: 1 | simple::D::~D() [scalar deleting] + // CHECK-NEXT: [this adjustment: vtordisp at -12, -8 non-virtual] + D(); + int z; + + // MANGLING-DAG: @"\01?f@B@simple@@$4PPPPPPPE@3AEXXZ" +}; + +D::D() {} + +struct E : V3 { + virtual void f(); +}; + +struct F : virtual E { + // CHECK-LABEL: VFTable for 'Z' in 'V3' in 'simple::E' in 'simple::F' (2 entries). + // CHECK-NEXT: 0 | void simple::F::g() + // CHECK-NEXT: [this adjustment: vtordisp at -4, 0 non-virtual] + // CHECK-NEXT: 1 | simple::F::~F() [scalar deleting] + // CHECK-NEXT: [this adjustment: vtordisp at -4, 0 non-virtual] + + // CHECK-LABEL: VFTable for 'V2' in 'V3' in 'simple::E' in 'simple::F' (2 entries). + // CHECK-NEXT: 0 | void simple::E::f() + // CHECK-NEXT: 1 | simple::F::~F() [scalar deleting] + // CHECK-NEXT: [this adjustment: vtordisp at -12, -8 non-virtual] + + F(); + virtual void g(); // Force a vtordisp. + int f; + + // MANGLING-DAG: @"\01?g@F@simple@@$4PPPPPPPM@A@AEXXZ"{{.*}}??_EF@simple@@$4PPPPPPPM@A@AEPAXI@Z + // MANGLING-DAG: ?f@E@simple@@UAEXXZ{{.*}}??_EF@simple@@$4PPPPPPPE@7AEPAXI@Z +}; + +F::F() {} + +struct G : F { + // CHECK-LABEL: VFTable for 'Z' in 'V3' in 'simple::E' in 'simple::F' in 'simple::G' (2 entries). + // CHECK-NEXT: 0 | void simple::F::g() + // CHECK-NEXT: [this adjustment: vtordisp at -4, -4 non-virtual] + // CHECK-NEXT: 1 | simple::G::~G() [scalar deleting] + // CHECK-NEXT: [this adjustment: vtordisp at -4, 0 non-virtual] + + // CHECK-LABEL: VFTable for 'V2' in 'V3' in 'simple::E' in 'simple::F' in 'simple::G' (2 entries). + // CHECK-NEXT: 0 | void simple::E::f() + // CHECK-NEXT: 1 | simple::G::~G() [scalar deleting] + // CHECK-NEXT: [this adjustment: vtordisp at -12, -8 non-virtual] + + G(); + int g; + + // MANGLING-DAG: @"\01?g@F@simple@@$4PPPPPPPM@3AEXXZ"{{.*}}@"\01??_EG@simple@@$4PPPPPPPM@A@AEPAXI@Z" + // MANGLING-DAG: @"\01?f@E@simple@@UAEXXZ"{{.*}}@"\01??_EG@simple@@$4PPPPPPPE@7AEPAXI@Z" +}; + +G::G() {} } namespace extended { @@ -355,6 +416,40 @@ void use(A *obj) { delete obj; } } +namespace pr19408 { +// In this test, the vptr used to vcall D::f() is located in the A vbase. +// The offset of A in different in C and D, so the D vtordisp thunk should +// adjust "this" so C::f gets the right value. +struct A { + A(); + virtual void f(); + int a; +}; + +struct B : virtual A { + B(); + int b; +}; + +struct C : B { + C(); + virtual void f(); + int c; +}; + +struct D : C { + // CHECK-LABEL: VFTable for 'pr19408::A' in 'pr19408::B' in 'pr19408::C' in 'pr19408::D' (1 entry). + // CHECK-NEXT: 0 | void pr19408::C::f() + // CHECK-NEXT: [this adjustment: vtordisp at -4, -4 non-virtual] + + // MANGLING-DAG: @"\01?f@C@pr19408@@$4PPPPPPPM@3AEXXZ" + D(); + int d; +}; + +D::D() {} +} + namespace access { struct A { virtual ~A();