From d7f64c9ed69b8c24c457570c5b45b79c86b133ea Mon Sep 17 00:00:00 2001 From: Warren Hunt Date: Fri, 10 Jan 2014 01:28:05 +0000 Subject: [PATCH] [ms-abi] Handle __declspec(align) on bitfields "properly" __declspec(align), when applied to bitfields affects their perferred alignment instead of their required alignment. We don't know why. Also, #pragma pack(n) turns packing *off* if n is greater than the pointer size. This is now observable because of the impact of declspec(align) on bitfields. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@198907 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/RecordLayoutBuilder.cpp | 61 ++++++++------ test/Layout/ms-x86-pack-and-align.cpp | 117 +++++++++++++++++++++++++- 2 files changed, 152 insertions(+), 26 deletions(-) diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index ceb49ff0bf..30293c27c1 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2028,7 +2028,9 @@ static bool isMsLayout(const RecordDecl* D) { // modes). // * There is no concept of non-virtual alignment or any distinction between // data size and non-virtual size. - +// * __declspec(align) on bitfields has the effect of changing the bitfield's +// alignment instead of its required alignment. This has implications on how +// it interacts with pragam pack. namespace { struct MicrosoftRecordLayoutBuilder { @@ -2165,6 +2167,9 @@ MicrosoftRecordLayoutBuilder::ElementInfo MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( const FieldDecl *FD) { ElementInfo Info; + // Respect align attributes. + CharUnits FieldRequiredAlignment = + Context.toCharUnitsFromBits(FD->getMaxAlignment()); // Respect attributes applied to subobjects of the field. if (const RecordType *RT = FD->getType()->getBaseElementTypeUnsafe()->getAs()) { @@ -2183,6 +2188,8 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( Context.getTypeInfoInChars(FD->getType()); Info.Size = FieldInfo.first; Info.Alignment = FieldInfo.second; + if (FD->isBitField() && FD->getMaxAlignment() != 0) + Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment); // Respect pragma pack. if (!MaxFieldAlignment.isZero()) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); @@ -2190,13 +2197,13 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // Respect packed field attribute. if (FD->hasAttr()) Info.Alignment = CharUnits::One(); - // Respect align attributes. - CharUnits FieldRequiredAlignment = - Context.toCharUnitsFromBits(FD->getMaxAlignment()); - // Take required alignment into account. - Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment); - // Capture required alignment as a side-effect. - RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment); + // Take required alignment into account. __declspec(align) on bitfields + // impacts the alignment rather than the required alignment. + if (!FD->isBitField()) { + Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment); + // Capture required alignment as a side-effect. + RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment); + } return Info; } @@ -2233,10 +2240,14 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { MaxFieldAlignment = CharUnits::Zero(); // Honor the default struct packing maximum alignment flag. if (unsigned DefaultMaxFieldAlignment = Context.getLangOpts().PackStruct) - MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment); - // Honor the packing attribute. - if (const MaxFieldAlignmentAttr *MFAA = RD->getAttr()) - MaxFieldAlignment = Context.toCharUnitsFromBits(MFAA->getAlignment()); + MaxFieldAlignment = CharUnits::fromQuantity(DefaultMaxFieldAlignment); + // Honor the packing attribute. The MS-ABI ignores pragma pack if its larger + // than the pointer size. + if (const MaxFieldAlignmentAttr *MFAA = RD->getAttr()){ + unsigned PackedAlignment = MFAA->getAlignment(); + if (PackedAlignment <= Context.getTargetInfo().getPointerWidth(0)) + MaxFieldAlignment = Context.toCharUnitsFromBits(PackedAlignment); + } // Packed attribute forces max field alignment to be 1. if (RD->hasAttr()) MaxFieldAlignment = CharUnits::One(); @@ -2335,18 +2346,18 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( const CXXRecordDecl *BaseDecl, const ASTRecordLayout &BaseLayout, const ASTRecordLayout *&PreviousBaseLayout) { - // Insert padding between two bases if the left first one is zero sized or - // contains a zero sized subobject and the right is zero sized or one leads - // with a zero sized base. - if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() && - BaseLayout.leadsWithZeroSizedBase()) - Size++; - ElementInfo Info = getAdjustedElementInfo(BaseLayout); - CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); - Bases.insert(std::make_pair(BaseDecl, BaseOffset)); - Size = BaseOffset + BaseLayout.getDataSize(); - updateAlignment(Info.Alignment); - PreviousBaseLayout = &BaseLayout; + // Insert padding between two bases if the left first one is zero sized or + // contains a zero sized subobject and the right is zero sized or one leads + // with a zero sized base. + if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() && + BaseLayout.leadsWithZeroSizedBase()) + Size++; + ElementInfo Info = getAdjustedElementInfo(BaseLayout); + CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); + Bases.insert(std::make_pair(BaseDecl, BaseOffset)); + Size = BaseOffset + BaseLayout.getDataSize(); + updateAlignment(Info.Alignment); + PreviousBaseLayout = &BaseLayout; VBPtrOffset = Size; } @@ -2598,7 +2609,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { Size = Size.RoundUpToAlignment(VtorDispAlignment) + VtorDispSize; // Insert the virtual base. ElementInfo Info = getAdjustedElementInfo(BaseLayout); - CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); + CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); VBases.insert(std::make_pair(BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); Size = BaseOffset + BaseLayout.getDataSize(); diff --git a/test/Layout/ms-x86-pack-and-align.cpp b/test/Layout/ms-x86-pack-and-align.cpp index b9d286211e..fba485f5a2 100644 --- a/test/Layout/ms-x86-pack-and-align.cpp +++ b/test/Layout/ms-x86-pack-and-align.cpp @@ -183,9 +183,124 @@ struct CA2 : public CA1, public CA0 { // CHECK-C64-NEXT: | [sizeof=17, align=1 // CHECK-C64-NEXT: | nvsize=17, nvalign=1] +#pragma pack(16) +struct YA { + __declspec(align(32)) char : 1; +}; +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct YA (empty) +// CHECK-NEXT: 0 | char +// CHECK-NEXT: | [sizeof=32, align=32 +// CHECK-NEXT: | nvsize=32, nvalign=32] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct YA (empty) +// CHECK-X64-NEXT: 0 | char +// CHECK-X64-NEXT: | [sizeof=32, align=32 +// CHECK-X64-NEXT: | nvsize=32, nvalign=32] + +#pragma pack(1) +struct YB { + char a; + YA b; +}; +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct YB +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 1 | struct YA b (empty) +// CHECK-NEXT: 1 | char +// CHECK: | [sizeof=33, align=1 +// CHECK-NEXT: | nvsize=33, nvalign=1] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct YB +// CHECK-X64-NEXT: 0 | char a +// CHECK-X64-NEXT: 1 | struct YA b (empty) +// CHECK-X64-NEXT: 1 | char +// CHECK-X64: | [sizeof=33, align=1 +// CHECK-X64-NEXT: | nvsize=33, nvalign=1] + +#pragma pack(8) +struct YC { + __declspec(align(32)) char : 1; +}; +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct YC (empty) +// CHECK-NEXT: 0 | char +// CHECK-NEXT: | [sizeof=32, align=32 +// CHECK-NEXT: | nvsize=32, nvalign=32] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct YC (empty) +// CHECK-X64-NEXT: 0 | char +// CHECK-X64-NEXT: | [sizeof=8, align=8 +// CHECK-X64-NEXT: | nvsize=8, nvalign=8] + +#pragma pack(1) +struct YD { + char a; + YC b; +}; +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct YD +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 1 | struct YC b (empty) +// CHECK-NEXT: 1 | char +// CHECK: | [sizeof=33, align=1 +// CHECK-NEXT: | nvsize=33, nvalign=1] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct YD +// CHECK-X64-NEXT: 0 | char a +// CHECK-X64-NEXT: 1 | struct YC b (empty) +// CHECK-X64-NEXT: 1 | char +// CHECK-X64: | [sizeof=9, align=1 +// CHECK-X64-NEXT: | nvsize=9, nvalign=1] + +#pragma pack(4) +struct YE { + __declspec(align(32)) char : 1; +}; +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct YE (empty) +// CHECK-NEXT: 0 | char +// CHECK-NEXT: | [sizeof=4, align=4 +// CHECK-NEXT: | nvsize=4, nvalign=4] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct YE (empty) +// CHECK-X64-NEXT: 0 | char +// CHECK-X64-NEXT: | [sizeof=4, align=4 +// CHECK-X64-NEXT: | nvsize=4, nvalign=4] + +#pragma pack(1) +struct YF { + char a; + YE b; +}; +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct YF +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 1 | struct YE b (empty) +// CHECK-NEXT: 1 | char +// CHECK: | [sizeof=5, align=1 +// CHECK-NEXT: | nvsize=5, nvalign=1] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct YF +// CHECK-X64-NEXT: 0 | char a +// CHECK-X64-NEXT: 1 | struct YE b (empty) +// CHECK-X64-NEXT: 1 | char +// CHECK-X64: | [sizeof=5, align=1 +// CHECK-X64-NEXT: | nvsize=5, nvalign=1] + int a[ sizeof(X)+ sizeof(Y)+ sizeof(Z)+ sizeof(C1)+ -sizeof(CA2)]; +sizeof(CA2)+ +sizeof(YA)+ +sizeof(YB)+ +sizeof(YC)+ +sizeof(YD)+ +sizeof(YE)+ +sizeof(YF)+ +0]; -- 2.40.0