From: Warren Hunt Date: Fri, 6 Dec 2013 19:54:25 +0000 (+0000) Subject: [MS-ABI] Fix alias-avoidance padding between bases X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=14bff0de022021931d9978d0c9d326b85f81b515;p=clang [MS-ABI] Fix alias-avoidance padding between bases Adds padding between bases or virtual bases in an attempt to avoid aliasing of zero-sized sub-objects. The approach used by the ABI adds two more bits of state. Detailed comments are in the code. Test cases included. Differential Revision: http://llvm-reviews.chandlerc.com/D2258 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@196602 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/RecordLayout.h b/include/clang/AST/RecordLayout.h index 5ee87b9184..1eae8c057c 100644 --- a/include/clang/AST/RecordLayout.h +++ b/include/clang/AST/RecordLayout.h @@ -104,6 +104,14 @@ private: /// a primary base class. bool HasExtendableVFPtr : 1; + /// HasZeroSizedSubObject - True if this class contains a zero sized member or base or a base + /// with a zero sized member or base. Only used for MS-ABI. + bool HasZeroSizedSubObject : 1; + + /// \brief True if this class is zero sized or first base is zero sized or + /// has this property. Only used for MS-ABI. + bool LeadsWithZeroSizedBase : 1; + /// PrimaryBase - The primary base info for this record. llvm::PointerIntPair PrimaryBase; @@ -145,6 +153,8 @@ private: const CXXRecordDecl *PrimaryBase, bool IsPrimaryBaseVirtual, const CXXRecordDecl *BaseSharingVBPtr, + bool HasZeroSizedSubObject, + bool LeadsWithZeroSizedBase, const BaseOffsetsMapTy& BaseOffsets, const VBaseOffsetsMapTy& VBaseOffsets); @@ -272,6 +282,16 @@ public: return RequiredAlignment; } + bool hasZeroSizedSubObject() const { + assert(CXXInfo && "Record layout does not have C++ specific info!"); + return CXXInfo->HasZeroSizedSubObject; + } + + bool leadsWithZeroSizedBase() const { + assert(CXXInfo && "Record layout does not have C++ specific info!"); + return CXXInfo->LeadsWithZeroSizedBase; + } + /// getVBPtrOffset - Get the offset for virtual base table pointer. /// This is only meaningful with the Microsoft ABI. CharUnits getVBPtrOffset() const { diff --git a/lib/AST/RecordLayout.cpp b/lib/AST/RecordLayout.cpp index a4a59c4f33..d72d5dcce0 100644 --- a/lib/AST/RecordLayout.cpp +++ b/lib/AST/RecordLayout.cpp @@ -58,6 +58,8 @@ ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, const CXXRecordDecl *PrimaryBase, bool IsPrimaryBaseVirtual, const CXXRecordDecl *BaseSharingVBPtr, + bool HasZeroSizedSubObject, + bool LeadsWithZeroSizedBase, const BaseOffsetsMapTy& BaseOffsets, const VBaseOffsetsMapTy& VBaseOffsets) : Size(size), DataSize(datasize), Alignment(alignment), @@ -80,6 +82,8 @@ ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CXXInfo->VBPtrOffset = vbptroffset; CXXInfo->HasExtendableVFPtr = hasExtendableVFPtr; CXXInfo->BaseSharingVBPtr = BaseSharingVBPtr; + CXXInfo->HasZeroSizedSubObject = HasZeroSizedSubObject; + CXXInfo->LeadsWithZeroSizedBase = LeadsWithZeroSizedBase; #ifndef NDEBUG diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index a1f6c41ef8..ec79156489 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2019,6 +2019,15 @@ static bool isMsLayout(const RecordDecl* D) { // obvious reason. // * When laying out empty non-virtual bases, an extra byte of padding is added // if the non-virtual base before the empty non-virtual base has a vbptr. +// * The ABI attempts to avoid aliasing of zero sized bases by adding padding +// between bases or vbases with specific properties. The criteria for +// additional padding between two bases is that the first base is zero sized +// or has a zero sized subobject and the second base is zero sized or leads +// with a zero sized base (sharing of vfptrs can reorder the layout of the +// so the leading base is not always the first one declared). The padding +// added for bases is 1 byte. The padding added for vbases depends on the +// alignment of the object but is at least 4 bytes (in both 32 and 64 bit +// modes). namespace { @@ -2139,13 +2148,17 @@ public: CharUnits PointerAlignment; /// \brief Holds an empty base we haven't yet laid out. const CXXRecordDecl *LazyEmptyBase; - /// \brief Lets us know if the last base we laid out was empty. Only used - /// when adjusting the placement of a last zero-sized base in 64 bit mode. - bool LastBaseWasEmpty; + /// \brief A pointer to the Layout for the most recent non-virtual base that + /// has been laid out. + const ASTRecordLayout *PreviousBaseLayout; /// \brief Lets us know if we're in 64-bit mode - bool Is64BitMode; - /// \brief True if the last non-virtual base has a vbptr. - bool LastNonVirtualBaseHasVBPtr; + bool Is64BitMode : 1; + /// \brief True if this class contains a zero sized member or base or a base + /// with a zero sized member or base. Only used for MS-ABI. + bool HasZeroSizedSubObject : 1; + /// \brief True if this class is zero sized or first base is zero sized or + /// has this property. Only used for MS-ABI. + bool LeadsWithZeroSizedBase : 1; }; } // namespace @@ -2164,18 +2177,6 @@ MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) { std::pair FieldInfo = Context.getTypeInfoInChars(FD->getType()); - // If we're not on win32 and using ms_struct the field alignment will be wrong - // for 64 bit types, so we fix that here. - if (FD->getASTContext().getTargetInfo().getTriple().getOS() != - llvm::Triple::Win32) { - QualType T = Context.getBaseElementType(FD->getType()); - if (const BuiltinType *BTy = T->getAs()) { - CharUnits TypeSize = Context.getTypeSizeInChars(BTy); - if (TypeSize > FieldInfo.second) - FieldInfo.second = TypeSize; - } - } - // Respect packed attribute. if (FD->hasAttr()) FieldInfo.second = CharUnits::One(); @@ -2188,7 +2189,7 @@ MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) { RequiredAlignment = std::max(RequiredAlignment, FieldAlign); FieldInfo.second = std::max(FieldInfo.second, FieldAlign); } - // Respect attributes applied inside field base types. + // Respect attributes applied to subobjects of the field. if (const RecordType *RT = FD->getType()->getBaseElementTypeUnsafe()->getAs()) { const ASTRecordLayout &Layout = Context.getASTRecordLayout(RT->getDecl()); @@ -2196,6 +2197,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) { Layout.getRequiredAlignment()); FieldInfo.second = std::max(FieldInfo.second, Layout.getRequiredAlignment()); + // Track zero-sized subobjects here where it's already avaliable. + if (Layout.hasZeroSizedSubObject()) + HasZeroSizedSubObject = true; } return FieldInfo; } @@ -2206,10 +2210,12 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) { Size = CharUnits::Zero(); Alignment = CharUnits::One(); + // In 64-bit mode we always perform an alignment step after laying out vbases. // In 32-bit mode we do not. The check to see if we need to perform alignment // checks the RequiredAlignment field and performs alignment if it isn't 0. RequiredAlignment = Is64BitMode ? CharUnits::One() : CharUnits::Zero(); + HasZeroSizedSubObject = false; // Compute the maximum field alignment. MaxFieldAlignment = CharUnits::Zero(); @@ -2259,6 +2265,7 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { SharedVBPtrBase = 0; PrimaryBase = 0; VirtualAlignment = CharUnits::One(); + LeadsWithZeroSizedBase = false; // If the record has a dynamic base class, attempt to choose a primary base // class. It is the first (in direct base class order) non-virtual dynamic @@ -2272,6 +2279,9 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) { // Handle required alignment. RequiredAlignment = std::max(RequiredAlignment, Layout.getRequiredAlignment()); + // Check for zero sized subobjects + if (Layout.hasZeroSizedSubObject()) + HasZeroSizedSubObject = true; // Handle virtual bases. if (i->isVirtual()) { VirtualAlignment = std::max(VirtualAlignment, getBaseAlignment(Layout)); @@ -2329,13 +2339,13 @@ void MicrosoftRecordLayoutBuilder::layoutVFPtr(const CXXRecordDecl *RD) { void MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { LazyEmptyBase = 0; - LastBaseWasEmpty = false; - LastNonVirtualBaseHasVBPtr = false; + PreviousBaseLayout = 0; // Lay out the primary base first. if (PrimaryBase) layoutNonVirtualBase(PrimaryBase); + const CXXRecordDecl *LeadingBase = PrimaryBase; // Iterate through the bases and lay out the non-virtual ones. for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(), e = RD->bases_end(); @@ -2344,6 +2354,11 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { continue; const CXXRecordDecl *BaseDecl = cast(i->getType()->castAs()->getDecl()); + if (!LeadingBase) { + const ASTRecordLayout &Layout = Context.getASTRecordLayout(BaseDecl); + LeadsWithZeroSizedBase = Layout.leadsWithZeroSizedBase(); + LeadingBase = BaseDecl; + } if (BaseDecl != PrimaryBase) layoutNonVirtualBase(BaseDecl); } @@ -2360,16 +2375,14 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(const CXXRecordDecl *RD) { Size = Size.RoundUpToAlignment(getBaseAlignment(LazyLayout)); // If the last non-virtual base has a vbptr we add a byte of padding for no // obvious reason. - if (LastNonVirtualBaseHasVBPtr) + if (PreviousBaseLayout && PreviousBaseLayout->hasVBPtr()) Size++; Bases.insert(std::make_pair(LazyEmptyBase, Size)); // Empty bases only consume space when followed by another empty base. - if (RD && Layout->getNonVirtualSize().isZero()) { - LastBaseWasEmpty = true; + if (RD && Layout->getNonVirtualSize().isZero()) Size++; - } LazyEmptyBase = 0; - LastNonVirtualBaseHasVBPtr = false; + PreviousBaseLayout = &LazyLayout; } // RD is null when flushing the final lazy base. @@ -2381,14 +2394,19 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(const CXXRecordDecl *RD) { return; } + // 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() && + Layout->leadsWithZeroSizedBase()) + Size++; // Insert the base here. CharUnits BaseOffset = Size.RoundUpToAlignment(getBaseAlignment(*Layout)); Bases.insert(std::make_pair(RD, BaseOffset)); Size = BaseOffset + Layout->getDataSize(); // Note: we don't update alignment here because it was accounted - // for during initialization. - LastBaseWasEmpty = false; - LastNonVirtualBaseHasVBPtr = Layout->hasVBPtr(); + // for during initalization. + PreviousBaseLayout = Layout; } void MicrosoftRecordLayoutBuilder::layoutVBPtr(const CXXRecordDecl *RD) { @@ -2410,10 +2428,11 @@ void MicrosoftRecordLayoutBuilder::layoutVBPtr(const CXXRecordDecl *RD) { // on if the second to last base was also zero sized. Size += OldSize % BasesAndFieldsAlignment.getQuantity(); } else { - if (Is64BitMode) - Size += LastBaseWasEmpty ? CharUnits::One() : CharUnits::Zero(); - else + if (!Is64BitMode) Size = OldSize + BasesAndFieldsAlignment; + else if (PreviousBaseLayout && + PreviousBaseLayout->getNonVirtualSize().isZero()) + Size += CharUnits::One(); } updateAlignment(PointerAlignment); } @@ -2529,6 +2548,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { return; updateAlignment(VirtualAlignment); + PreviousBaseLayout = 0; // Zero-sized v-bases obey the alignment attribute so apply it here. The // alignment attribute is normally accounted for in FinalizeLayout. @@ -2555,31 +2575,15 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { void MicrosoftRecordLayoutBuilder::layoutVirtualBase(const CXXRecordDecl *RD, bool HasVtordisp) { - if (LazyEmptyBase) { - const ASTRecordLayout &LazyLayout = - Context.getASTRecordLayout(LazyEmptyBase); - Size = Size.RoundUpToAlignment(getBaseAlignment(LazyLayout)); - VBases.insert( - std::make_pair(LazyEmptyBase, ASTRecordLayout::VBaseInfo(Size, false))); - // Empty bases only consume space when followed by another empty base. - // The space consumed is in an Alignment sized/aligned block and the v-base - // is placed at its alignment offset into the chunk, unless its alignment - // is less than 4 bytes, at which it is placed at 4 byte offset in the - // chunk. We have no idea why. - if (RD && Context.getASTRecordLayout(RD).getNonVirtualSize().isZero()) - Size = Size.RoundUpToAlignment(Alignment) + CharUnits::fromQuantity(4); - LazyEmptyBase = 0; - } - - // RD is null when flushing the final lazy virtual base. - if (!RD) - return; - const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); - if (Layout.getNonVirtualSize().isZero() && !HasVtordisp) { - LazyEmptyBase = RD; - return; - } + // 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. The minimal padding between virtual bases is 4 + // bytes (in both 32 and 64 bits modes), we don't know why. + if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() && + Layout.leadsWithZeroSizedBase()) + Size = Size.RoundUpToAlignment(Alignment) + + std::max(CharUnits::fromQuantity(4), Layout.getAlignment()); CharUnits BaseNVSize = Layout.getNonVirtualSize(); CharUnits BaseAlign = getBaseAlignment(Layout); @@ -2596,17 +2600,19 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBase(const CXXRecordDecl *RD, Size = BaseOffset + BaseNVSize; // Note: we don't update alignment here because it was accounted for in // InitializeLayout. + + PreviousBaseLayout = &Layout; } void MicrosoftRecordLayoutBuilder::finalizeCXXLayout(const CXXRecordDecl *RD) { - // Flush the lazy virtual base. - layoutVirtualBase(0, false); - if (RD->vbases_begin() == RD->vbases_end() || !RequiredAlignment.isZero()) Size = Size.RoundUpToAlignment(Alignment); - if (Size.isZero()) + if (Size.isZero()) { + HasZeroSizedSubObject = true; + LeadsWithZeroSizedBase = true; Size = Alignment; + } } void MicrosoftRecordLayoutBuilder::honorDeclspecAlign(const RecordDecl *RD) { @@ -2713,8 +2719,9 @@ ASTContext::BuildMicrosoftASTRecordLayout(const RecordDecl *D) const { Builder.VBPtrOffset, Builder.DataSize, Builder.FieldOffsets.data(), Builder.FieldOffsets.size(), Builder.DataSize, Builder.NonVirtualAlignment, CharUnits::Zero(), Builder.PrimaryBase, - false, Builder.SharedVBPtrBase, Builder.Bases, - Builder.VBases); + false, Builder.SharedVBPtrBase, + Builder.HasZeroSizedSubObject, Builder.LeadsWithZeroSizedBase, + Builder.Bases, Builder.VBases); } else { Builder.layout(D); return new (*this) ASTRecordLayout( @@ -2783,7 +2790,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { EmptySubobjects.SizeOfLargestEmptySubobject, Builder.PrimaryBase, Builder.PrimaryBaseIsVirtual, - 0, + 0, false, false, Builder.Bases, Builder.VBases); } else { RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0); diff --git a/test/Layout/ms-x86-alias-avoidance-padding.cpp b/test/Layout/ms-x86-alias-avoidance-padding.cpp new file mode 100644 index 0000000000..86b3cebe24 --- /dev/null +++ b/test/Layout/ms-x86-alias-avoidance-padding.cpp @@ -0,0 +1,178 @@ +// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only -cxx-abi microsoft %s 2>/dev/null \ +// RUN: | FileCheck %s +// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only -cxx-abi microsoft %s 2>/dev/null \ +// RUN: | FileCheck %s -check-prefix CHECK-X64 + +extern "C" int printf(const char *fmt, ...); +__declspec(align(4096)) char buffer[4096]; + +struct AT {}; + +struct V : AT { + char c; + V() { + printf("V - this: %d\n", (int)((char*)this - buffer)); + } +}; + +struct AT0 { + union { struct { int a; AT t; } y; int b; } x; + char c; + AT0() { + printf("AT0 - this: %d\n", (int)((char*)this - buffer)); + } +}; + +struct AT1 : V { + int a; + AT1() { + printf("AT1 - this: %d\n", (int)((char*)this - buffer)); + } +}; + +struct AT2 { + AT0 t; + char AT2FieldName0; + AT2() { + printf("AT2 - this: %d\n", (int)((char*)this - buffer)); + printf("AT2 - Fiel: %d\n", (int)((char*)&AT2FieldName0 - buffer)); + } +}; + +struct AT3 : AT2, AT1 { + AT3() { + printf("AT3 - this: %d\n", (int)((char*)this - buffer)); + } +}; + +// CHECK: *** Dumping AST Record Layout +// CHECK: 0 | struct AT3 +// CHECK: 0 | struct AT2 (base) +// CHECK: 0 | struct AT0 t +// CHECK: 0 | union AT0::