diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 7c89b7b47b0d..c292834f5465 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -207,10 +207,66 @@ void ASTRecordLayoutBuilder::LayoutNonVirtualBase(const CXXRecordDecl *RD) { assert(false && "Added same base offset more than once!"); } +void +ASTRecordLayoutBuilder::AddPrimaryVirtualBaseOffsets(const CXXRecordDecl *RD, + uint64_t Offset, + const CXXRecordDecl *MostDerivedClass) { + // We already have the offset for the primary base of the most derived class. + if (RD != MostDerivedClass) { + const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD); + const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase(); + + // If this is a primary virtual base and we haven't seen it before, add it. + if (PrimaryBase && Layout.getPrimaryBaseWasVirtual() && + !VBases.count(PrimaryBase)) + VBases.insert(std::make_pair(PrimaryBase, Offset)); + } + + for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(), + E = RD->bases_end(); I != E; ++I) { + assert(!I->getType()->isDependentType() && + "Cannot layout class with dependent bases."); + + const CXXRecordDecl *BaseDecl = + cast(I->getType()->getAs()->getDecl()); + + if (!BaseDecl->getNumVBases()) { + // This base isn't interesting since it doesn't have any virtual bases. + continue; + } + + // Compute the offset of this base. + uint64_t BaseOffset; + + if (I->isVirtual()) { + // If we don't know this vbase yet, don't visit it. It will be visited + // later. + if (!VBases.count(BaseDecl)) { + continue; + } + + // Check if we've already visited this base. + if (!VisitedVirtualBases.insert(BaseDecl)) + continue; + + // We want the vbase offset from the class we're currently laying out. + BaseOffset = VBases[BaseDecl]; + } else if (RD == MostDerivedClass) { + // We want the base offset from the class we're currently laying out. + assert(Bases.count(BaseDecl) && "Did not find base!"); + BaseOffset = Bases[BaseDecl]; + } else { + const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD); + BaseOffset = Offset + Layout.getBaseClassOffset(BaseDecl); + } + + AddPrimaryVirtualBaseOffsets(BaseDecl, BaseOffset, MostDerivedClass); + } +} + void ASTRecordLayoutBuilder::LayoutVirtualBases(const CXXRecordDecl *RD, - uint64_t Offset, - const CXXRecordDecl *MostDerivedClass) { + const CXXRecordDecl *MostDerivedClass) { const CXXRecordDecl *PrimaryBase; bool PrimaryBaseIsVirtual; @@ -223,14 +279,6 @@ ASTRecordLayoutBuilder::LayoutVirtualBases(const CXXRecordDecl *RD, PrimaryBaseIsVirtual = Layout.getPrimaryBaseWasVirtual(); } - // Check the primary base first. - if (PrimaryBase && PrimaryBaseIsVirtual && - VisitedVirtualBases.insert(PrimaryBase)) { - assert(!VBases.count(PrimaryBase) && "vbase offset already exists!"); - - VBases.insert(std::make_pair(PrimaryBase, Offset)); - } - for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(), E = RD->bases_end(); I != E; ++I) { assert(!I->getType()->isDependentType() && @@ -259,27 +307,7 @@ ASTRecordLayoutBuilder::LayoutVirtualBases(const CXXRecordDecl *RD, continue; } - // Compute the offset of this base. - uint64_t BaseOffset; - - if (I->isVirtual()) { - // If we don't know this vbase yet, don't visit it. It will be visited - // later. - if (!VBases.count(Base)) - continue; - - // We want the vbase offset from the class we're currently laying out. - BaseOffset = VBases[Base]; - } else if (RD == MostDerivedClass) { - // We want the base offset from the class we're currently laying out. - assert(Bases.count(Base) && "Did not find base!"); - BaseOffset = Bases[Base]; - } else { - const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD); - BaseOffset = Offset + Layout.getBaseClassOffset(Base); - } - - LayoutVirtualBases(Base, BaseOffset, MostDerivedClass); + LayoutVirtualBases(Base, MostDerivedClass); } } @@ -501,9 +529,14 @@ void ASTRecordLayoutBuilder::Layout(const RecordDecl *D) { NonVirtualSize = Size; NonVirtualAlignment = Alignment; - // If this is a C++ class, lay out its virtual bases. - if (RD) - LayoutVirtualBases(RD, 0, RD); + // If this is a C++ class, lay out its virtual bases and add its primary + // virtual base offsets. + if (RD) { + LayoutVirtualBases(RD, RD); + + VisitedVirtualBases.clear(); + AddPrimaryVirtualBaseOffsets(RD, 0, RD); + } // Finally, round the size of the total struct up to the alignment of the // struct itself. diff --git a/clang/lib/AST/RecordLayoutBuilder.h b/clang/lib/AST/RecordLayoutBuilder.h index a4bce753126a..d0e336231c5a 100644 --- a/clang/lib/AST/RecordLayoutBuilder.h +++ b/clang/lib/AST/RecordLayoutBuilder.h @@ -112,8 +112,11 @@ class ASTRecordLayoutBuilder { /// LayoutNonVirtualBase - Lays out a single non-virtual base. void LayoutNonVirtualBase(const CXXRecordDecl *RD); + void AddPrimaryVirtualBaseOffsets(const CXXRecordDecl *RD, uint64_t Offset, + const CXXRecordDecl *MostDerivedClass); + /// LayoutVirtualBases - Lays out all the virtual bases. - void LayoutVirtualBases(const CXXRecordDecl *RD, uint64_t Offset, + void LayoutVirtualBases(const CXXRecordDecl *RD, const CXXRecordDecl *MostDerivedClass); /// LayoutVirtualBase - Lays out a single virtual base. diff --git a/clang/test/CodeGenCXX/vtable-layout.cpp b/clang/test/CodeGenCXX/vtable-layout.cpp index 184b95818e01..692e8deef351 100644 --- a/clang/test/CodeGenCXX/vtable-layout.cpp +++ b/clang/test/CodeGenCXX/vtable-layout.cpp @@ -1511,3 +1511,90 @@ struct F : E { void F::f() { } } + +namespace Test35 { + +// Test that we lay out the virtual bases of 'Test35::H' in the correct order. + +struct A { + virtual void a(); + + int i; +}; + +struct B : virtual A { + virtual void b(); +}; + +struct C { + virtual void c(); +}; + +struct D : C, virtual B { + virtual void d(); +}; + +struct E : D { + virtual void e(); + + bool b; +}; + +struct F : virtual D { }; +struct G : virtual E { }; + +// CHECK: Vtable for 'Test35::H' (32 entries). +// CHECK-NEXT: 0 | vbase_offset (32) +// CHECK-NEXT: 1 | vbase_offset (0) +// CHECK-NEXT: 2 | vcall_offset (0) +// CHECK-NEXT: 3 | vcall_offset (0) +// CHECK-NEXT: 4 | vbase_offset (16) +// CHECK-NEXT: 5 | vbase_offset (8) +// CHECK-NEXT: 6 | offset_to_top (0) +// CHECK-NEXT: 7 | Test35::H RTTI +// CHECK-NEXT: -- (Test35::C, 0) vtable address -- +// CHECK-NEXT: -- (Test35::D, 0) vtable address -- +// CHECK-NEXT: -- (Test35::F, 0) vtable address -- +// CHECK-NEXT: -- (Test35::H, 0) vtable address -- +// CHECK-NEXT: 8 | void Test35::C::c() +// CHECK-NEXT: 9 | void Test35::D::d() +// CHECK-NEXT: 10 | void Test35::H::h() +// CHECK-NEXT: 11 | vbase_offset (0) +// CHECK-NEXT: 12 | vbase_offset (24) +// CHECK-NEXT: 13 | vcall_offset (0) +// CHECK-NEXT: 14 | vbase_offset (8) +// CHECK-NEXT: 15 | offset_to_top (-8) +// CHECK-NEXT: 16 | Test35::H RTTI +// CHECK-NEXT: -- (Test35::B, 8) vtable address -- +// CHECK-NEXT: -- (Test35::G, 8) vtable address -- +// CHECK-NEXT: 17 | void Test35::B::b() +// CHECK-NEXT: 18 | vcall_offset (0) +// CHECK-NEXT: 19 | offset_to_top (-16) +// CHECK-NEXT: 20 | Test35::H RTTI +// CHECK-NEXT: -- (Test35::A, 16) vtable address -- +// CHECK-NEXT: 21 | void Test35::A::a() +// CHECK-NEXT: 22 | vcall_offset (0) +// CHECK-NEXT: 23 | vcall_offset (0) +// CHECK-NEXT: 24 | vcall_offset (0) +// CHECK-NEXT: 25 | vbase_offset (-16) +// CHECK-NEXT: 26 | vbase_offset (-24) +// CHECK-NEXT: 27 | offset_to_top (-32) +// CHECK-NEXT: 28 | Test35::H RTTI +// CHECK-NEXT: -- (Test35::C, 32) vtable address -- +// CHECK-NEXT: -- (Test35::D, 32) vtable address -- +// CHECK-NEXT: -- (Test35::E, 32) vtable address -- +// CHECK-NEXT: 29 | void Test35::C::c() +// CHECK-NEXT: 30 | void Test35::D::d() +// CHECK-NEXT: 31 | void Test35::E::e() + +// CHECK: Virtual base offset offsets for 'Test35::H' (4 entries). +// CHECK-NEXT: Test35::A | -32 +// CHECK-NEXT: Test35::B | -24 +// CHECK-NEXT: Test35::D | -56 +// CHECK-NEXT: Test35::E | -64 +struct H : F, G { + virtual void h(); +}; +void H::h() { } + +}