[cfe-commits] r101373 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp lib/AST/RecordLayoutBuilder.h test/CodeGenCXX/vtable-layout.cpp
Anders Carlsson
andersca at mac.com
Thu Apr 15 09:12:58 PDT 2010
Author: andersca
Date: Thu Apr 15 11:12:58 2010
New Revision: 101373
URL: http://llvm.org/viewvc/llvm-project?rev=101373&view=rev
Log:
Split adding the primary virtual base offsets out into a separate pass. This fixes a bug where we would lay out virtual bases in the wrong order.
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/lib/AST/RecordLayoutBuilder.h
cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=101373&r1=101372&r2=101373&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Apr 15 11:12:58 2010
@@ -208,9 +208,65 @@
}
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<CXXRecordDecl>(I->getType()->getAs<RecordType>()->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 @@
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 @@
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 @@
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.
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.h?rev=101373&r1=101372&r2=101373&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.h (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.h Thu Apr 15 11:12:58 2010
@@ -112,8 +112,11 @@
/// 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.
Modified: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-layout.cpp?rev=101373&r1=101372&r2=101373&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp Thu Apr 15 11:12:58 2010
@@ -1511,3 +1511,90 @@
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() { }
+
+}
More information about the cfe-commits
mailing list