From 10b2c5623bb1383b42fa722d1de69a36bf018d65 Mon Sep 17 00:00:00 2001 From: Warren Hunt Date: Thu, 10 Apr 2014 22:15:18 +0000 Subject: [PATCH] [MS-ABI] Fixed __declspec(align()) on bitfields under #pragma pack. When __declspec(align()) is applied to a bitfield it affects the alignment rather than the required alignment of the struct. The major feature that this patch adds is that the alignment of the structure obeys the alignment of __declspec(align()) from the bitfield over the value specified in pragma pack. Test cases are included. The patch also includes some small cleanups in recordlayoutbuilder and some cleanups to some lit tests, including line endings (but no functionality change to lit tests) git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@205994 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/RecordLayoutBuilder.cpp | 65 +++++----- test/Layout/ms-x86-pack-and-align.cpp | 167 +++++++++++++++++++++++--- 2 files changed, 184 insertions(+), 48 deletions(-) diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index 8427eb5604..623d164d22 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2151,8 +2151,7 @@ public: void finalizeLayout(const RecordDecl *RD); /// \brief Gets the size and alignment of a base taking pragma pack and /// __declspec(align) into account. - ElementInfo getAdjustedElementInfo(const ASTRecordLayout &Layout, - bool AsBase = true); + ElementInfo getAdjustedElementInfo(const ASTRecordLayout &Layout); /// \brief Gets the size and alignment of a field taking pragma pack and /// __declspec(align) into account. It also updates RequiredAlignment as a /// side effect because it is most convenient to do so here. @@ -2226,7 +2225,7 @@ public: MicrosoftRecordLayoutBuilder::ElementInfo MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( - const ASTRecordLayout &Layout, bool AsBase) { + const ASTRecordLayout &Layout) { ElementInfo Info; Info.Alignment = Layout.getAlignment(); // Respect pragma pack. @@ -2238,8 +2237,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // the alignment in the case of pragam pack. Note that the required alignment // doesn't actually apply to the struct alignment at this point. Alignment = std::max(Alignment, Info.Alignment); + RequiredAlignment = std::max(RequiredAlignment, Layout.getRequiredAlignment()); Info.Alignment = std::max(Info.Alignment, Layout.getRequiredAlignment()); - Info.Size = AsBase ? Layout.getNonVirtualSize() : Layout.getSize(); + Info.Size = Layout.getNonVirtualSize(); return Info; } @@ -2253,37 +2253,30 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( CharUnits FieldRequiredAlignment = Context.toCharUnitsFromBits(FD->getMaxAlignment()); // Respect attributes applied to subobjects of the field. - if (const RecordType *RT = - FD->getType()->getBaseElementTypeUnsafe()->getAs()) { - const ASTRecordLayout &Layout = Context.getASTRecordLayout(RT->getDecl()); - // Get the element info for a layout, respecting pack. - Info.Alignment = getAdjustedElementInfo(Layout, false).Alignment; - // Capture required alignment as a side-effect. - RequiredAlignment = std::max(RequiredAlignment, - Layout.getRequiredAlignment()); - } else { - 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); - } - // Respect packed field attribute. - if (FD->hasAttr()) - Info.Alignment = CharUnits::One(); - // Take required alignment into account. __declspec(align) on bitfields - // impacts the alignment rather than the required alignment. - if (!FD->isBitField()) { + if (FD->isBitField()) + // For some reason __declspec align impacts alignment rather than required + // alignment when it is applied to bitfields. Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment); + else { + if (auto RT = + FD->getType()->getBaseElementTypeUnsafe()->getAs()) { + auto const &Layout = Context.getASTRecordLayout(RT->getDecl()); + EndsWithZeroSizedObject = Layout.hasZeroSizedSubObject(); + FieldRequiredAlignment = std::max(FieldRequiredAlignment, + Layout.getRequiredAlignment()); + } // Capture required alignment as a side-effect. RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment); } + // Respect pragma pack, attribute pack and declspec align + if (!MaxFieldAlignment.isZero()) + Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); + if (FD->hasAttr()) + Info.Alignment = CharUnits::One(); + Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment); // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. - if (!(FD->isBitField() && IsUnion)) { + if (!(FD->isBitField() && IsUnion)) Alignment = std::max(Alignment, Info.Alignment); - if (!MaxFieldAlignment.isZero()) - Alignment = std::min(Alignment, MaxFieldAlignment); - } return Info; } @@ -2305,7 +2298,10 @@ void MicrosoftRecordLayoutBuilder::cxxLayout(const CXXRecordDecl *RD) { injectVFPtr(RD); if (HasOwnVFPtr || (HasVBPtr && !SharedVBPtrBase)) Alignment = std::max(Alignment, PointerInfo.Alignment); - NonVirtualSize = Size = Size.RoundUpToAlignment(Alignment); + auto RoundingAlignment = Alignment; + if (!MaxFieldAlignment.isZero()) + RoundingAlignment = std::min(RoundingAlignment, MaxFieldAlignment); + NonVirtualSize = Size = Size.RoundUpToAlignment(RoundingAlignment); RequiredAlignment = std::max( RequiredAlignment, Context.toCharUnitsFromBits(RD->getMaxAlignment())); layoutVirtualBases(RD); @@ -2374,9 +2370,6 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { HasVBPtr = true; continue; } - // Track RequiredAlignment for all bases in this pass. - RequiredAlignment = std::max(RequiredAlignment, - BaseLayout.getRequiredAlignment()); // Check fo a base to share a VBPtr with. if (!SharedVBPtrBase && BaseLayout.hasVBPtr()) { SharedVBPtrBase = BaseDecl; @@ -2633,7 +2626,11 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { DataSize = Size; if (!RequiredAlignment.isZero()) { Alignment = std::max(Alignment, RequiredAlignment); - Size = Size.RoundUpToAlignment(Alignment); + auto RoundingAlignment = Alignment; + if (!MaxFieldAlignment.isZero()) + RoundingAlignment = std::min(RoundingAlignment, MaxFieldAlignment); + RoundingAlignment = std::max(RoundingAlignment, RequiredAlignment); + Size = Size.RoundUpToAlignment(RoundingAlignment); } // Zero-sized structures have size equal to their alignment. if (Size.isZero()) { diff --git a/test/Layout/ms-x86-pack-and-align.cpp b/test/Layout/ms-x86-pack-and-align.cpp index 27de729074..0843b3e0bf 100644 --- a/test/Layout/ms-x86-pack-and-align.cpp +++ b/test/Layout/ms-x86-pack-and-align.cpp @@ -40,7 +40,9 @@ struct X { // CHECK-NEXT: 0 | struct X // CHECK-NEXT: 0 | struct B a // CHECK-NEXT: 0 | long long a -// CHECK: 8 | char b +// CHECK-NEXT: | [sizeof=8, align=8 +// CHECK-NEXT: | nvsize=8, nvalign=8] +// CHECK-NEXT: 8 | char b // CHECK-NEXT: 10 | int c // CHECK-NEXT: | [sizeof=16, align=4 // CHECK-NEXT: | nvsize=14, nvalign=4] @@ -49,7 +51,9 @@ struct X { // CHECK-X64-NEXT: 0 | struct X // CHECK-X64-NEXT: 0 | struct B a // CHECK-X64-NEXT: 0 | long long a -// CHECK-X64: 8 | char b +// CHECK-X64-NEXT: | [sizeof=8, align=8 +// CHECK-X64-NEXT: | nvsize=8, nvalign=8] +// CHECK-X64-NEXT: 8 | char b // CHECK-X64-NEXT: 10 | int c // CHECK-X64-NEXT: | [sizeof=16, align=4 // CHECK-X64-NEXT: | nvsize=14, nvalign=4] @@ -208,14 +212,18 @@ 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: | [sizeof=32, align=32 +// CHECK-NEXT: | nvsize=32, nvalign=32] +// CHECK-NEXT: | [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: | [sizeof=32, align=32 +// CHECK-X64-NEXT: | nvsize=32, nvalign=32] +// CHECK-X64-NEXT: | [sizeof=33, align=1 // CHECK-X64-NEXT: | nvsize=33, nvalign=1] #pragma pack(8) @@ -230,8 +238,8 @@ struct YC { // 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] +// CHECK-X64-NEXT: | [sizeof=8, align=32 +// CHECK-X64-NEXT: | nvsize=8, nvalign=32] #pragma pack(1) struct YD { @@ -243,14 +251,18 @@ 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: | [sizeof=32, align=32 +// CHECK-NEXT: | nvsize=32, nvalign=32] +// CHECK-NEXT: | [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: | [sizeof=8, align=32 +// CHECK-X64-NEXT: | nvsize=8, nvalign=32] +// CHECK-X64-NEXT: | [sizeof=9, align=1 // CHECK-X64-NEXT: | nvsize=9, nvalign=1] #pragma pack(4) @@ -260,13 +272,13 @@ struct YE { // 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-NEXT: | [sizeof=4, align=32 +// CHECK-NEXT: | nvsize=4, nvalign=32] // 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] +// CHECK-X64-NEXT: | [sizeof=4, align=32 +// CHECK-X64-NEXT: | nvsize=4, nvalign=32] #pragma pack(1) struct YF { @@ -278,14 +290,18 @@ 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: | [sizeof=4, align=32 +// CHECK-NEXT: | nvsize=4, nvalign=32] +// CHECK-NEXT: | [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: | [sizeof=4, align=32 +// CHECK-X64-NEXT: | nvsize=4, nvalign=32] +// CHECK-X64-NEXT: | [sizeof=5, align=1 // CHECK-X64-NEXT: | nvsize=5, nvalign=1] #pragma pack(16) @@ -411,6 +427,123 @@ struct MB : virtual MA { // CHECK-X64-NEXT: | [sizeof=512, align=256 // CHECK-X64-NEXT: | nvsize=260, nvalign=256] +struct RA {}; +#pragma pack(1) +struct __declspec(align(8)) RB0 { + __declspec(align(1024)) int b : 3; +}; + +struct __declspec(align(8)) RB1 { + __declspec(align(1024)) int b : 3; + virtual void f() {} +}; + +struct __declspec(align(8)) RB2 : virtual RA { + __declspec(align(1024)) int b : 3; +}; + +struct __declspec(align(8)) RB3 : virtual RA { + __declspec(align(1024)) int b : 3; + virtual void f() {} +}; + +struct RC { + char _; + __declspec(align(1024)) int c : 3; +}; +struct RE { + char _; + RC c; +}; +#pragma pack() + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct RB0 +// CHECK-NEXT: 0 | int b +// CHECK-NEXT: | [sizeof=8, align=1024 +// CHECK-NEXT: | nvsize=4, nvalign=1024] +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct RB1 +// CHECK-NEXT: 0 | (RB1 vftable pointer) +// CHECK-NEXT: 1024 | int b +// CHECK-NEXT: | [sizeof=1032, align=1024 +// CHECK-NEXT: | nvsize=1028, nvalign=1024] +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct RB2 +// CHECK-NEXT: 0 | (RB2 vbtable pointer) +// CHECK-NEXT: 1024 | int b +// CHECK-NEXT: 1032 | struct RA (virtual base) (empty) +// CHECK-NEXT: | [sizeof=1032, align=1024 +// CHECK-NEXT: | nvsize=1028, nvalign=1024] +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct RB3 +// CHECK-NEXT: 0 | (RB3 vftable pointer) +// CHECK-NEXT: 1024 | (RB3 vbtable pointer) +// CHECK-NEXT: 2048 | int b +// CHECK-NEXT: 2056 | struct RA (virtual base) (empty) +// CHECK-NEXT: | [sizeof=2056, align=1024 +// CHECK-NEXT: | nvsize=2052, nvalign=1024] +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct RC +// CHECK-NEXT: 0 | char _ +// CHECK-NEXT: 1024 | int c +// CHECK-NEXT: | [sizeof=1028, align=1024 +// CHECK-NEXT: | nvsize=1028, nvalign=1024] +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct RE +// CHECK-NEXT: 0 | char _ +// CHECK-NEXT: 1 | struct RC c +// CHECK-NEXT: 1 | char _ +// CHECK-NEXT: 1025 | int c +// CHECK-NEXT: | [sizeof=1028, align=1024 +// CHECK-NEXT: | nvsize=1028, nvalign=1024] +// CHECK-NEXT: | [sizeof=1029, align=1 +// CHECK-NEXT: | nvsize=1029, nvalign=1] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct RB0 +// CHECK-X64-NEXT: 0 | int b +// CHECK-X64-NEXT: | [sizeof=8, align=1024 +// CHECK-X64-NEXT: | nvsize=4, nvalign=1024] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct RB1 +// CHECK-X64-NEXT: 0 | (RB1 vftable pointer) +// CHECK-X64-NEXT: 1024 | int b +// CHECK-X64-NEXT: | [sizeof=1032, align=1024 +// CHECK-X64-NEXT: | nvsize=1028, nvalign=1024] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct RB2 +// CHECK-X64-NEXT: 0 | (RB2 vbtable pointer) +// CHECK-X64-NEXT: 1024 | int b +// CHECK-X64-NEXT: 1032 | struct RA (virtual base) (empty) +// CHECK-X64-NEXT: | [sizeof=1032, align=1024 +// CHECK-X64-NEXT: | nvsize=1028, nvalign=1024] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct RB3 +// CHECK-X64-NEXT: 0 | (RB3 vftable pointer) +// CHECK-X64-NEXT: 1024 | (RB3 vbtable pointer) +// CHECK-X64-NEXT: 2048 | int b +// CHECK-X64-NEXT: 2056 | struct RA (virtual base) (empty) +// CHECK-X64-NEXT: | [sizeof=2056, align=1024 +// CHECK-X64-NEXT: | nvsize=2052, nvalign=1024] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct RC +// CHECK-X64-NEXT: 0 | char _ +// CHECK-X64-NEXT: 1024 | int c +// CHECK-X64-NEXT: | [sizeof=1028, align=1024 +// CHECK-X64-NEXT: | nvsize=1028, nvalign=1024] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct RE +// CHECK-X64-NEXT: 0 | char _ +// CHECK-X64-NEXT: 1 | struct RC c +// CHECK-X64-NEXT: 1 | char _ +// CHECK-X64-NEXT: 1025 | int c +// CHECK-X64-NEXT: | [sizeof=1028, align=1024 +// CHECK-X64-NEXT: | nvsize=1028, nvalign=1024] +// CHECK-X64-NEXT: | [sizeof=1029, align=1 +// CHECK-X64-NEXT: | nvsize=1029, nvalign=1] + int a[ sizeof(X)+ sizeof(Y)+ @@ -429,4 +562,10 @@ sizeof(JC)+ sizeof(KB)+ sizeof(L)+ sizeof(MB)+ +sizeof(RB0)+ +sizeof(RB1)+ +sizeof(RB2)+ +sizeof(RB3)+ +sizeof(RC)+ +sizeof(RE)+ 0]; -- 2.40.0