From ce57dd5f9ff5bd6cebe3c94ae48eaf166ea50a23 Mon Sep 17 00:00:00 2001 From: Anders Carlsson Date: Wed, 3 Mar 2010 04:58:02 +0000 Subject: [PATCH] Fix a bug with base offset merging that Devang noticed. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@97641 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGVtable.cpp | 46 +++++------------------- test/CodeGenCXX/vtable-layout.cpp | 58 +++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 37 deletions(-) diff --git a/lib/CodeGen/CGVtable.cpp b/lib/CodeGen/CGVtable.cpp index ade5d3981e..932bd079e9 100644 --- a/lib/CodeGen/CGVtable.cpp +++ b/lib/CodeGen/CGVtable.cpp @@ -99,7 +99,8 @@ private: /// need to perform return value adjustments. AdjustmentOffsetsMapTy ReturnAdjustments; - typedef llvm::SmallVector OffsetVectorTy; + // FIXME: We might be able to get away with making this a SmallSet. + typedef llvm::SmallSetVector OffsetSetVectorTy; /// SubobjectOffsetsMapTy - This map is used for keeping track of all the /// base subobject offsets that a single class declaration might refer to. @@ -114,7 +115,7 @@ private: /// when we determine that C::f() overrides A::f(), we need to update the /// overriders map for both A-in-B1 and A-in-B2 and the subobject offsets map /// will have the subobject offsets for both A copies. - typedef llvm::DenseMap + typedef llvm::DenseMap SubobjectOffsetsMapTy; /// ComputeFinalOverriders - Compute the final overriders for a given base @@ -360,7 +361,7 @@ void FinalOverriders::PropagateOverrider(const CXXMethodDecl *OldMD, /// struct C : B1, B2 { virtual void f(); }; /// /// When overriding A::f with C::f we need to do so in both A subobjects. - const OffsetVectorTy &OffsetVector = Offsets[OverriddenRD]; + const OffsetSetVectorTy &OffsetVector = Offsets[OverriddenRD]; // Go through all the subobjects. for (unsigned I = 0, E = OffsetVector.size(); I != E; ++I) { @@ -404,41 +405,12 @@ FinalOverriders::MergeSubobjectOffsets(const SubobjectOffsetsMapTy &NewOffsets, for (SubobjectOffsetsMapTy::const_iterator I = NewOffsets.begin(), E = NewOffsets.end(); I != E; ++I) { const CXXRecordDecl *NewRD = I->first; - const OffsetVectorTy& NewOffsetVector = I->second; + const OffsetSetVectorTy& NewOffsetVector = I->second; - OffsetVectorTy &OffsetVector = Offsets[NewRD]; - if (OffsetVector.empty()) { - // There were no previous offsets in this vector, just insert all entries - // from the new offset vector. - OffsetVector.append(NewOffsetVector.begin(), NewOffsetVector.end()); - continue; - } - - // We need to merge the new offsets vector into the old, but we don't want - // to have duplicate entries. Do this by inserting the old offsets in a set - // so they'll be unique. After this, we iterate over the new offset vector - // and only append elements that aren't in the set. - - // First, add the existing offsets to the set. - llvm::SmallSet OffsetSet; - for (unsigned I = 0, E = OffsetVector.size(); I != E; ++I) { - bool Inserted = OffsetSet.insert(OffsetVector[I]); - if (!Inserted) - assert(false && "Set of offsets should be unique!"); - } + OffsetSetVectorTy &OffsetVector = Offsets[NewRD]; - // Next, only add the new offsets if they are not already in the set. - for (unsigned I = 0, E = NewOffsetVector.size(); I != E; ++I) { - uint64_t Offset = NewOffsetVector[I]; - - if (OffsetSet.count(Offset)) { - // Ignore the offset. - continue; - } - - // Otherwise, add it to the offsets vector. - OffsetVector.push_back(Offset); - } + // Merge the new offsets set vector into the old. + OffsetVector.insert(NewOffsetVector.begin(), NewOffsetVector.end()); } } @@ -503,7 +475,7 @@ void FinalOverriders::ComputeFinalOverriders(BaseSubobject Base, MergeSubobjectOffsets(NewOffsets, Offsets); /// Finally, add the offset for our own subobject. - Offsets[RD].push_back(Base.getBaseOffset()); + Offsets[RD].insert(Base.getBaseOffset()); } void FinalOverriders::dump(llvm::raw_ostream &Out, BaseSubobject Base) { diff --git a/test/CodeGenCXX/vtable-layout.cpp b/test/CodeGenCXX/vtable-layout.cpp index 022e49661f..a65af6ed33 100644 --- a/test/CodeGenCXX/vtable-layout.cpp +++ b/test/CodeGenCXX/vtable-layout.cpp @@ -978,3 +978,61 @@ struct D : B, C { void D::f() { } } + +namespace Test25 { + +// This mainly tests that we don't assert on this class hierarchy. + +struct V { + virtual void f(); +}; + +struct A : virtual V { }; +struct B : virtual V { }; + +// CHECK: Vtable for 'Test25::C' (11 entries). +// CHECK-NEXT: 0 | vbase_offset (0) +// CHECK-NEXT: 1 | vcall_offset (0) +// CHECK-NEXT: 2 | offset_to_top (0) +// CHECK-NEXT: 3 | Test25::C RTTI +// CHECK-NEXT: -- (Test25::A, 0) vtable address -- +// CHECK-NEXT: -- (Test25::C, 0) vtable address -- +// CHECK-NEXT: -- (Test25::V, 0) vtable address -- +// CHECK-NEXT: 4 | void Test25::V::f() +// CHECK-NEXT: 5 | void Test25::C::g() +// CHECK-NEXT: 6 | vbase_offset (-8) +// CHECK-NEXT: 7 | vcall_offset (-8) +// CHECK-NEXT: 8 | offset_to_top (-8) +// CHECK-NEXT: 9 | Test25::C RTTI +// CHECK-NEXT: -- (Test25::B, 8) vtable address -- +// CHECK-NEXT: -- (Test25::V, 8) vtable address -- +// CHECK-NEXT: 10 | [unused] void Test25::V::f() + +// CHECK: Construction vtable for ('Test25::A', 0) in 'Test25::C' (5 entries). +// CHECK-NEXT: 0 | vbase_offset (0) +// CHECK-NEXT: 1 | vcall_offset (0) +// CHECK-NEXT: 2 | offset_to_top (0) +// CHECK-NEXT: 3 | Test25::A RTTI +// CHECK-NEXT: -- (Test25::A, 0) vtable address -- +// CHECK-NEXT: -- (Test25::V, 0) vtable address -- +// CHECK-NEXT: 4 | void Test25::V::f() + +// CHECK: Construction vtable for ('Test25::B', 8) in 'Test25::C' (9 entries). +// CHECK-NEXT: 0 | vbase_offset (-8) +// CHECK-NEXT: 1 | vcall_offset (-8) +// CHECK-NEXT: 2 | offset_to_top (0) +// CHECK-NEXT: 3 | Test25::B RTTI +// CHECK-NEXT: -- (Test25::B, 8) vtable address -- +// CHECK-NEXT: -- (Test25::V, 8) vtable address -- +// CHECK-NEXT: 4 | [unused] void Test25::V::f() +// CHECK-NEXT: 5 | vcall_offset (0) +// CHECK-NEXT: 6 | offset_to_top (8) +// CHECK-NEXT: 7 | Test25::B RTTI +// CHECK-NEXT: -- (Test25::V, 0) vtable address -- +// CHECK-NEXT: 8 | void Test25::V::f() +struct C : A, virtual V, B { + virtual void g(); +}; +void C::g() { } + +} -- 2.40.0