From e62fd7d3d76c9075bf2d5b8d8bf961e7ae52dc8f Mon Sep 17 00:00:00 2001 From: Warren Hunt Date: Fri, 21 Feb 2014 01:40:35 +0000 Subject: [PATCH] [MS-ABI] Update to zero-sized padding algorithm Slight change to the way zero-sized sub-objects are tracked in the presence of virtual bases. In addition we correctly distinguish between dsize and nvsize. addresses http://llvm.org/bugs/show_bug.cgi?id=18826 Unit tests are included. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@201832 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/RecordLayoutBuilder.cpp | 43 ++++++++++++----------- test/Layout/ms-x86-empty-virtual-base.cpp | 37 ++++++++++++++++++- 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index 139ee60ef8..a038e2dc6a 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2165,7 +2165,8 @@ 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); + ElementInfo getAdjustedElementInfo(const ASTRecordLayout &Layout, + bool AsBase = true); /// \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. @@ -2184,7 +2185,9 @@ public: const ASTContext &Context; /// \brief The size of the record being laid out. CharUnits Size; - /// \brief The data alignment of the record layout. + /// \brief The non-virtual size of the record layout. + CharUnits NonVirtualSize; + /// \brief The data size of the record layout. CharUnits DataSize; /// \brief The current alignment of the record layout. CharUnits Alignment; @@ -2236,7 +2239,7 @@ public: MicrosoftRecordLayoutBuilder::ElementInfo MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( - const ASTRecordLayout &Layout) { + const ASTRecordLayout &Layout, bool AsBase) { ElementInfo Info; Info.Alignment = Layout.getAlignment(); // Respect pragma pack. @@ -2250,7 +2253,7 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // 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(); + Info.Size = AsBase ? Layout.getNonVirtualSize() : Layout.getSize(); return Info; } @@ -2266,10 +2269,11 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( FD->getType()->getBaseElementTypeUnsafe()->getAs()) { const ASTRecordLayout &Layout = Context.getASTRecordLayout(RT->getDecl()); // Get the element info for a layout, respecting pack. - Info = getAdjustedElementInfo(Layout); - // Nomally getAdjustedElementInfo returns the non-virtual size, which is - // correct for bases but not for fields. - Info.Size = Context.getTypeInfoInChars(FD->getType()).first; + Info = getAdjustedElementInfo(Layout, false); + // If the field is an array type, scale it's size properly. + if (const ConstantArrayType *CAT = + dyn_cast(FD->getType())) + Info.Size = Info.Size * (int64_t)CAT->getSize().getZExtValue(); // Capture required alignment as a side-effect. RequiredAlignment = std::max(RequiredAlignment, Layout.getRequiredAlignment()); @@ -2317,7 +2321,7 @@ void MicrosoftRecordLayoutBuilder::cxxLayout(const CXXRecordDecl *RD) { layoutNonVirtualBases(RD); layoutFields(RD); injectVPtrs(RD); - DataSize = Size = Size.RoundUpToAlignment(Alignment); + NonVirtualSize = Size = Size.RoundUpToAlignment(Alignment); RequiredAlignment = std::max( RequiredAlignment, Context.toCharUnitsFromBits(RD->getMaxAlignment())); layoutVirtualBases(RD); @@ -2458,7 +2462,7 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase( ElementInfo Info = getAdjustedElementInfo(BaseLayout); CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); Bases.insert(std::make_pair(BaseDecl, BaseOffset)); - Size = BaseOffset + BaseLayout.getDataSize(); + Size = BaseOffset + BaseLayout.getNonVirtualSize(); PreviousBaseLayout = &BaseLayout; VBPtrOffset = Size; } @@ -2569,9 +2573,6 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) { i != e; ++i) if (i->second >= InjectionSite) i->second += Offset; - // The presence of a vbptr suppresses zero sized objects that are not in - // virtual bases. - HasZeroSizedSubObject = false; } void MicrosoftRecordLayoutBuilder::injectVFPtr(const CXXRecordDecl *RD) { @@ -2645,17 +2646,17 @@ void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) { // different from the general case layout but it may have to do with lazy // placement of zero sized bases. VBPtrOffset = Size; - if (LastBaseLayout && LastBaseLayout->getDataSize().isZero()) { + if (LastBaseLayout && LastBaseLayout->getNonVirtualSize().isZero()) { VBPtrOffset = Bases[LastBaseDecl]; - if (PenultBaseLayout && PenultBaseLayout->getDataSize().isZero()) + if (PenultBaseLayout && PenultBaseLayout->getNonVirtualSize().isZero()) VBPtrOffset = Bases[PenultBaseDecl]; } // Once we've located a spot for the vbptr, place it. VBPtrOffset = VBPtrOffset.RoundUpToAlignment(PointerInfo.Alignment); Size = VBPtrOffset + PointerInfo.Size; - if (LastBaseLayout && LastBaseLayout->getDataSize().isZero()) { + if (LastBaseLayout && LastBaseLayout->getNonVirtualSize().isZero()) { // Add the padding between zero sized bases after the vbptr. - if (PenultBaseLayout && PenultBaseLayout->getDataSize().isZero()) + if (PenultBaseLayout && PenultBaseLayout->getNonVirtualSize().isZero()) Size += CharUnits::One(); Size = Size.RoundUpToAlignment(LastBaseLayout->getRequiredAlignment()); Bases[LastBaseDecl] = Size; @@ -2715,11 +2716,12 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { if (HasVtordisp) Size = Size.RoundUpToAlignment(VtorDispAlignment) + VtorDispSize; // Insert the virtual base. + HasZeroSizedSubObject = false; ElementInfo Info = getAdjustedElementInfo(BaseLayout); CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment); VBases.insert(std::make_pair(BaseDecl, ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp))); - Size = BaseOffset + BaseLayout.getDataSize(); + Size = BaseOffset + BaseLayout.getNonVirtualSize(); PreviousBaseLayout = &BaseLayout; } } @@ -2727,6 +2729,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { // Respect required alignment. Note that in 32-bit mode Required alignment // may be 0 nad cause size not to be updated. + DataSize = Size; if (!RequiredAlignment.isZero()) { Alignment = std::max(Alignment, RequiredAlignment); Size = Size.RoundUpToAlignment(Alignment); @@ -2852,8 +2855,8 @@ ASTContext::BuildMicrosoftASTRecordLayout(const RecordDecl *D) const { *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment, Builder.HasOwnVFPtr, Builder.HasOwnVFPtr || Builder.PrimaryBase, - Builder.VBPtrOffset, Builder.DataSize, Builder.FieldOffsets.data(), - Builder.FieldOffsets.size(), Builder.DataSize, + Builder.VBPtrOffset, Builder.NonVirtualSize, Builder.FieldOffsets.data(), + Builder.FieldOffsets.size(), Builder.NonVirtualSize, Builder.Alignment, CharUnits::Zero(), Builder.PrimaryBase, false, Builder.SharedVBPtrBase, Builder.HasZeroSizedSubObject, Builder.LeadsWithZeroSizedBase, diff --git a/test/Layout/ms-x86-empty-virtual-base.cpp b/test/Layout/ms-x86-empty-virtual-base.cpp index 819246ee2a..267f82afef 100644 --- a/test/Layout/ms-x86-empty-virtual-base.cpp +++ b/test/Layout/ms-x86-empty-virtual-base.cpp @@ -730,6 +730,40 @@ struct T3 : virtual T1, virtual T0 { long long a; }; // CHECK-X64-NEXT: | [sizeof=24, align=8 // CHECK-X64-NEXT: | nvsize=16, nvalign=8] +struct Q0A {}; +struct Q0B { char Q0BField; }; +struct Q0C : virtual Q0A, virtual Q0B { char Q0CField; }; +struct Q0D : Q0C, Q0A {}; + +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct Q0D +// CHECK-NEXT: 0 | struct Q0C (base) +// CHECK-NEXT: 0 | (Q0C vbtable pointer) +// CHECK-NEXT: 4 | char Q0CField +// CHECK-NEXT: 8 | struct Q0A (base) (empty) +// CHECK-NEXT: 8 | struct Q0A (virtual base) (empty) +// CHECK-NEXT: 8 | struct Q0B (virtual base) +// CHECK-NEXT: 8 | char Q0BField +// CHECK-NEXT: | [sizeof=9, align=4 +// CHECK-NEXT: | nvsize=8, nvalign=4] +// 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 Q0D +// CHECK-X64-NEXT: 0 | struct Q0C (base) +// CHECK-X64-NEXT: 0 | (Q0C vbtable pointer) +// CHECK-X64-NEXT: 8 | char Q0CField +// CHECK-X64-NEXT: 16 | struct Q0A (base) (empty) +// CHECK-X64-NEXT: 16 | struct Q0A (virtual base) (empty) +// CHECK-X64-NEXT: 16 | struct Q0B (virtual base) +// CHECK-X64-NEXT: 16 | char Q0BField +// CHECK-X64-NEXT: | [sizeof=24, align=8 +// CHECK-X64-NEXT: | nvsize=16, nvalign=8] + int a[ sizeof(A)+ sizeof(B)+ @@ -753,4 +787,5 @@ sizeof(S)+ sizeof(T)+ sizeof(U)+ sizeof(V)+ -sizeof(T3)]; +sizeof(T3)+ +sizeof(Q0D)]; -- 2.40.0