Hi majnemer, rsmith,
This patch adds appropriate padding to mimic the way that the MS-ABI attempts
to avoid aliasing among 0 sized bases.
This require adding two additional bits of state to be tracked, but fortunately
allows us to remove some other tracked state.
Also I went ahead and deleted the misaligned int64 code (it's been dead for a
while, that case is covered by the existing ms-bitfields code).
http://llvm-reviews.chandlerc.com/D2258
Files:
include/clang/AST/RecordLayout.h
lib/AST/RecordLayout.cpp
lib/AST/RecordLayoutBuilder.cpp
Index: include/clang/AST/RecordLayout.h
===================================================================
--- include/clang/AST/RecordLayout.h
+++ include/clang/AST/RecordLayout.h
@@ -103,7 +103,18 @@
/// AlignAfterVBases - Force appropriate alignment after virtual bases are
/// laid out in MS-C++-ABI.
bool AlignAfterVBases : 1;
-
+
+ /// HasZeroSizedMemberOrBase - True if this class contains a zero sized member or base or a base
+ /// with a zero sized member or base. Only used for MS-ABI.
+ bool HasZeroSizedMemberOrBase : 1;
+
+ /// \brief True if this class is zero sized or first base is zero sized or
+ /// has this property. Only used for MS-ABI.
+ bool LeftMostBaseIsZeroSized : 1;
+
+ /// \brief True if this record is empty according to the MS-ABI.
+ bool IsEmpty : 1;
+
/// PrimaryBase - The primary base info for this record.
llvm::PointerIntPair<const CXXRecordDecl *, 1, bool> PrimaryBase;
@@ -143,7 +154,10 @@
const CXXRecordDecl *PrimaryBase,
bool IsPrimaryBaseVirtual,
const CXXRecordDecl *BaseSharingVBPtr,
- bool ForceAlign,
+ bool alignAfterVBases,
+ bool hasZeroSizedMemberOrBase,
+ bool leftMostBaseIsZeroSized,
+ bool isEmpty,
const BaseOffsetsMapTy& BaseOffsets,
const VBaseOffsetsMapTy& VBaseOffsets);
@@ -272,6 +286,21 @@
return CXXInfo->AlignAfterVBases;
}
+ bool hasZeroSizedMemberOrBase() const {
+ assert(CXXInfo && "Record layout does not have C++ specific info!");
+ return CXXInfo->HasZeroSizedMemberOrBase;
+ }
+
+ bool leftMostBaseIsZeroSized() const {
+ assert(CXXInfo && "Record layout does not have C++ specific info!");
+ return CXXInfo->LeftMostBaseIsZeroSized;
+ }
+
+ bool isEmpty() const {
+ assert(CXXInfo && "Record layout does not have C++ specific info!");
+ return CXXInfo->IsEmpty;
+ }
+
/// getVBPtrOffset - Get the offset for virtual base table pointer.
/// This is only meaningful with the Microsoft ABI.
CharUnits getVBPtrOffset() const {
Index: lib/AST/RecordLayout.cpp
===================================================================
--- lib/AST/RecordLayout.cpp
+++ lib/AST/RecordLayout.cpp
@@ -54,7 +54,10 @@
const CXXRecordDecl *PrimaryBase,
bool IsPrimaryBaseVirtual,
const CXXRecordDecl *BaseSharingVBPtr,
- bool AlignAfterVBases,
+ bool alignAfterVBases,
+ bool hasZeroSizedMemberOrBase,
+ bool leftMostBaseIsZeroSized,
+ bool isEmpty,
const BaseOffsetsMapTy& BaseOffsets,
const VBaseOffsetsMapTy& VBaseOffsets)
: Size(size), DataSize(datasize), Alignment(alignment), FieldOffsets(0),
@@ -76,7 +79,10 @@
CXXInfo->VBPtrOffset = vbptroffset;
CXXInfo->HasExtendableVFPtr = hasExtendableVFPtr;
CXXInfo->BaseSharingVBPtr = BaseSharingVBPtr;
- CXXInfo->AlignAfterVBases = AlignAfterVBases;
+ CXXInfo->AlignAfterVBases = alignAfterVBases;
+ CXXInfo->HasZeroSizedMemberOrBase = hasZeroSizedMemberOrBase;
+ CXXInfo->LeftMostBaseIsZeroSized = leftMostBaseIsZeroSized;
+ CXXInfo->IsEmpty = isEmpty;
#ifndef NDEBUG
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -2019,6 +2019,12 @@
// obvious reason.
// * When laying out empty non-virtual bases, an extra byte of padding is added
// if the non-virtual base before the empty non-virtual base has a vbptr.
+// * The ABI attempts to avoid aliasing of zero sized bases by adding padding
+// between bases and vbases with specific properties. The criteria for
+// additional padding between two bases is that the first base is zero sized
+// or has a zero sized element as a member or base or any member or base has
+// that property and the second base is zero sized or has a zero sized base
+// as its first laid out base.
namespace {
@@ -2138,13 +2144,19 @@
CharUnits PointerAlignment;
/// \brief Holds an empty base we haven't yet laid out.
const CXXRecordDecl *LazyEmptyBase;
- /// \brief Lets us know if the last base we laid out was empty. Only used
- /// when adjusting the placement of a last zero-sized base in 64 bit mode.
- bool LastBaseWasEmpty;
+ /// \brief A pointer to the Layout for the most recent non-virtual base that
+ /// has been laid out.
+ const ASTRecordLayout* PreviousBaseLayout;
/// \brief Lets us know if we're in 64-bit mode
- bool Is64BitMode;
- /// \brief True if the last non-virtual base has a vbptr.
- bool LastNonVirtualBaseHasVBPtr;
+ bool Is64BitMode : 1;
+ /// \brief True if this class contains a zero sized member or base or a base
+ /// with a zero sized member or base. Only used for MS-ABI.
+ bool HasZeroSizedMemberOrBase : 1;
+ /// \brief True if this class is zero sized or first base is zero sized or
+ /// has this property. Only used for MS-ABI.
+ bool LeftMostBaseIsZeroSized : 1;
+ /// \brief True if this record is empty according to the MS-ABI.
+ bool IsEmpty : 1;
};
} // namespace
@@ -2153,18 +2165,6 @@
std::pair<CharUnits, CharUnits> FieldInfo =
Context.getTypeInfoInChars(FD->getType());
- // If we're not on win32 and using ms_struct the field alignment will be wrong
- // for 64 bit types, so we fix that here.
- if (FD->getASTContext().getTargetInfo().getTriple().getOS() !=
- llvm::Triple::Win32) {
- QualType T = Context.getBaseElementType(FD->getType());
- if (const BuiltinType *BTy = T->getAs<BuiltinType>()) {
- CharUnits TypeSize = Context.getTypeSizeInChars(BTy);
- if (TypeSize > FieldInfo.second)
- FieldInfo.second = TypeSize;
- }
- }
-
// Respect packed attribute.
if (FD->hasAttr<PackedAttr>())
FieldInfo.second = CharUnits::One();
@@ -2187,6 +2187,7 @@
Size = CharUnits::Zero();
Alignment = CharUnits::One();
AlignAfterVBases = false;
+ HasZeroSizedMemberOrBase = false;
// Compute the maximum field alignment.
MaxFieldAlignment = CharUnits::Zero();
@@ -2237,6 +2238,7 @@
PrimaryBase = 0;
VirtualAlignment = CharUnits::One();
AlignAfterVBases = Is64BitMode;
+ LeftMostBaseIsZeroSized = false;
// If the record has a dynamic base class, attempt to choose a primary base
// class. It is the first (in direct base class order) non-virtual dynamic
@@ -2250,6 +2252,8 @@
// Handle forced alignment.
if (Layout.getAlignAfterVBases())
AlignAfterVBases = true;
+ if (Layout.hasZeroSizedMemberOrBase())
+ HasZeroSizedMemberOrBase = true;
// Handle virtual bases.
if (i->isVirtual()) {
VirtualAlignment = std::max(VirtualAlignment, Layout.getAlignment());
@@ -2307,14 +2311,14 @@
void
MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
LazyEmptyBase = 0;
- LastBaseWasEmpty = false;
- LastNonVirtualBaseHasVBPtr = false;
+ PreviousBaseLayout = 0;
// Lay out the primary base first.
if (PrimaryBase)
layoutNonVirtualBase(PrimaryBase);
// Iterate through the bases and lay out the non-virtual ones.
+ bool LeftMostBase = true;
for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(),
e = RD->bases_end();
i != e; ++i) {
@@ -2322,8 +2326,14 @@
continue;
const CXXRecordDecl *BaseDecl =
cast<CXXRecordDecl>(i->getType()->castAs<RecordType>()->getDecl());
- if (BaseDecl != PrimaryBase)
+ if (BaseDecl != PrimaryBase) {
+ if (LeftMostBase) {
+ const ASTRecordLayout &Layout = Context.getASTRecordLayout(BaseDecl);
+ LeftMostBaseIsZeroSized = Layout.leftMostBaseIsZeroSized();
+ LeftMostBase = false;
+ }
layoutNonVirtualBase(BaseDecl);
+ }
}
}
@@ -2338,16 +2348,14 @@
Size = Size.RoundUpToAlignment(LazyLayout.getAlignment());
// If the last non-virtual base has a vbptr we add a byte of padding for no
// obvious reason.
- if (LastNonVirtualBaseHasVBPtr)
+ if (PreviousBaseLayout && PreviousBaseLayout->hasVBPtr())
Size++;
Bases.insert(std::make_pair(LazyEmptyBase, Size));
// Empty bases only consume space when followed by another empty base.
- if (RD && Layout->getNonVirtualSize().isZero()) {
- LastBaseWasEmpty = true;
+ if (RD && Layout->getNonVirtualSize().isZero())
Size++;
- }
LazyEmptyBase = 0;
- LastNonVirtualBaseHasVBPtr = false;
+ PreviousBaseLayout = &LazyLayout;
}
// RD is null when flushing the final lazy base.
@@ -2359,14 +2367,16 @@
return;
}
+ if (Layout->leftMostBaseIsZeroSized() && PreviousBaseLayout &&
+ PreviousBaseLayout->hasZeroSizedMemberOrBase())
+ Size++;
// Insert the base here.
CharUnits BaseOffset = Size.RoundUpToAlignment(Layout->getAlignment());
Bases.insert(std::make_pair(RD, BaseOffset));
Size = BaseOffset + Layout->getDataSize();
// Note: we don't update alignment here because it was accounted
// for during initalization.
- LastBaseWasEmpty = false;
- LastNonVirtualBaseHasVBPtr = Layout->hasVBPtr();
+ PreviousBaseLayout = Layout;
}
void MicrosoftRecordLayoutBuilder::layoutVBPtr(const CXXRecordDecl *RD) {
@@ -2389,7 +2399,8 @@
Size += OldSize % BasesAndFieldsAlignment.getQuantity();
} else {
if (Is64BitMode)
- Size += LastBaseWasEmpty ? CharUnits::One() : CharUnits::Zero();
+ Size += PreviousBaseLayout && PreviousBaseLayout->isEmpty()
+ ? CharUnits::One() : CharUnits::Zero();
else
Size = OldSize + BasesAndFieldsAlignment;
}
@@ -2416,6 +2427,12 @@
}
LastFieldIsNonZeroWidthBitfield = false;
+ if (const RecordType *RT =
+ FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) {
+ const ASTRecordLayout &Layout = Context.getASTRecordLayout(RT->getDecl());
+ if (Layout.hasZeroSizedMemberOrBase())
+ HasZeroSizedMemberOrBase = true;
+ }
std::pair<CharUnits, CharUnits> FieldInfo = getAdjustedFieldInfo(FD);
CharUnits FieldSize = FieldInfo.first;
CharUnits FieldAlign = FieldInfo.second;
@@ -2507,6 +2524,7 @@
return;
updateAlignment(VirtualAlignment);
+ PreviousBaseLayout = 0;
// Zero-sized v-bases obey the alignment attribute so apply it here. The
// alignment attribute is normally accounted for in FinalizeLayout.
@@ -2533,31 +2551,16 @@
void MicrosoftRecordLayoutBuilder::layoutVirtualBase(const CXXRecordDecl *RD,
bool HasVtordisp) {
- if (LazyEmptyBase) {
- const ASTRecordLayout &LazyLayout =
- Context.getASTRecordLayout(LazyEmptyBase);
- Size = Size.RoundUpToAlignment(LazyLayout.getAlignment());
- VBases.insert(
- std::make_pair(LazyEmptyBase, ASTRecordLayout::VBaseInfo(Size, false)));
- // Empty bases only consume space when followed by another empty base.
- // The space consumed is in an Alignment sized/aligned block and the v-base
- // is placed at its alignment offset into the chunk, unless its alignment
- // is less than 4 bytes, at which it is placed at 4 byte offset in the
- // chunk. We have no idea why.
- if (RD && Context.getASTRecordLayout(RD).getNonVirtualSize().isZero())
- Size = Size.RoundUpToAlignment(Alignment) + CharUnits::fromQuantity(4);
- LazyEmptyBase = 0;
- }
-
- // RD is null when flushing the final lazy virtual base.
- if (!RD)
- return;
-
const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
- if (Layout.getNonVirtualSize().isZero() && !HasVtordisp) {
- LazyEmptyBase = RD;
- return;
- }
+ // In order to avoid aliasing, the ABI adds padding between a pair of virtual
+ // bases if the first virtual base has a zero-size element anywhere an it and
+ // the second virtual base has a leftmost base that is zero sized. The
+ // minimal padding added is 4 bytes (in both 32 and 64 bits modes), we don't
+ // know why.
+ if (Layout.leftMostBaseIsZeroSized() && PreviousBaseLayout &&
+ PreviousBaseLayout->hasZeroSizedMemberOrBase())
+ Size = Size.RoundUpToAlignment(Alignment) +
+ std::max(CharUnits::fromQuantity(4), Layout.getAlignment());
CharUnits BaseNVSize = Layout.getNonVirtualSize();
CharUnits BaseAlign = Layout.getAlignment();
@@ -2574,17 +2577,19 @@
Size = BaseOffset + BaseNVSize;
// Note: we don't update alignment here because it was accounted for in
// InitializeLayout.
+
+ PreviousBaseLayout = &Layout;
}
void MicrosoftRecordLayoutBuilder::finalizeCXXLayout(const CXXRecordDecl *RD) {
- // Flush the lazy virtual base.
- layoutVirtualBase(0, false);
-
if (RD->vbases_begin() == RD->vbases_end() || AlignAfterVBases)
Size = Size.RoundUpToAlignment(Alignment);
- if (Size.isZero())
+ if ((IsEmpty = Size.isZero())) {
+ HasZeroSizedMemberOrBase = true;
+ LeftMostBaseIsZeroSized = true;
Size = Alignment;
+ }
}
void MicrosoftRecordLayoutBuilder::honorDeclspecAlign(const RecordDecl *RD) {
@@ -2690,8 +2695,9 @@
Builder.VBPtrOffset, Builder.DataSize, Builder.FieldOffsets.data(),
Builder.FieldOffsets.size(), Builder.DataSize,
Builder.NonVirtualAlignment, CharUnits::Zero(), Builder.PrimaryBase,
- false, Builder.SharedVBPtrBase, Builder.AlignAfterVBases, Builder.Bases,
- Builder.VBases);
+ false, Builder.SharedVBPtrBase, Builder.AlignAfterVBases,
+ Builder.HasZeroSizedMemberOrBase, Builder.LeftMostBaseIsZeroSized,
+ Builder.IsEmpty, Builder.Bases, Builder.VBases);
} else {
Builder.layout(D);
return new (*this) ASTRecordLayout(
@@ -2759,6 +2765,7 @@
Builder.PrimaryBase,
Builder.PrimaryBaseIsVirtual,
0, true,
+ false, false, false /*don't care*/,
Builder.Bases, Builder.VBases);
} else {
RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
