diff --git a/include/clang/AST/RecordLayout.h b/include/clang/AST/RecordLayout.h index 7268b3a8240cd973165103d41caf9f97715eae2e..5ee87b9184337dfef0bd3610806c92a9118b65ee 100644 --- a/include/clang/AST/RecordLayout.h +++ b/include/clang/AST/RecordLayout.h @@ -66,6 +66,10 @@ private: // Alignment - Alignment of record in characters. CharUnits Alignment; + /// RequiredAlignment - The required alignment of the object. In the MS-ABI + /// the __declspec(align()) trumps #pramga pack and must always be obeyed. + CharUnits RequiredAlignment; + /// FieldOffsets - Array of field offsets in bits. uint64_t *FieldOffsets; @@ -100,10 +104,6 @@ private: /// a primary base class. bool HasExtendableVFPtr : 1; - /// AlignAfterVBases - Force appropriate alignment after virtual bases are - /// laid out in MS-C++-ABI. - bool AlignAfterVBases : 1; - /// PrimaryBase - The primary base info for this record. llvm::PointerIntPair<const CXXRecordDecl *, 1, bool> PrimaryBase; @@ -127,6 +127,7 @@ private: friend class ASTContext; ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment, + CharUnits requiredAlignment, CharUnits datasize, const uint64_t *fieldoffsets, unsigned fieldcount); @@ -134,6 +135,7 @@ private: typedef CXXRecordLayoutInfo::BaseOffsetsMapTy BaseOffsetsMapTy; ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment, + CharUnits requiredAlignment, bool hasOwnVFPtr, bool hasExtendableVFPtr, CharUnits vbptroffset, CharUnits datasize, @@ -143,7 +145,6 @@ private: const CXXRecordDecl *PrimaryBase, bool IsPrimaryBaseVirtual, const CXXRecordDecl *BaseSharingVBPtr, - bool ForceAlign, const BaseOffsetsMapTy& BaseOffsets, const VBaseOffsetsMapTy& VBaseOffsets); @@ -267,9 +268,8 @@ public: return !CXXInfo->VBPtrOffset.isNegative(); } - bool getAlignAfterVBases() const { - assert(CXXInfo && "Record layout does not have C++ specific info!"); - return CXXInfo->AlignAfterVBases; + CharUnits getRequiredAlignment() const { + return RequiredAlignment; } /// getVBPtrOffset - Get the offset for virtual base table pointer. diff --git a/lib/AST/RecordLayout.cpp b/lib/AST/RecordLayout.cpp index 71e44ecf9219db7d91523f748ffa3bfc5a9afece..a4a59c4f330523fed05b7120385d157f7591a6bd 100644 --- a/lib/AST/RecordLayout.cpp +++ b/lib/AST/RecordLayout.cpp @@ -29,10 +29,13 @@ void ASTRecordLayout::Destroy(ASTContext &Ctx) { } ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CharUnits size, - CharUnits alignment, CharUnits datasize, + CharUnits alignment, + CharUnits requiredAlignment, + CharUnits datasize, const uint64_t *fieldoffsets, unsigned fieldcount) - : Size(size), DataSize(datasize), Alignment(alignment), FieldOffsets(0), + : Size(size), DataSize(datasize), Alignment(alignment), + RequiredAlignment(requiredAlignment), FieldOffsets(0), FieldCount(fieldcount), CXXInfo(0) { if (FieldCount > 0) { FieldOffsets = new (Ctx) uint64_t[FieldCount]; @@ -43,6 +46,7 @@ ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CharUnits size, // Constructor for C++ records. ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment, + CharUnits requiredAlignment, bool hasOwnVFPtr, bool hasExtendableVFPtr, CharUnits vbptroffset, CharUnits datasize, @@ -54,10 +58,10 @@ ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, const CXXRecordDecl *PrimaryBase, bool IsPrimaryBaseVirtual, const CXXRecordDecl *BaseSharingVBPtr, - bool AlignAfterVBases, const BaseOffsetsMapTy& BaseOffsets, const VBaseOffsetsMapTy& VBaseOffsets) - : Size(size), DataSize(datasize), Alignment(alignment), FieldOffsets(0), + : Size(size), DataSize(datasize), Alignment(alignment), + RequiredAlignment(requiredAlignment), FieldOffsets(0), FieldCount(fieldcount), CXXInfo(new (Ctx) CXXRecordLayoutInfo) { if (FieldCount > 0) { @@ -76,7 +80,6 @@ ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CXXInfo->VBPtrOffset = vbptroffset; CXXInfo->HasExtendableVFPtr = hasExtendableVFPtr; CXXInfo->BaseSharingVBPtr = BaseSharingVBPtr; - CXXInfo->AlignAfterVBases = AlignAfterVBases; #ifndef NDEBUG diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index f0142525b1ab30bd902c07e4b27038e9244bba31..a1f6c41ef86222882a7b4829fc6aff0a92d16ba4 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2064,6 +2064,7 @@ public: void updateAlignment(CharUnits NewAlignment) { Alignment = std::max(Alignment, NewAlignment); } + CharUnits getBaseAlignment(const ASTRecordLayout& Layout); /// \brief Gets the size and alignment taking attributes into account. std::pair<CharUnits, CharUnits> getAdjustedFieldInfo(const FieldDecl *FD); /// \brief Places a field at offset 0. @@ -2089,9 +2090,9 @@ public: SmallVector<uint64_t, 16> FieldOffsets; /// \brief The maximum allowed field alignment. This is set by #pragma pack. CharUnits MaxFieldAlignment; - /// \brief Alignment does not occur for virtual bases unless something - /// forces it to by explicitly using __declspec(align()) - bool AlignAfterVBases : 1; + /// \brief The alignment that this record must obey. This is imposed by + /// __declspec(align()) on the record itself or one of its fields or bases. + CharUnits RequiredAlignment; bool IsUnion : 1; /// \brief True if the last field laid out was a bitfield and was not 0 /// width. @@ -2148,6 +2149,16 @@ public: }; } // namespace +CharUnits +MicrosoftRecordLayoutBuilder::getBaseAlignment(const ASTRecordLayout& Layout) { + CharUnits BaseAlignment = Layout.getAlignment(); + if (!MaxFieldAlignment.isZero()) + BaseAlignment = std::min(BaseAlignment, + std::max(MaxFieldAlignment, + Layout.getRequiredAlignment())); + return BaseAlignment; +} + std::pair<CharUnits, CharUnits> MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) { std::pair<CharUnits, CharUnits> FieldInfo = @@ -2174,9 +2185,18 @@ MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) { // Respect alignment attributes. if (unsigned fieldAlign = FD->getMaxAlignment()) { CharUnits FieldAlign = Context.toCharUnitsFromBits(fieldAlign); - AlignAfterVBases = true; + RequiredAlignment = std::max(RequiredAlignment, FieldAlign); FieldInfo.second = std::max(FieldInfo.second, FieldAlign); } + // Respect attributes applied inside field base types. + if (const RecordType *RT = + FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) { + const ASTRecordLayout &Layout = Context.getASTRecordLayout(RT->getDecl()); + RequiredAlignment = std::max(RequiredAlignment, + Layout.getRequiredAlignment()); + FieldInfo.second = std::max(FieldInfo.second, + Layout.getRequiredAlignment()); + } return FieldInfo; } @@ -2186,7 +2206,10 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { Size = CharUnits::Zero(); Alignment = CharUnits::One(); - AlignAfterVBases = false; + // In 64-bit mode we always perform an alignment step after laying out vbases. + // In 32-bit mode we do not. The check to see if we need to perform alignment + // checks the RequiredAlignment field and performs alignment if it isn't 0. + RequiredAlignment = Is64BitMode ? CharUnits::One() : CharUnits::Zero(); // Compute the maximum field alignment. MaxFieldAlignment = CharUnits::Zero(); @@ -2236,7 +2259,6 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { SharedVBPtrBase = 0; PrimaryBase = 0; VirtualAlignment = CharUnits::One(); - AlignAfterVBases = Is64BitMode; // 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 @@ -2247,12 +2269,12 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { const CXXRecordDecl *BaseDecl = cast<CXXRecordDecl>(i->getType()->getAs<RecordType>()->getDecl()); const ASTRecordLayout &Layout = Context.getASTRecordLayout(BaseDecl); - // Handle forced alignment. - if (Layout.getAlignAfterVBases()) - AlignAfterVBases = true; + // Handle required alignment. + RequiredAlignment = std::max(RequiredAlignment, + Layout.getRequiredAlignment()); // Handle virtual bases. if (i->isVirtual()) { - VirtualAlignment = std::max(VirtualAlignment, Layout.getAlignment()); + VirtualAlignment = std::max(VirtualAlignment, getBaseAlignment(Layout)); HasVBPtr = true; continue; } @@ -2266,7 +2288,7 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { SharedVBPtrBase = BaseDecl; HasVBPtr = true; } - updateAlignment(Layout.getAlignment()); + updateAlignment(getBaseAlignment(Layout)); } // Use LayoutFields to compute the alignment of the fields. The layout @@ -2335,7 +2357,7 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(const CXXRecordDecl *RD) { if (LazyEmptyBase) { const ASTRecordLayout &LazyLayout = Context.getASTRecordLayout(LazyEmptyBase); - Size = Size.RoundUpToAlignment(LazyLayout.getAlignment()); + Size = Size.RoundUpToAlignment(getBaseAlignment(LazyLayout)); // If the last non-virtual base has a vbptr we add a byte of padding for no // obvious reason. if (LastNonVirtualBaseHasVBPtr) @@ -2360,7 +2382,7 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(const CXXRecordDecl *RD) { } // Insert the base here. - CharUnits BaseOffset = Size.RoundUpToAlignment(Layout->getAlignment()); + CharUnits BaseOffset = Size.RoundUpToAlignment(getBaseAlignment(*Layout)); Bases.insert(std::make_pair(RD, BaseOffset)); Size = BaseOffset + Layout->getDataSize(); // Note: we don't update alignment here because it was accounted @@ -2536,7 +2558,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBase(const CXXRecordDecl *RD, if (LazyEmptyBase) { const ASTRecordLayout &LazyLayout = Context.getASTRecordLayout(LazyEmptyBase); - Size = Size.RoundUpToAlignment(LazyLayout.getAlignment()); + Size = Size.RoundUpToAlignment(getBaseAlignment(LazyLayout)); VBases.insert( std::make_pair(LazyEmptyBase, ASTRecordLayout::VBaseInfo(Size, false))); // Empty bases only consume space when followed by another empty base. @@ -2560,7 +2582,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBase(const CXXRecordDecl *RD, } CharUnits BaseNVSize = Layout.getNonVirtualSize(); - CharUnits BaseAlign = Layout.getAlignment(); + CharUnits BaseAlign = getBaseAlignment(Layout); // vtordisps are always 4 bytes (even in 64-bit mode) if (HasVtordisp) @@ -2580,7 +2602,7 @@ void MicrosoftRecordLayoutBuilder::finalizeCXXLayout(const CXXRecordDecl *RD) { // Flush the lazy virtual base. layoutVirtualBase(0, false); - if (RD->vbases_begin() == RD->vbases_end() || AlignAfterVBases) + if (RD->vbases_begin() == RD->vbases_end() || !RequiredAlignment.isZero()) Size = Size.RoundUpToAlignment(Alignment); if (Size.isZero()) @@ -2588,9 +2610,10 @@ void MicrosoftRecordLayoutBuilder::finalizeCXXLayout(const CXXRecordDecl *RD) { } void MicrosoftRecordLayoutBuilder::honorDeclspecAlign(const RecordDecl *RD) { - if (unsigned MaxAlign = RD->getMaxAlignment()) { - AlignAfterVBases = true; - updateAlignment(Context.toCharUnitsFromBits(MaxAlign)); + RequiredAlignment = std::max(RequiredAlignment, + Context.toCharUnitsFromBits(RD->getMaxAlignment())); + if (!RequiredAlignment.isZero()) { + updateAlignment(RequiredAlignment); Size = Size.RoundUpToAlignment(Alignment); } } @@ -2684,19 +2707,19 @@ ASTContext::BuildMicrosoftASTRecordLayout(const RecordDecl *D) const { if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) { Builder.cxxLayout(RD); return new (*this) ASTRecordLayout( - *this, Builder.Size, Builder.Alignment, + *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment, Builder.HasExtendableVFPtr && !Builder.PrimaryBase, Builder.HasExtendableVFPtr, 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, + false, Builder.SharedVBPtrBase, Builder.Bases, Builder.VBases); } else { Builder.layout(D); return new (*this) ASTRecordLayout( - *this, Builder.Size, Builder.Alignment, Builder.Size, - Builder.FieldOffsets.data(), Builder.FieldOffsets.size()); + *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment, + Builder.Size, Builder.FieldOffsets.data(), Builder.FieldOffsets.size()); } } @@ -2746,6 +2769,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { skipTailPadding ? DataSize : Builder.NonVirtualSize; NewEntry = new (*this) ASTRecordLayout(*this, Builder.getSize(), + Builder.Alignment, + /*RequiredAlignment : used by MS-ABI)*/ Builder.Alignment, Builder.HasOwnVFPtr, RD->isDynamicClass(), @@ -2758,7 +2783,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { EmptySubobjects.SizeOfLargestEmptySubobject, Builder.PrimaryBase, Builder.PrimaryBaseIsVirtual, - 0, true, + 0, Builder.Bases, Builder.VBases); } else { RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0); @@ -2766,6 +2791,8 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { NewEntry = new (*this) ASTRecordLayout(*this, Builder.getSize(), + Builder.Alignment, + /*RequiredAlignment : used by MS-ABI)*/ Builder.Alignment, Builder.getSize(), Builder.FieldOffsets.data(), @@ -2875,6 +2902,8 @@ ASTContext::getObjCLayout(const ObjCInterfaceDecl *D, const ASTRecordLayout *NewEntry = new (*this) ASTRecordLayout(*this, Builder.getSize(), + Builder.Alignment, + /*RequiredAlignment : used by MS-ABI)*/ Builder.Alignment, Builder.getDataSize(), Builder.FieldOffsets.data(), diff --git a/test/Layout/ms-x86-pack-and-align.cpp b/test/Layout/ms-x86-pack-and-align.cpp new file mode 100644 index 0000000000000000000000000000000000000000..f9f2b30f5f73e006aa18b040392f51a0442ee572 --- /dev/null +++ b/test/Layout/ms-x86-pack-and-align.cpp @@ -0,0 +1,120 @@ +// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only -cxx-abi microsoft %s 2>&1 \ +// RUN: | FileCheck %s +// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only -cxx-abi microsoft %s 2>/dev/null \ +// RUN: | FileCheck %s -check-prefix CHECK-X64 + +extern "C" int printf(const char *fmt, ...); +char buffer[419430400]; + +struct A { + char a; + A() { + printf("A = %d\n", (int)((char*)this - buffer)); + printf("A.a = %d\n", (int)((char*)&a - buffer)); + } +}; + +struct B { + __declspec(align(4)) long long a; + B() { + printf("B = %d\n", (int)((char*)this - buffer)); + printf("B.a = %d\n", (int)((char*)&a - buffer)); + } +}; + +#pragma pack(push, 2) +struct X { + B a; + char b; + int c; + X() { + printf("X = %d\n", (int)((char*)this - buffer)); + printf("X.a = %d\n", (int)((char*)&a - buffer)); + printf("X.b = %d\n", (int)((char*)&b - buffer)); + printf("X.c = %d\n", (int)((char*)&c - buffer)); + } +}; + +// CHECK: *** Dumping AST Record Layout +// CHECK: 0 | struct X +// CHECK: 0 | struct B a +// CHECK: 0 | long long a +// CHECK: 8 | char b +// CHECK: 10 | int c +// CHECK: | [sizeof=16, align=4 +// CHECK: | nvsize=16, nvalign=4] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: 0 | struct X +// CHECK-X64: 0 | struct B a +// CHECK-X64: 0 | long long a +// CHECK-X64: 8 | char b +// CHECK-X64: 10 | int c +// CHECK-X64: | [sizeof=16, align=4 +// CHECK-X64: | nvsize=16, nvalign=4] + +struct Y : A, B { + char a; + int b; + Y() { + printf("Y = %d\n", (int)((char*)this - buffer)); + printf("Y.a = %d\n", (int)((char*)&a - buffer)); + printf("Y.b = %d\n", (int)((char*)&b - buffer)); + } +}; + +// CHECK: *** Dumping AST Record Layout +// CHECK: 0 | struct Y +// CHECK: 0 | struct A (base) +// CHECK: 0 | char a +// CHECK: 4 | struct B (base) +// CHECK: 4 | long long a +// CHECK: 12 | char a +// CHECK: 14 | int b +// CHECK: | [sizeof=20, align=4 +// CHECK: | nvsize=20, nvalign=4] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: 0 | struct Y +// CHECK-X64: 0 | struct A (base) +// CHECK-X64: 0 | char a +// CHECK-X64: 4 | struct B (base) +// CHECK-X64: 4 | long long a +// CHECK-X64: 12 | char a +// CHECK-X64: 14 | int b +// CHECK-X64: | [sizeof=20, align=4 +// CHECK-X64: | nvsize=20, nvalign=4] + +struct Z : virtual B { + char a; + int b; + Z() { + printf("Z = %d\n", (int)((char*)this - buffer)); + printf("Z.a = %d\n", (int)((char*)&a - buffer)); + printf("Z.b = %d\n", (int)((char*)&b - buffer)); + } +}; + +// CHECK: *** Dumping AST Record Layout +// CHECK: 0 | struct Z +// CHECK: 0 | (Z vbtable pointer) +// CHECK: 4 | char a +// CHECK: 6 | int b +// CHECK: 12 | struct B (virtual base) +// CHECK: 12 | long long a +// CHECK: | [sizeof=20, align=4 +// CHECK: | nvsize=10, nvalign=2] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: 0 | struct Z +// CHECK-X64: 0 | (Z vbtable pointer) +// CHECK-X64: 8 | char a +// CHECK-X64: 10 | int b +// CHECK-X64: 16 | struct B (virtual base) +// CHECK-X64: 16 | long long a +// CHECK-X64: | [sizeof=24, align=4 +// CHECK-X64: | nvsize=14, nvalign=2] + +#pragma pack(pop) + +int a[ +sizeof(X)+ +sizeof(Y)+ +sizeof(Z)];