[cfe-commits] r144072 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp lib/CodeGen/CGRecordLayoutBuilder.cpp test/Sema/ms_class_layout.cpp
John McCall
rjmccall at apple.com
Mon Nov 7 20:01:03 PST 2011
Author: rjmccall
Date: Mon Nov 7 22:01:03 2011
New Revision: 144072
URL: http://llvm.org/viewvc/llvm-project?rev=144072&view=rev
Log:
Fix the layout of vb-tables and vf-tables in the MS C++ ABI.
Based on work by Dmitry Sokolov!
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
cfe/trunk/test/Sema/ms_class_layout.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=144072&r1=144071&r2=144072&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Mon Nov 7 22:01:03 2011
@@ -618,10 +618,10 @@
/// avoid visiting virtual bases more than once.
llvm::SmallPtrSet<const CXXRecordDecl *, 4> VisitedVirtualBases;
- RecordLayoutBuilder(const ASTContext &Context, EmptySubobjectMap
- *EmptySubobjects, CharUnits Alignment)
+ RecordLayoutBuilder(const ASTContext &Context,
+ EmptySubobjectMap *EmptySubobjects)
: Context(Context), EmptySubobjects(EmptySubobjects), Size(0),
- Alignment(Alignment), UnpackedAlignment(Alignment),
+ Alignment(CharUnits::One()), UnpackedAlignment(CharUnits::One()),
Packed(false), IsUnion(false),
IsMac68kAlign(false), IsMsStruct(false),
UnfilledBitsInLastByte(0), MaxFieldAlignment(CharUnits::Zero()),
@@ -633,6 +633,17 @@
VBPtrOffset(CharUnits::fromQuantity(-1)),
FirstNearlyEmptyVBase(0) { }
+ /// Reset this RecordLayoutBuilder to a fresh state, using the given
+ /// alignment as the initial alignment. This is used for the
+ /// correct layout of vb-table pointers in MSVC.
+ void resetWithTargetAlignment(CharUnits TargetAlignment) {
+ const ASTContext &Context = this->Context;
+ EmptySubobjectMap *EmptySubobjects = this->EmptySubobjects;
+ this->~RecordLayoutBuilder();
+ new (this) RecordLayoutBuilder(Context, EmptySubobjects);
+ Alignment = UnpackedAlignment = TargetAlignment;
+ }
+
void Layout(const RecordDecl *D);
void Layout(const CXXRecordDecl *D);
void Layout(const ObjCInterfaceDecl *D);
@@ -642,8 +653,12 @@
void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize,
bool FieldPacked, const FieldDecl *D);
void LayoutBitField(const FieldDecl *D);
+
+ bool isMicrosoftCXXABI() const {
+ return Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft;
+ }
+
void MSLayoutVirtualBases(const CXXRecordDecl *RD);
- void MSLayout(const CXXRecordDecl *RD);
/// BaseSubobjectInfoAllocator - Allocator for BaseSubobjectInfo objects.
llvm::SpecificBumpPtrAllocator<BaseSubobjectInfo> BaseSubobjectInfoAllocator;
@@ -686,8 +701,9 @@
void AddPrimaryVirtualBaseOffsets(const BaseSubobjectInfo *Info,
CharUnits Offset);
- bool HasNewVirtualFunction(const CXXRecordDecl *RD) const;
- bool BaseHasVFPtr(const CXXRecordDecl *RD) const;
+ bool needsVFTable(const CXXRecordDecl *RD) const;
+ bool hasNewVirtualFunction(const CXXRecordDecl *RD) const;
+ bool isPossiblePrimaryBase(const CXXRecordDecl *Base) const;
/// LayoutVirtualBases - Lays out all the virtual bases.
void LayoutVirtualBases(const CXXRecordDecl *RD,
@@ -742,8 +758,6 @@
void operator=(const RecordLayoutBuilder&); // DO NOT IMPLEMENT
public:
static const CXXMethodDecl *ComputeKeyFunction(const CXXRecordDecl *RD);
-
- virtual ~RecordLayoutBuilder() { }
};
} // end anonymous namespace
@@ -800,7 +814,7 @@
const CXXRecordDecl *Base =
cast<CXXRecordDecl>(i->getType()->getAs<RecordType>()->getDecl());
- if (Base->isDynamicClass()) {
+ if (isPossiblePrimaryBase(Base)) {
// We found it.
PrimaryBase = Base;
PrimaryBaseIsVirtual = false;
@@ -809,7 +823,7 @@
}
// The Microsoft ABI doesn't have primary virtual bases.
- if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft) {
+ if (isMicrosoftCXXABI()) {
assert(!PrimaryBase && "Should not get here with a primary base!");
return;
}
@@ -989,34 +1003,45 @@
LayoutNonVirtualBase(PrimaryBaseInfo);
}
- }
- if (Context.getTargetInfo().getCXXABI() != CXXABI_Microsoft &&
- !PrimaryBase && RD->isDynamicClass()) {
- // Under the Itanium ABI, a dynamic class without a primary base has a
- // vtable pointer. It is placed at offset 0.
+ // If this class needs a vtable/vf-table and didn't get one from a
+ // primary base, add it in now.
+ } else if (needsVFTable(RD)) {
assert(DataSize == 0 && "Vtable pointer must be at offset zero!");
CharUnits PtrWidth =
Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
CharUnits PtrAlign =
Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerAlign(0));
EnsureVTablePointerAlignment(PtrAlign);
+ if (isMicrosoftCXXABI())
+ VFPtrOffset = getSize();
setSize(getSize() + PtrWidth);
setDataSize(getSize());
}
+ bool HasDirectVirtualBases = false;
+ bool HasNonVirtualBaseWithVBTable = false;
+
// Now lay out the non-virtual bases.
for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
E = RD->bases_end(); I != E; ++I) {
- // Ignore virtual bases.
- if (I->isVirtual())
+ // Ignore virtual bases, but remember that we saw one.
+ if (I->isVirtual()) {
+ HasDirectVirtualBases = true;
continue;
+ }
const CXXRecordDecl *BaseDecl =
- cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
+ cast<CXXRecordDecl>(I->getType()->castAs<RecordType>()->getDecl());
- // Skip the primary base.
+ // Remember if this base has virtual bases itself.
+ if (BaseDecl->getNumVBases())
+ HasNonVirtualBaseWithVBTable = true;
+
+ // Skip the primary base, because we've already laid it out. The
+ // !PrimaryBaseIsVirtual check is required because we might have a
+ // non-virtual base of the same type as a primary virtual base.
if (BaseDecl == PrimaryBase && !PrimaryBaseIsVirtual)
continue;
@@ -1027,32 +1052,35 @@
LayoutNonVirtualBase(BaseInfo);
}
- if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft) {
- // Under the MS ABI, there are separate virtual function table and
- // virtual base table pointers. A vfptr is necessary a if a class defines
- // a virtual function which is not overriding a function from a base;
- // a vbptr is necessary if a class has virtual bases. Either can come
- // from a primary base, if it exists. Otherwise, they are placed
- // after any base classes.
+ // In the MS ABI, add the vb-table pointer if we need one, which is
+ // whenever we have a virtual base and we can't re-use a vb-table
+ // pointer from a non-virtual base.
+ if (isMicrosoftCXXABI() &&
+ HasDirectVirtualBases && !HasNonVirtualBaseWithVBTable) {
CharUnits PtrWidth =
Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
CharUnits PtrAlign =
Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerAlign(0));
+
+ // MSVC potentially over-aligns the vb-table pointer by giving it
+ // the max alignment of all the non-virtual objects in the class.
+ // This is completely unnecessary, but we're not here to pass
+ // judgment.
+ //
+ // Note that we've only laid out the non-virtual bases, so on the
+ // first pass Alignment won't be set correctly here, but if the
+ // vb-table doesn't end up aligned correctly we'll come through
+ // and redo the layout from scratch with the right alignment.
+ //
+ // TODO: Instead of doing this, just lay out the fields as if the
+ // vb-table were at offset zero, then retroactively bump the field
+ // offsets up.
PtrAlign = std::max(PtrAlign, Alignment);
- if (HasNewVirtualFunction(RD) &&
- (!PrimaryBase || !BaseHasVFPtr(PrimaryBase))) {
- EnsureVTablePointerAlignment(PtrAlign);
- VFPtrOffset = getSize();
- setSize(getSize() + PtrWidth);
- setDataSize(getSize());
- }
- if (RD->getNumVBases() &&
- (!PrimaryBase || !PrimaryBase->getNumVBases())) {
- EnsureVTablePointerAlignment(PtrAlign);
- VBPtrOffset = getSize();
- setSize(getSize() + PtrWidth);
- setDataSize(getSize());
- }
+
+ EnsureVTablePointerAlignment(PtrAlign);
+ VBPtrOffset = getSize();
+ setSize(getSize() + PtrWidth);
+ setDataSize(getSize());
}
}
@@ -1102,28 +1130,81 @@
}
}
+/// needsVFTable - Return true if this class needs a vtable or vf-table
+/// when laid out as a base class. These are treated the same because
+/// they're both always laid out at offset zero.
+///
+/// This function assumes that the class has no primary base.
+bool RecordLayoutBuilder::needsVFTable(const CXXRecordDecl *RD) const {
+ assert(!PrimaryBase);
+
+ // In the Itanium ABI, every dynamic class needs a vtable: even if
+ // this class has no virtual functions as a base class (i.e. it's
+ // non-polymorphic or only has virtual functions from virtual
+ // bases),x it still needs a vtable to locate its virtual bases.
+ if (!isMicrosoftCXXABI())
+ return RD->isDynamicClass();
+
+ // In the MS ABI, we need a vfptr if the class has virtual functions
+ // other than those declared by its virtual bases. The AST doesn't
+ // tell us that directly, and checking manually for virtual
+ // functions that aren't overrides is expensive, but there are
+ // some important shortcuts:
+
+ // - Non-polymorphic classes have no virtual functions at all.
+ if (!RD->isPolymorphic()) return false;
+
+ // - Polymorphic classes with no virtual bases must either declare
+ // virtual functions directly or inherit them, but in the latter
+ // case we would have a primary base.
+ if (RD->getNumVBases() == 0) return true;
+
+ return hasNewVirtualFunction(RD);
+}
+
+/// hasNewVirtualFunction - Does the given polymorphic class declare a
+/// virtual function that does not override a method from any of its
+/// base classes?
bool
-RecordLayoutBuilder::HasNewVirtualFunction(const CXXRecordDecl *RD) const {
+RecordLayoutBuilder::hasNewVirtualFunction(const CXXRecordDecl *RD) const {
+ assert(RD->isPolymorphic());
+ if (!RD->getNumBases())
+ return true;
+
for (CXXRecordDecl::method_iterator method = RD->method_begin();
method != RD->method_end();
++method) {
- if (method->isVirtual() &&
- !method->size_overridden_methods()) {
+ if (method->isVirtual() && !method->size_overridden_methods()) {
return true;
}
}
return false;
}
+/// isPossiblePrimaryBase - Is the given base class an acceptable
+/// primary base class?
bool
-RecordLayoutBuilder::BaseHasVFPtr(const CXXRecordDecl *Base) const {
- // FIXME: This function is inefficient.
- if (HasNewVirtualFunction(Base))
- return true;
- const ASTRecordLayout &Layout = Context.getASTRecordLayout(Base);
- if (const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase())
- return BaseHasVFPtr(PrimaryBase);
- return false;
+RecordLayoutBuilder::isPossiblePrimaryBase(const CXXRecordDecl *Base) const {
+ // In the Itanium ABI, a class can be a primary base class if it has
+ // a vtable for any reason.
+ if (!isMicrosoftCXXABI())
+ return Base->isDynamicClass();
+
+ // In the MS ABI, a class can only be a primary base class if it
+ // provides a vf-table at a static offset. That means it has to be
+ // non-virtual base. The existence of a separate vb-table means
+ // that it's possible to get virtual functions only from a virtual
+ // base, which we have to guard against.
+
+ // First off, it has to have virtual functions.
+ if (!Base->isPolymorphic()) return false;
+
+ // If it has no virtual bases, then everything is at a static offset.
+ if (!Base->getNumVBases()) return true;
+
+ // Okay, just ask the base class's layout.
+ return (Context.getASTRecordLayout(Base).getVFPtrOffset()
+ != CharUnits::fromQuantity(-1));
}
void
@@ -1147,7 +1228,7 @@
"Cannot layout class with dependent bases.");
const CXXRecordDecl *BaseDecl =
- cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
+ cast<CXXRecordDecl>(I->getType()->castAs<RecordType>()->getDecl());
if (I->isVirtual()) {
if (PrimaryBase != BaseDecl || !PrimaryBaseIsVirtual) {
@@ -1175,6 +1256,23 @@
}
}
+void RecordLayoutBuilder::MSLayoutVirtualBases(const CXXRecordDecl *RD) {
+
+ if (!RD->getNumVBases())
+ return;
+
+ // This is substantially simplified because there are no virtual
+ // primary bases.
+ for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
+ E = RD->vbases_end(); I != E; ++I) {
+ const CXXRecordDecl *BaseDecl = I->getType()->getAsCXXRecordDecl();
+ const BaseSubobjectInfo *BaseInfo = VirtualBaseInfo.lookup(BaseDecl);
+ assert(BaseInfo && "Did not find virtual base info!");
+
+ LayoutVirtualBase(BaseInfo);
+ }
+}
+
void RecordLayoutBuilder::LayoutVirtualBase(const BaseSubobjectInfo *Base) {
assert(!Base->Derived && "Trying to lay out a primary virtual base!");
@@ -1269,11 +1367,6 @@
}
void RecordLayoutBuilder::Layout(const CXXRecordDecl *RD) {
- if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft) {
- MSLayout(RD);
- return;
- }
-
InitializeLayout(RD);
// Lay out the vtable and the non-virtual bases.
@@ -1286,14 +1379,30 @@
Context.getTargetInfo().getCharAlign()));
NonVirtualAlignment = Alignment;
- // Lay out the virtual bases and add the primary virtual base offsets.
- LayoutVirtualBases(RD, RD);
+ if (isMicrosoftCXXABI() &&
+ NonVirtualSize != NonVirtualSize.RoundUpToAlignment(Alignment)) {
+ CharUnits AlignMember =
+ NonVirtualSize.RoundUpToAlignment(Alignment) - NonVirtualSize;
- VisitedVirtualBases.clear();
+ setSize(getSize() + AlignMember);
+ setDataSize(getSize());
- // Finally, round the size of the total struct up to the alignment of the
- // struct itself.
- FinishLayout(RD);
+ NonVirtualSize = Context.toCharUnitsFromBits(
+ llvm::RoundUpToAlignment(getSizeInBits(),
+ Context.getTargetInfo().getCharAlign()));
+
+ MSLayoutVirtualBases(RD);
+
+ } else {
+ // Lay out the virtual bases and add the primary virtual base offsets.
+ LayoutVirtualBases(RD, RD);
+ }
+
+ // Finally, round the size of the total struct up to the alignment
+ // of the struct itself. Amazingly, this does not occur in the MS
+ // ABI after virtual base layout.
+ if (!isMicrosoftCXXABI() || RD->getNumVBases())
+ FinishLayout(RD);
#ifndef NDEBUG
// Check that we have base offsets for all bases.
@@ -1760,81 +1869,6 @@
UpdateAlignment(FieldAlign, UnpackedFieldAlign);
}
-void RecordLayoutBuilder::MSLayoutVirtualBases(const CXXRecordDecl *RD) {
-
- if (!RD->getNumVBases())
- return;
-
- for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
- E = RD->vbases_end(); I != E; ++I) {
-
- const CXXRecordDecl* BaseDecl = I->getType()->getAsCXXRecordDecl();
- const BaseSubobjectInfo* BaseInfo = VirtualBaseInfo.lookup(BaseDecl);
-
- assert(BaseInfo && "Did not find virtual base info!");
-
- LayoutVirtualBase(BaseInfo);
- }
-}
-
-void RecordLayoutBuilder::MSLayout(const CXXRecordDecl *RD) {
-
- InitializeLayout(RD);
-
- LayoutNonVirtualBases(RD);
-
- LayoutFields(RD);
-
- NonVirtualSize = Context.toCharUnitsFromBits(
- llvm::RoundUpToAlignment(getSizeInBits(),
- Context.getTargetInfo().getCharAlign()));
- NonVirtualAlignment = Alignment;
-
- if (NonVirtualSize != NonVirtualSize.RoundUpToAlignment(Alignment)) {
- CharUnits AlignMember =
- NonVirtualSize.RoundUpToAlignment(Alignment) - NonVirtualSize;
-
- setSize(getSize() + AlignMember);
- setDataSize(getSize());
-
- NonVirtualSize = Context.toCharUnitsFromBits(
- llvm::RoundUpToAlignment(getSizeInBits(),
- Context.getTargetInfo().getCharAlign()));
- }
-
- MSLayoutVirtualBases(RD);
-
- VisitedVirtualBases.clear();
-
- // Finally, round the size of the total struct up to the alignment of the
- // struct itself.
- if (!RD->getNumVBases())
- FinishLayout(RD);
-
-#ifndef NDEBUG
- // Check that we have base offsets for all bases.
- for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
- E = RD->bases_end(); I != E; ++I) {
- if (I->isVirtual())
- continue;
-
- const CXXRecordDecl *BaseDecl =
- cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
-
- assert(Bases.count(BaseDecl) && "Did not find base offset!");
- }
-
- // And all virtual bases.
- for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
- E = RD->vbases_end(); I != E; ++I) {
- const CXXRecordDecl *BaseDecl =
- cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
-
- assert(VBases.count(BaseDecl) && "Did not find base offset!");
- }
-#endif
-}
-
void RecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
// In C++, records cannot be of size 0.
if (Context.getLangOptions().CPlusPlus && getSizeInBits() == 0) {
@@ -2024,36 +2058,21 @@
if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
EmptySubobjectMap EmptySubobjects(*this, RD);
+ RecordLayoutBuilder Builder(*this, &EmptySubobjects);
+ Builder.Layout(RD);
- llvm::OwningPtr<RecordLayoutBuilder> Builder;
- CharUnits TargetAlign = CharUnits::One();
-
- Builder.reset(new RecordLayoutBuilder(*this,
- &EmptySubobjects,
- TargetAlign));
-
- // Recover resources if we crash before exiting this method.
- llvm::CrashRecoveryContextCleanupRegistrar<RecordLayoutBuilder>
- RecordBuilderCleanup(Builder.get());
-
- Builder->Layout(RD);
-
- TargetAlign = Builder->NonVirtualAlignment;
-
- if (getTargetInfo().getCXXABI() == CXXABI_Microsoft &&
- TargetAlign.getQuantity() > 4) {
- // MSVC rounds the vtable pointer to the struct alignment in what must
- // be a multi-pass operation. For now, let the builder figure out the
- // alignment and recalculate the layout once its known.
- Builder.reset(new RecordLayoutBuilder(*this,
- &EmptySubobjects,
- TargetAlign));
-
- Builder->Layout(RD);
-
- // Recover resources if we crash before exiting this method.
- llvm::CrashRecoveryContextCleanupRegistrar<RecordLayoutBuilder>
- RecordBuilderCleanup(Builder.get());
+ // MSVC gives the vb-table pointer an alignment equal to that of
+ // the non-virtual part of the structure. That's an inherently
+ // multi-pass operation. If our first pass doesn't give us
+ // adequate alignment, try again with the specified minimum
+ // alignment. This is *much* more maintainable than computing the
+ // alignment in advance in a separately-coded pass; it's also
+ // significantly more efficient in the common case where the
+ // vb-table doesn't need extra padding.
+ if (Builder.VBPtrOffset != CharUnits::fromQuantity(-1) &&
+ (Builder.VBPtrOffset % Builder.NonVirtualAlignment) != 0) {
+ Builder.resetWithTargetAlignment(Builder.NonVirtualAlignment);
+ Builder.Layout(RD);
}
// FIXME: This is not always correct. See the part about bitfields at
@@ -2061,31 +2080,30 @@
// FIXME: IsPODForThePurposeOfLayout should be stored in the record layout.
// This does not affect the calculations of MSVC layouts
bool IsPODForThePurposeOfLayout =
- (getTargetInfo().getCXXABI() != CXXABI_Microsoft) &&
- cast<CXXRecordDecl>(D)->isPOD();
+ (!Builder.isMicrosoftCXXABI() && cast<CXXRecordDecl>(D)->isPOD());
// FIXME: This should be done in FinalizeLayout.
CharUnits DataSize =
- IsPODForThePurposeOfLayout ? Builder->getSize() : Builder->getDataSize();
+ IsPODForThePurposeOfLayout ? Builder.getSize() : Builder.getDataSize();
CharUnits NonVirtualSize =
- IsPODForThePurposeOfLayout ? DataSize : Builder->NonVirtualSize;
+ IsPODForThePurposeOfLayout ? DataSize : Builder.NonVirtualSize;
NewEntry =
- new (*this) ASTRecordLayout(*this, Builder->getSize(),
- Builder->Alignment,
- Builder->VFPtrOffset,
- Builder->VBPtrOffset,
+ new (*this) ASTRecordLayout(*this, Builder.getSize(),
+ Builder.Alignment,
+ Builder.VFPtrOffset,
+ Builder.VBPtrOffset,
DataSize,
- Builder->FieldOffsets.data(),
- Builder->FieldOffsets.size(),
+ Builder.FieldOffsets.data(),
+ Builder.FieldOffsets.size(),
NonVirtualSize,
- Builder->NonVirtualAlignment,
+ Builder.NonVirtualAlignment,
EmptySubobjects.SizeOfLargestEmptySubobject,
- Builder->PrimaryBase,
- Builder->PrimaryBaseIsVirtual,
- Builder->Bases, Builder->VBases);
+ Builder.PrimaryBase,
+ Builder.PrimaryBaseIsVirtual,
+ Builder.Bases, Builder.VBases);
} else {
- RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0, CharUnits::One());
+ RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0);
Builder.Layout(D);
NewEntry =
@@ -2144,7 +2162,7 @@
return getObjCLayout(D, 0);
}
- RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0, CharUnits::One());
+ RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0);
Builder.Layout(D);
const ASTRecordLayout *NewEntry =
Modified: cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp?rev=144072&r1=144071&r2=144072&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp Mon Nov 7 22:01:03 2011
@@ -131,6 +131,11 @@
/// LayoutVirtualBases - layout the virtual bases of a record decl.
void LayoutVirtualBases(const CXXRecordDecl *RD,
const ASTRecordLayout &Layout);
+
+ /// MSLayoutVirtualBases - layout the virtual bases of a record decl,
+ /// like MSVC.
+ void MSLayoutVirtualBases(const CXXRecordDecl *RD,
+ const ASTRecordLayout &Layout);
/// LayoutNonVirtualBase - layout a single non-virtual base.
void LayoutNonVirtualBase(const CXXRecordDecl *base,
@@ -633,6 +638,25 @@
VirtualBases[base] = (FieldTypes.size() - 1);
}
+void
+CGRecordLayoutBuilder::MSLayoutVirtualBases(const CXXRecordDecl *RD,
+ const ASTRecordLayout &Layout) {
+ if (!RD->getNumVBases())
+ return;
+
+ // The vbases list is uniqued and ordered by a depth-first
+ // traversal, which is what we need here.
+ for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
+ E = RD->vbases_end(); I != E; ++I) {
+
+ const CXXRecordDecl *BaseDecl =
+ cast<CXXRecordDecl>(I->getType()->castAs<RecordType>()->getDecl());
+
+ CharUnits vbaseOffset = Layout.getVBaseClassOffset(BaseDecl);
+ LayoutVirtualBase(BaseDecl, vbaseOffset);
+ }
+}
+
/// LayoutVirtualBases - layout the non-virtual bases of a record decl.
void
CGRecordLayoutBuilder::LayoutVirtualBases(const CXXRecordDecl *RD,
@@ -667,25 +691,25 @@
const ASTRecordLayout &Layout) {
const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase();
- // Check if we need to add a vtable pointer.
- if (RD->isDynamicClass()) {
- if (PrimaryBase) {
- if (!Layout.isPrimaryBaseVirtual())
- LayoutNonVirtualBase(PrimaryBase, CharUnits::Zero());
- else
- LayoutVirtualBase(PrimaryBase, CharUnits::Zero());
- } else if (Types.getContext().getTargetInfo().getCXXABI() !=
- CXXABI_Microsoft) {
- llvm::Type *FunctionType =
- llvm::FunctionType::get(llvm::Type::getInt32Ty(Types.getLLVMContext()),
- /*isVarArg=*/true);
- llvm::Type *VTableTy = FunctionType->getPointerTo();
-
- assert(NextFieldOffset.isZero() &&
- "VTable pointer must come first!");
- AppendField(CharUnits::Zero(), VTableTy->getPointerTo());
-
- }
+ // If we have a primary base, lay it out first.
+ if (PrimaryBase) {
+ if (!Layout.isPrimaryBaseVirtual())
+ LayoutNonVirtualBase(PrimaryBase, CharUnits::Zero());
+ else
+ LayoutVirtualBase(PrimaryBase, CharUnits::Zero());
+
+ // Otherwise, add a vtable / vf-table if the layout says to do so.
+ } else if (Types.getContext().getTargetInfo().getCXXABI() == CXXABI_Microsoft
+ ? Layout.getVFPtrOffset() != CharUnits::fromQuantity(-1)
+ : RD->isDynamicClass()) {
+ llvm::Type *FunctionType =
+ llvm::FunctionType::get(llvm::Type::getInt32Ty(Types.getLLVMContext()),
+ /*isVarArg=*/true);
+ llvm::Type *VTableTy = FunctionType->getPointerTo();
+
+ assert(NextFieldOffset.isZero() &&
+ "VTable pointer must come first!");
+ AppendField(CharUnits::Zero(), VTableTy->getPointerTo());
}
// Layout the non-virtual bases.
@@ -703,6 +727,14 @@
LayoutNonVirtualBase(BaseDecl, Layout.getBaseClassOffset(BaseDecl));
}
+
+ // Add a vb-table pointer if the layout insists.
+ if (Layout.getVBPtrOffset() != CharUnits::fromQuantity(-1)) {
+ CharUnits VBPtrOffset = Layout.getVBPtrOffset();
+ llvm::Type *Vbptr = llvm::Type::getInt32PtrTy(Types.getLLVMContext());
+ AppendPadding(VBPtrOffset, getTypeAlignment(Vbptr));
+ AppendField(VBPtrOffset, Vbptr);
+ }
}
bool
@@ -733,7 +765,6 @@
CharUnits NumBytes = AlignedNonVirtualTypeSize - AlignedNextFieldOffset;
FieldTypes.push_back(getByteArrayType(NumBytes));
}
-
BaseSubobjectType = llvm::StructType::create(Types.getLLVMContext(),
FieldTypes, "", Packed);
@@ -787,11 +818,17 @@
return false;
}
- // And lay out the virtual bases.
- RD->getIndirectPrimaryBases(IndirectPrimaryBases);
- if (Layout.isPrimaryBaseVirtual())
- IndirectPrimaryBases.insert(Layout.getPrimaryBase());
- LayoutVirtualBases(RD, Layout);
+ // Lay out the virtual bases. The MS ABI uses a different
+ // algorithm here due to the lack of primary virtual bases.
+ if (Types.getContext().getTargetInfo().getCXXABI() != CXXABI_Microsoft) {
+ RD->getIndirectPrimaryBases(IndirectPrimaryBases);
+ if (Layout.isPrimaryBaseVirtual())
+ IndirectPrimaryBases.insert(Layout.getPrimaryBase());
+
+ LayoutVirtualBases(RD, Layout);
+ } else {
+ MSLayoutVirtualBases(RD, Layout);
+ }
}
// Append tail padding if necessary.
@@ -833,17 +870,24 @@
assert(NextFieldOffset <= fieldOffset &&
"Incorrect field layout!");
- // Round up the field offset to the alignment of the field type.
- CharUnits alignedNextFieldOffset =
- NextFieldOffset.RoundUpToAlignment(fieldAlignment);
-
- if (alignedNextFieldOffset < fieldOffset) {
- // Even with alignment, the field offset is not at the right place,
- // insert padding.
- CharUnits padding = fieldOffset - NextFieldOffset;
+ // Do nothing if we're already at the right offset.
+ if (fieldOffset == NextFieldOffset) return;
- AppendBytes(padding);
+ // If we're not emitting a packed LLVM type, try to avoid adding
+ // unnecessary padding fields.
+ if (!Packed) {
+ // Round up the field offset to the alignment of the field type.
+ CharUnits alignedNextFieldOffset =
+ NextFieldOffset.RoundUpToAlignment(fieldAlignment);
+ assert(alignedNextFieldOffset <= fieldOffset);
+
+ // If that's the right offset, we're done.
+ if (alignedNextFieldOffset == fieldOffset) return;
}
+
+ // Otherwise we need explicit padding.
+ CharUnits padding = fieldOffset - NextFieldOffset;
+ AppendBytes(padding);
}
bool CGRecordLayoutBuilder::ResizeLastBaseFieldIfNecessary(CharUnits offset) {
Modified: cfe/trunk/test/Sema/ms_class_layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/ms_class_layout.cpp?rev=144072&r1=144071&r2=144072&view=diff
==============================================================================
--- cfe/trunk/test/Sema/ms_class_layout.cpp (original)
+++ cfe/trunk/test/Sema/ms_class_layout.cpp Mon Nov 7 22:01:03 2011
@@ -67,6 +67,35 @@
double q;
};
+struct K
+{
+ int k;
+};
+
+struct L
+{
+ int l;
+};
+
+struct M : public virtual K
+{
+ int m;
+};
+
+struct N : public L, public M
+{
+ virtual void f(){}
+};
+
+struct O : public H, public G {
+ virtual void fo(){}
+};
+
+struct P : public M, public virtual L {
+ int p;
+};
+
+
#pragma pack(pop)
// This needs only for building layouts.
@@ -79,6 +108,9 @@
H* g;
BaseStruct* u;
I* i;
+ N* n;
+ O* o;
+ P* p;
return 0;
}
@@ -89,7 +121,7 @@
// CHECK-NEXT: sizeof=16, dsize=16, align=8
// CHECK-NEXT: nvsize=16, nvalign=8
-// CHECK: %class.D = type { [8 x i8], double }
+// CHECK: %class.D = type { i32 (...)**, double }
// CHECK: 0 | class B
// CHECK-NEXT: 0 | (B vftable pointer)
@@ -98,7 +130,7 @@
// CHECK-NEXT: sizeof=8, dsize=8, align=4
// CHECK-NEXT: nvsize=8, nvalign=4
-// CHECK: %class.B = type { [4 x i8], i32 }
+// CHECK: %class.B = type { i32 (...)**, i32 }
// CHECK: 0 | class A
// CHECK-NEXT: 0 | class B (primary base)
@@ -134,8 +166,8 @@
// CHECK: %class.A = type { %class.B, i32, i8 }
-// CHECK: %class.C = type { %class.D, %class.B, [8 x i8], double, i32, double, i32, [4 x i8], %class.A }
-// CHECK: %class.C.base = type { %class.D, %class.B, [8 x i8], double, i32, double, i32 }
+// CHECK: %class.C = type { %class.D, %class.B, i32*, double, i32, double, i32, [4 x i8], %class.A }
+// CHECK: %class.C.base = type { %class.D, %class.B, i32*, double, i32, double, i32 }
// CHECK: 0 | struct BaseStruct
// CHECK-NEXT: 0 | double v0
@@ -213,7 +245,7 @@
// CHECK-NEXT: sizeof=24, dsize=24, align=8
// CHECK-NEXT: nvsize=8, nvalign=4
-// CHECK: %struct.H = type { %struct.G, [4 x i8], %class.D }
+// CHECK: %struct.H = type { %struct.G, i32*, %class.D }
// CHECK: 0 | struct I
// CHECK-NEXT: 0 | (I vftable pointer)
@@ -225,5 +257,71 @@
// CHECK-NEXT: sizeof=40, dsize=40, align=8
// CHECK-NEXT: nvsize=24, nvalign=8
-// CHECK: %struct.I = type { [16 x i8], double, %class.D }
-// CHECK: %struct.I.base = type { [16 x i8], double }
+// CHECK: %struct.I = type { i32 (...)**, [4 x i8], i32*, double, %class.D }
+// CHECK: %struct.I.base = type { i32 (...)**, [4 x i8], i32*, double }
+
+// CHECK: 0 | struct L
+// CHECK-NEXT: 0 | int l
+// CHECK-NEXT: sizeof=4, dsize=4, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+// CHECK: 0 | struct K
+// CHECK-NEXT: 0 | int k
+// CHECK-NEXT: sizeof=4, dsize=4, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+// CHECK: 0 | struct M
+// CHECK-NEXT: 0 | (M vbtable pointer)
+// CHECK-NEXT: 4 | int m
+// CHECK-NEXT: 8 | struct K (virtual base)
+// CHECK-NEXT: 8 | int k
+// CHECK-NEXT: sizeof=12, dsize=12, align=4
+
+//CHECK: %struct.M = type { i32*, i32, %struct.K }
+//CHECK: %struct.M.base = type { i32*, i32 }
+
+// CHECK: 0 | struct N
+// CHECK-NEXT: 4 | struct L (base)
+// CHECK-NEXT: 4 | int l
+// CHECK-NEXT: 8 | struct M (base)
+// CHECK-NEXT: 8 | (M vbtable pointer)
+// CHECK-NEXT: 12 | int m
+// CHECK-NEXT: 0 | (N vftable pointer)
+// CHECK-NEXT: 16 | struct K (virtual base)
+// CHECK-NEXT: 16 | int k
+// CHECK-NEXT: sizeof=20, dsize=20, align=4
+// CHECK-NEXT: nvsize=16, nvalign=4
+
+//CHECK: %struct.N = type { i32 (...)**, %struct.L, %struct.M.base, %struct.K }
+
+// FIXME: MSVC place struct H at offset 8.
+// CHECK: 0 | struct O
+// CHECK-NEXT: 4 | struct H (base)
+// CHECK-NEXT: 4 | struct G (base)
+// CHECK-NEXT: 4 | int g_field
+// CHECK-NEXT: 8 | (H vbtable pointer)
+// CHECK-NEXT: 12 | struct G (base)
+// CHECK-NEXT: 12 | int g_field
+// CHECK-NEXT: 0 | (O vftable pointer)
+// CHECK-NEXT: 16 | class D (virtual base)
+// CHECK-NEXT: 16 | (D vftable pointer)
+// CHECK-NEXT: 24 | double a
+// CHECK-NEXT: sizeof=32, dsize=32, align=8
+// CHECK-NEXT: nvsize=16, nvalign=4
+
+//CHECK: %struct.O = type { i32 (...)**, %struct.H.base, %struct.G, %class.D }
+//CHECK: %struct.O.base = type { i32 (...)**, %struct.H.base, %struct.G }
+
+// CHECK: 0 | struct P
+// CHECK-NEXT: 0 | struct M (base)
+// CHECK-NEXT: 0 | (M vbtable pointer)
+// CHECK-NEXT: 4 | int m
+// CHECK-NEXT: 8 | int p
+// CHECK-NEXT: 12 | struct K (virtual base)
+// CHECK-NEXT: 12 | int k
+// CHECK-NEXT: 16 | struct L (virtual base)
+// CHECK-NEXT: 16 | int l
+// CHECK-NEXT: sizeof=20, dsize=20, align=4
+// CHECK-NEXT: nvsize=12, nvalign=4
+
+//CHECK: %struct.P = type { %struct.M.base, i32, %struct.K, %struct.L }
More information about the cfe-commits
mailing list