Index: lib/AST/VTableBuilder.cpp =================================================================== --- lib/AST/VTableBuilder.cpp +++ lib/AST/VTableBuilder.cpp @@ -2649,6 +2649,8 @@ CharUnits Ret; bool First = true; + const ASTRecordLayout &OverriderRDLayout = + Context.getASTRecordLayout(Overrider.Method->getParent()); for (CXXBasePaths::paths_iterator I = Paths.begin(), E = Paths.end(); I != E; ++I) { const CXXBasePath &Path = (*I); @@ -2665,19 +2667,18 @@ const ASTRecordLayout &Layout = Context.getASTRecordLayout(PrevRD); if (Element.Base->isVirtual()) { - LastVBaseOffset = MostDerivedClassLayout.getVBaseClassOffset(CurRD); - if (Overrider.Method->getParent() == PrevRD) { - // This one's interesting. If the final overrider is in a vbase B of the - // most derived class and it overrides a method of the B's own vbase A, - // it uses A* as "this". In its prologue, it can cast A* to B* with - // a static offset. This offset is used regardless of the actual - // offset of A from B in the most derived class, requiring an - // this-adjusting thunk in the vftable if A and B are laid out - // differently in the most derived class. - ThisOffset += Layout.getVBaseClassOffset(CurRD); - } else { - ThisOffset = LastVBaseOffset; - } + // The interesting things begin when you have virtual inheritance. + // The final overrider will use a static adjustment equal to the offset + // of the vbase in the final overrider class. + // For example, if the final overrider is in a vbase B of the most + // derived class and it overrides a method of the B's own vbase A, + // it uses A* as "this". In its prologue, it can cast A* to B* with + // a static offset. This offset is used regardless of the actual + // offset of A from B in the most derived class, requiring an + // this-adjusting thunk in the vftable if A and B are laid out + // differently in the most derived class. + LastVBaseOffset = ThisOffset = + Overrider.Offset + OverriderRDLayout.getVBaseClassOffset(CurRD); } else { ThisOffset += Layout.getBaseClassOffset(CurRD); } @@ -2739,21 +2740,8 @@ // 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; - - 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(); + if (OverriderRD == MostDerivedClass || !OverriderVBase) 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.cpp =================================================================== --- test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp +++ test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp @@ -455,6 +455,35 @@ W w; } +namespace Test12 { +struct X : B, A { }; + +struct Y : X { + virtual void f(); // Overrides A::f. +}; + +struct Z : virtual Y { + // CHECK-LABEL: VFTable for 'A' in 'Test12::X' in 'Test12::Y' in 'Test12::Z' (2 entries). + // CHECK-NEXT: 0 | void Test12::Y::f() + // CHECK-NEXT: 1 | void A::z() + + int z; + // MANGLING-DAG: @"\01??_7Z@Test12@@6BA@@@" = {{.*}}@"\01?f@Y@Test12@@UAEXXZ" +}; + +struct W : Z { + // CHECK-LABEL: VFTable for 'A' in 'Test12::X' in 'Test12::Y' in 'Test12::Z' in 'Test12::W' (2 entries). + // CHECK-NEXT: 0 | void Test12::Y::f() + // CHECK-NEXT: 1 | void A::z() + W(); + + int w; + // MANGLING-DAG: @"\01??_7W@Test12@@6BA@@@" = {{.*}}@"\01?f@Y@Test12@@UAEXXZ" +}; + +W::W() {} +} + namespace vdtors { struct X { virtual ~X(); @@ -697,3 +726,41 @@ // MANGLING-DAG: @"\01??_7B@pr19240@@6B@" } + +namespace pr19408 { +// This test is a non-vtordisp version of the reproducer for PR19408. +struct X : virtual A { + int x; +}; + +struct Y : X { + virtual void f(); + int y; +}; + +struct Z : Y { + // CHECK-LABEL: VFTable for 'A' in 'pr19408::X' in 'pr19408::Y' in 'pr19408::Z' (2 entries). + // CHECK-NEXT: 0 | void pr19408::Y::f() + // CHECK-NEXT: [this adjustment: -4 non-virtual] + // CHECK-NEXT: 1 | void A::z() + + Z(); + int z; + // MANGLING-DAG: @"\01??_7Z@pr19408@@6B@" = {{.*}}@"\01?f@Y@pr19408@@W3AEXXZ" +}; + +Z::Z() {} + +struct W : B, Y { + // CHECK-LABEL: VFTable for 'A' in 'pr19408::X' in 'pr19408::Y' in 'pr19408::W' (2 entries). + // CHECK-NEXT: 0 | void pr19408::Y::f() + // CHECK-NEXT: [this adjustment: -4 non-virtual] + // CHECK-NEXT: 1 | void A::z() + + W(); + int w; + // MANGLING-DAG: @"\01??_7W@pr19408@@6BY@1@@" = {{.*}}@"\01?f@Y@pr19408@@W3AEXXZ" +}; + +W::W() {} +}