r205994 - [MS-ABI] Fixed __declspec(align()) on bitfields under #pragma pack.
Warren Hunt
whunt at google.com
Thu Apr 10 15:15:18 PDT 2014
Author: whunt
Date: Thu Apr 10 17:15:18 2014
New Revision: 205994
URL: http://llvm.org/viewvc/llvm-project?rev=205994&view=rev
Log:
[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)
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=205994&r1=205993&r2=205994&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Apr 10 17:15:18 2014
@@ -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::getAdjuste
// 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::getAdjuste
CharUnits FieldRequiredAlignment =
Context.toCharUnitsFromBits(FD->getMaxAlignment());
// Respect attributes applied to subobjects of the field.
- if (const RecordType *RT =
- FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) {
- 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<PackedAttr>())
- 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<RecordType>()) {
+ 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<PackedAttr>())
+ 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::cxxLa
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::layoutNonV
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::final
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()) {
Modified: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=205994&r1=205993&r2=205994&view=diff
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Thu Apr 10 17:15:18 2014
@@ -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];
More information about the cfe-commits
mailing list