From: Warren Hunt Date: Sat, 11 Jan 2014 01:16:40 +0000 (+0000) Subject: [ms-abi] Change the way alignment is tracked X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c3a5420a77b7b578f07cfbd8054612f45aa0c939;p=clang [ms-abi] Change the way alignment is tracked This patch more cleanly seperates the concepts of Preferred Alignment and Required Alignment. Most notable that changes to Required Alignment do *not* impact preferred alignment until late in struct layout. This is observable when using pragma pack and non-virtual bases and the use of tail padding when laying them out. Test cases included. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@198988 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index 6a708a7e8d..ff6b950474 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2077,10 +2077,6 @@ public: /// __declspec(align) into account. It also updates RequiredAlignment as a /// side effect because it is most convenient to do so here. ElementInfo getAdjustedElementInfo(const FieldDecl *FD); - /// \brief Updates the alignment of the record. - void updateAlignment(CharUnits MemberAlignment) { - Alignment = std::max(Alignment, MemberAlignment); - } /// \brief Places a field at an offset in CharUnits. void placeFieldAtOffset(CharUnits FieldOffset) { FieldOffsets.push_back(Context.toBits(FieldOffset)); @@ -2157,7 +2153,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( if (Layout.hasZeroSizedSubObject()) HasZeroSizedSubObject = true; // Respect required alignment, this is necessary because we may have adjusted - // the alignment in the case of pragam pack. + // 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); Info.Alignment = std::max(Info.Alignment, Layout.getRequiredAlignment()); Info.Size = Layout.getDataSize(); return Info; @@ -2204,6 +2202,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // Capture required alignment as a side-effect. RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment); } + // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. + if (!(FD->isBitField() && IsUnion)) + Alignment = std::max(Alignment, Info.Alignment); return Info; } @@ -2356,7 +2357,6 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); Bases.insert(std::make_pair(BaseDecl, BaseOffset)); Size = BaseOffset + BaseLayout.getDataSize(); - updateAlignment(Info.Alignment); PreviousBaseLayout = &BaseLayout; VBPtrOffset = Size; } @@ -2384,7 +2384,6 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { placeFieldAtOffset(FieldOffset); Size = FieldOffset + Info.Size; } - updateAlignment(Info.Alignment); } void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { @@ -2412,13 +2411,11 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { if (IsUnion) { placeFieldAtOffset(CharUnits::Zero()); Size = std::max(Size, Info.Size); - // TODO: Add a Sema warning that MS ignores bitfield alignment in unions. } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.RoundUpToAlignment(Info.Alignment); placeFieldAtOffset(FieldOffset); Size = FieldOffset + Info.Size; - updateAlignment(Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; } } @@ -2443,7 +2440,6 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { CharUnits FieldOffset = Size.RoundUpToAlignment(Info.Alignment); placeFieldAtOffset(FieldOffset); Size = FieldOffset; - updateAlignment(Info.Alignment); } } @@ -2475,8 +2471,6 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) { i != e; ++i) if (i->second >= InjectionSite) i->second += Offset; - // Update the object alignment. - updateAlignment(PointerInfo.Alignment); // The presence of a vbptr suppresses zero sized objects that are not in // virtual bases. HasZeroSizedSubObject = false; @@ -2500,7 +2494,6 @@ void MicrosoftRecordLayoutBuilder::injectVFPtr(const CXXRecordDecl *RD) { for (BaseOffsetsMapTy::iterator i = Bases.begin(), e = Bases.end(); i != e; ++i) i->second += Offset; - updateAlignment(PointerInfo.Alignment); } void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) { @@ -2512,6 +2505,7 @@ void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) { // the record. injectVBPtr(RD); injectVFPtr(RD); + Alignment = std::max(Alignment, PointerInfo.Alignment); return; } // In 64-bit mode, structs with RequiredAlignment greater than 8 get special @@ -2521,7 +2515,7 @@ void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) { FieldOffsets.clear(); Bases.clear(); Size = CharUnits::Zero(); - updateAlignment(PointerInfo.Alignment); + Alignment = std::max(Alignment, PointerInfo.Alignment); if (HasOwnVFPtr) Size = PointerInfo.Size; layoutNonVirtualBases(RD); @@ -2619,7 +2613,6 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { VBases.insert(std::make_pair(BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); Size = BaseOffset + BaseLayout.getDataSize(); - updateAlignment(Info.Alignment); PreviousBaseLayout = &BaseLayout; } } diff --git a/test/Layout/ms-x86-pack-and-align.cpp b/test/Layout/ms-x86-pack-and-align.cpp index fba485f5a2..5c2710e6d6 100644 --- a/test/Layout/ms-x86-pack-and-align.cpp +++ b/test/Layout/ms-x86-pack-and-align.cpp @@ -74,7 +74,7 @@ struct Y : A, B { // CHECK-NEXT: 12 | char a // CHECK-NEXT: 14 | int b // CHECK-NEXT: | [sizeof=20, align=4 -// CHECK-NEXT: | nvsize=20, nvalign=4] +// CHECK-NEXT: | nvsize=18, nvalign=4] // CHECK-X64: *** Dumping AST Record Layout // CHECK-X64: *** Dumping AST Record Layout // CHECK-X64-NEXT: 0 | struct Y @@ -85,7 +85,7 @@ struct Y : A, B { // CHECK-X64-NEXT: 12 | char a // CHECK-X64-NEXT: 14 | int b // CHECK-X64-NEXT: | [sizeof=20, align=4 -// CHECK-X64-NEXT: | nvsize=20, nvalign=4] +// CHECK-X64-NEXT: | nvsize=18, nvalign=4] struct Z : virtual B { char a; @@ -291,6 +291,36 @@ struct YF { // CHECK-X64: | [sizeof=5, align=1 // CHECK-X64-NEXT: | nvsize=5, nvalign=1] +#pragma pack(16) +struct __declspec(align(16)) D0 { char a; }; +#pragma pack(1) +struct D1 : public D0 { char a; }; +#pragma pack(16) +struct D2 : D1 { char a; }; + +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct D2 +// CHECK-NEXT: 0 | struct D1 (base) +// CHECK-NEXT: 0 | struct D0 (base) +// CHECK-NEXT: 0 | char a +// CHECK-NEXT: 1 | char a +// CHECK-NEXT: 2 | char a +// CHECK-NEXT: | [sizeof=16, align=16 +// CHECK-NEXT: | nvsize=16, nvalign=16] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct D2 +// CHECK-X64-NEXT: 0 | struct D1 (base) +// CHECK-X64-NEXT: 0 | struct D0 (base) +// CHECK-X64-NEXT: 0 | char a +// CHECK-X64-NEXT: 1 | char a +// CHECK-X64-NEXT: 2 | char a +// CHECK-X64-NEXT: | [sizeof=16, align=16 +// CHECK-X64-NEXT: | nvsize=16, nvalign=16] + int a[ sizeof(X)+ sizeof(Y)+ @@ -303,4 +333,6 @@ sizeof(YC)+ sizeof(YD)+ sizeof(YE)+ sizeof(YF)+ +sizeof(YF)+ +sizeof(D2)+ 0];