diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp index aa2bc6910e23..f86697488121 100644 --- a/clang/lib/AST/VTableBuilder.cpp +++ b/clang/lib/AST/VTableBuilder.cpp @@ -3162,16 +3162,9 @@ void MicrosoftVTableContext::computeVTablePaths(bool ForVBTables, Paths.push_back(new VPtrInfo(RD)); // Recursive case: get all the vbtables from our bases and remove anything - // that shares a virtual base. Look at non-virtual bases first so we get - // longer inheritance paths from the derived class to the virtual bases. - llvm::SmallVector Bases; - std::copy_if(RD->bases_begin(), RD->bases_end(), std::back_inserter(Bases), - [](CXXBaseSpecifier bs) { return !bs.isVirtual(); }); - std::copy_if(RD->bases_begin(), RD->bases_end(), std::back_inserter(Bases), - [](CXXBaseSpecifier bs) { return bs.isVirtual(); }); - + // that shares a virtual base. llvm::SmallPtrSet VBasesSeen; - for (const auto &B : Bases) { + for (const auto &B : RD->bases()) { const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl(); if (B.isVirtual() && VBasesSeen.count(Base)) continue; @@ -3196,10 +3189,6 @@ void MicrosoftVTableContext::computeVTablePaths(bool ForVBTables, if (P->MangledPath.empty() || P->MangledPath.back() != Base) P->NextBaseToMangle = Base; - // Keep track of the full path. - // FIXME: Why do we need this? - P->PathToBaseWithVPtr.insert(P->PathToBaseWithVPtr.begin(), Base); - // Keep track of which vtable the derived class is going to extend with // new methods or bases. We append to either the vftable of our primary // base, or the first non-virtual base that has a vbtable. @@ -3287,6 +3276,59 @@ MicrosoftVTableContext::~MicrosoftVTableContext() { llvm::DeleteContainerSeconds(VBaseInfo); } +static bool +findPathForVPtr(ASTContext &Context, const ASTRecordLayout &MostDerivedLayout, + const CXXRecordDecl *RD, CharUnits Offset, + llvm::SmallPtrSetImpl &VBasesSeen, + VPtrInfo::BasePath &FullPath, VPtrInfo *Info) { + if (RD == Info->BaseWithVPtr && Offset == Info->FullOffsetInMDC) { + Info->PathToBaseWithVPtr = FullPath; + return true; + } + + const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); + + // Recurse with non-virtual bases first. + // FIXME: Does this need to be in layout order? Virtual bases will be in base + // specifier order, which isn't necessarily layout order. + SmallVector Bases(RD->bases_begin(), RD->bases_end()); + std::stable_partition(Bases.begin(), Bases.end(), + [](CXXBaseSpecifier bs) { return !bs.isVirtual(); }); + + for (const auto &B : Bases) { + const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl(); + CharUnits NewOffset; + if (!B.isVirtual()) + NewOffset = Offset + Layout.getBaseClassOffset(Base); + else { + if (VBasesSeen.count(Base)) + return false; + VBasesSeen.insert(Base); + NewOffset = MostDerivedLayout.getVBaseClassOffset(Base); + } + FullPath.push_back(Base); + if (findPathForVPtr(Context, MostDerivedLayout, Base, NewOffset, VBasesSeen, + FullPath, Info)) + return true; + FullPath.pop_back(); + } + return false; +} + +static void computeFullPathsForVFTables(ASTContext &Context, + const CXXRecordDecl *RD, + VPtrInfoVector &Paths) { + llvm::SmallPtrSet VBasesSeen; + const ASTRecordLayout &MostDerivedLayout = Context.getASTRecordLayout(RD); + VPtrInfo::BasePath FullPath; + for (VPtrInfo *Info : Paths) { + findPathForVPtr(Context, MostDerivedLayout, RD, CharUnits::Zero(), + VBasesSeen, FullPath, Info); + VBasesSeen.clear(); + FullPath.clear(); + } +} + void MicrosoftVTableContext::computeVTableRelatedInformation( const CXXRecordDecl *RD) { assert(RD->isDynamicClass()); @@ -3299,6 +3341,7 @@ void MicrosoftVTableContext::computeVTableRelatedInformation( VPtrInfoVector *VFPtrs = new VPtrInfoVector(); computeVTablePaths(/*ForVBTables=*/false, RD, *VFPtrs); + computeFullPathsForVFTables(Context, RD, *VFPtrs); VFPtrLocations[RD] = VFPtrs; MethodVFTableLocationsTy NewMethodLocations; diff --git a/clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp b/clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp index 2e4a056316c6..23129a3e2286 100644 --- a/clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp +++ b/clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp @@ -573,10 +573,6 @@ struct Q { // PR19172: Yet another diamond we miscompiled. struct R : virtual Q, X { - // CHECK-LABEL: VFTable for 'vdtors::X' in 'vdtors::R' (2 entries). - // CHECK-NEXT: 0 | vdtors::R::~R() [scalar deleting] - // CHECK-NEXT: 1 | void vdtors::X::zzz() - // CHECK-LABEL: VFTable for 'vdtors::Q' in 'vdtors::R' (1 entry). // CHECK-NEXT: 0 | vdtors::R::~R() [scalar deleting] // CHECK-NEXT: [this adjustment: -8 non-virtual] @@ -584,6 +580,10 @@ struct R : virtual Q, X { // CHECK-LABEL: Thunks for 'vdtors::R::~R()' (1 entry). // CHECK-NEXT: 0 | [this adjustment: -8 non-virtual] + // CHECK-LABEL: VFTable for 'vdtors::X' in 'vdtors::R' (2 entries). + // CHECK-NEXT: 0 | vdtors::R::~R() [scalar deleting] + // CHECK-NEXT: 1 | void vdtors::X::zzz() + // CHECK-LABEL: VFTable indices for 'vdtors::R' (1 entry). // CHECK-NEXT: 0 | vdtors::R::~R() [scalar deleting] virtual ~R(); @@ -694,10 +694,10 @@ struct X : virtual B {}; struct Y : virtual X, B { Y(); - // CHECK-LABEL: VFTable for 'B' in 'pr19066::Y' (1 entry). + // CHECK-LABEL: VFTable for 'B' in 'pr19066::X' in 'pr19066::Y' (1 entry). // CHECK-NEXT: 0 | void B::g() - // CHECK-LABEL: VFTable for 'B' in 'pr19066::X' in 'pr19066::Y' (1 entry). + // CHECK-LABEL: VFTable for 'B' in 'pr19066::Y' (1 entry). // CHECK-NEXT: 0 | void B::g() }; @@ -772,3 +772,36 @@ struct __declspec(dllexport) A { virtual void f() = delete; }; } + +namespace pr21031_1 { +// This ordering of base specifiers regressed in r202425. +struct A { virtual void f(void); }; +struct B : virtual A { virtual void g(void); }; +struct C : virtual A, B { C(); }; +C::C() {} + +// CHECK-LABEL: VFTable for 'pr21031_1::A' in 'pr21031_1::B' in 'pr21031_1::C' (1 entry) +// CHECK-NEXT: 0 | void pr21031_1::A::f() + +// CHECK-LABEL: VFTable for 'pr21031_1::B' in 'pr21031_1::C' (1 entry) +// CHECK-NEXT: 0 | void pr21031_1::B::g() + +// MANGLING-DAG: @"\01??_7C@pr21031_1@@6BB@1@@" = {{.*}} constant [1 x i8*] +// MANGLING-DAG: @"\01??_7C@pr21031_1@@6B@" = {{.*}} constant [1 x i8*] +} + +namespace pr21031_2 { +struct A { virtual void f(void); }; +struct B : virtual A { virtual void g(void); }; +struct C : B, virtual A { C(); }; +C::C() {} + +// CHECK-LABEL: VFTable for 'pr21031_2::B' in 'pr21031_2::C' (1 entry) +// CHECK-NEXT: 0 | void pr21031_2::B::g() + +// CHECK-LABEL: VFTable for 'pr21031_2::A' in 'pr21031_2::B' in 'pr21031_2::C' (1 entry) +// CHECK-NEXT: 0 | void pr21031_2::A::f() + +// MANGLING-DAG: @"\01??_7C@pr21031_2@@6BA@1@@" = {{.*}} constant [1 x i8*] +// MANGLING-DAG: @"\01??_7C@pr21031_2@@6BB@1@@" = {{.*}} constant [1 x i8*] +}