From 2782dacfbcb48a1430b9114d801ba90fc2babd69 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Wed, 26 Jun 2013 20:50:34 +0000 Subject: [PATCH] Rewrite record layout for ms_struct structs. The old implementation of ms_struct in RecordLayoutBuilder was a complete mess: it depended on complicated conditionals which didn't really reflect the underlying logic, and placed a burden on users of the resulting RecordLayout. This commit rips out almost all of the old code, and replaces it with simple checks in RecordLayoutBuilder::LayoutBitField. This commit also fixes , a bug where class inheritance would cause us to lay out bitfields incorrectly. llvm-svn: 185018 --- clang/include/clang/AST/ASTContext.h | 25 -- clang/lib/AST/ASTContext.cpp | 32 --- clang/lib/AST/Decl.cpp | 14 +- clang/lib/AST/RecordLayoutBuilder.cpp | 240 +++++++------------- clang/lib/CodeGen/CGDebugInfo.cpp | 11 - clang/lib/CodeGen/CGExprConstant.cpp | 32 +-- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 35 +-- clang/lib/CodeGen/CodeGenTBAA.cpp | 21 -- clang/test/CodeGen/tbaa-struct.cpp | 2 +- clang/test/CodeGen/tbaa.cpp | 2 +- clang/test/SemaCXX/ms_struct.cpp | 19 ++ 11 files changed, 104 insertions(+), 329 deletions(-) create mode 100644 clang/test/SemaCXX/ms_struct.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index ae7a8f14d44f..ea772647f93e 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -634,31 +634,6 @@ public: void setInstantiatedFromUnnamedFieldDecl(FieldDecl *Inst, FieldDecl *Tmpl); - /// \brief Return \c true if \p FD is a zero-length bitfield which follows - /// the non-bitfield \p LastFD. - bool ZeroBitfieldFollowsNonBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const; - - /// \brief Return \c true if \p FD is a zero-length bitfield which follows - /// the bitfield \p LastFD. - bool ZeroBitfieldFollowsBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const; - - /// \brief Return \c true if \p FD is a bitfield which follows the bitfield - /// \p LastFD. - bool BitfieldFollowsBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const; - - /// \brief Return \c true if \p FD is not a bitfield which follows the - /// bitfield \p LastFD. - bool NonBitfieldFollowsBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const; - - /// \brief Return \c true if \p FD is a bitfield which follows the - /// non-bitfield \p LastFD. - bool BitfieldFollowsNonBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const; - // Access to the set of methods overridden by the given C++ method. typedef CXXMethodVector::const_iterator overridden_cxx_method_iterator; overridden_cxx_method_iterator diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index b5ef5fa2adc4..44cb32db01a2 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -1132,38 +1132,6 @@ void ASTContext::setInstantiatedFromUnnamedFieldDecl(FieldDecl *Inst, InstantiatedFromUnnamedFieldDecl[Inst] = Tmpl; } -bool ASTContext::ZeroBitfieldFollowsNonBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const { - return (FD->isBitField() && LastFD && !LastFD->isBitField() && - FD->getBitWidthValue(*this) == 0); -} - -bool ASTContext::ZeroBitfieldFollowsBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const { - return (FD->isBitField() && LastFD && LastFD->isBitField() && - FD->getBitWidthValue(*this) == 0 && - LastFD->getBitWidthValue(*this) != 0); -} - -bool ASTContext::BitfieldFollowsBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const { - return (FD->isBitField() && LastFD && LastFD->isBitField() && - FD->getBitWidthValue(*this) && - LastFD->getBitWidthValue(*this)); -} - -bool ASTContext::NonBitfieldFollowsBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const { - return (!FD->isBitField() && LastFD && LastFD->isBitField() && - LastFD->getBitWidthValue(*this)); -} - -bool ASTContext::BitfieldFollowsNonBitfield(const FieldDecl *FD, - const FieldDecl *LastFD) const { - return (FD->isBitField() && LastFD && !LastFD->isBitField() && - FD->getBitWidthValue(*this)); -} - ASTContext::overridden_cxx_method_iterator ASTContext::overridden_methods_begin(const CXXMethodDecl *Method) const { llvm::DenseMap::const_iterator Pos diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 9702d0324534..f934c76a4193 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -2917,23 +2917,11 @@ unsigned FieldDecl::getFieldIndex() const { unsigned Index = 0; const RecordDecl *RD = getParent(); - const FieldDecl *LastFD = 0; - bool IsMsStruct = RD->isMsStruct(getASTContext()); for (RecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end(); - I != E; ++I, ++Index) { + I != E; ++I, ++Index) I->CachedFieldIndex = Index + 1; - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are ignored. - if (getASTContext().ZeroBitfieldFollowsNonBitfield(*I, LastFD)) { - --Index; - continue; - } - LastFD = *I; - } - } - assert(CachedFieldIndex && "failed to find field in parent"); return CachedFieldIndex - 1; } diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index c4ee565c8ad0..3a91c6be91de 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -573,10 +573,14 @@ protected: unsigned IsMsStruct : 1; - /// UnfilledBitsInLastByte - If the last field laid out was a bitfield, - /// this contains the number of bits in the last byte that can be used for - /// an adjacent bitfield if necessary. - unsigned char UnfilledBitsInLastByte; + /// UnfilledBitsInLastUnit - If the last field laid out was a bitfield, + /// this contains the number of bits in the last unit that can be used for + /// an adjacent bitfield if necessary. The unit in question is usually + /// a byte, but larger units are used if IsMsStruct. + unsigned char UnfilledBitsInLastUnit; + /// LastBitfieldTypeSize - If IsMsStruct, represents the size of the type + /// of the previous field if it was a bitfield. + unsigned char LastBitfieldTypeSize; /// MaxFieldAlignment - The maximum allowed field alignment. This is set by /// #pragma pack. @@ -646,7 +650,8 @@ protected: Alignment(CharUnits::One()), UnpackedAlignment(CharUnits::One()), ExternalLayout(false), InferAlignment(false), Packed(false), IsUnion(false), IsMac68kAlign(false), IsMsStruct(false), - UnfilledBitsInLastByte(0), MaxFieldAlignment(CharUnits::Zero()), + UnfilledBitsInLastUnit(0), LastBitfieldTypeSize(0), + MaxFieldAlignment(CharUnits::Zero()), DataSize(0), NonVirtualSize(CharUnits::Zero()), NonVirtualAlignment(CharUnits::One()), ZeroLengthBitfield(0), PrimaryBase(0), @@ -1728,123 +1733,16 @@ void RecordLayoutBuilder::Layout(const ObjCInterfaceDecl *D) { void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) { // Layout each field, for now, just sequentially, respecting alignment. In // the future, this will need to be tweakable by targets. - const FieldDecl *LastFD = 0; ZeroLengthBitfield = 0; - unsigned RemainingInAlignment = 0; for (RecordDecl::field_iterator Field = D->field_begin(), FieldEnd = D->field_end(); Field != FieldEnd; ++Field) { - if (IsMsStruct) { - FieldDecl *FD = *Field; - if (Context.ZeroBitfieldFollowsBitfield(FD, LastFD)) - ZeroLengthBitfield = FD; - // Zero-length bitfields following non-bitfield members are - // ignored: - else if (Context.ZeroBitfieldFollowsNonBitfield(FD, LastFD)) - continue; - // FIXME. streamline these conditions into a simple one. - else if (Context.BitfieldFollowsBitfield(FD, LastFD) || - Context.BitfieldFollowsNonBitfield(FD, LastFD) || - Context.NonBitfieldFollowsBitfield(FD, LastFD)) { - // 1) Adjacent bit fields are packed into the same 1-, 2-, or - // 4-byte allocation unit if the integral types are the same - // size and if the next bit field fits into the current - // allocation unit without crossing the boundary imposed by the - // common alignment requirements of the bit fields. - // 2) Establish a new alignment for a bitfield following - // a non-bitfield if size of their types differ. - // 3) Establish a new alignment for a non-bitfield following - // a bitfield if size of their types differ. - std::pair FieldInfo = - Context.getTypeInfo(FD->getType()); - uint64_t TypeSize = FieldInfo.first; - unsigned FieldAlign = FieldInfo.second; - // This check is needed for 'long long' in -m32 mode. - if (TypeSize > FieldAlign && - (Context.hasSameType(FD->getType(), - Context.UnsignedLongLongTy) - ||Context.hasSameType(FD->getType(), - Context.LongLongTy))) - FieldAlign = TypeSize; - FieldInfo = Context.getTypeInfo(LastFD->getType()); - uint64_t TypeSizeLastFD = FieldInfo.first; - unsigned FieldAlignLastFD = FieldInfo.second; - // This check is needed for 'long long' in -m32 mode. - if (TypeSizeLastFD > FieldAlignLastFD && - (Context.hasSameType(LastFD->getType(), - Context.UnsignedLongLongTy) - || Context.hasSameType(LastFD->getType(), - Context.LongLongTy))) - FieldAlignLastFD = TypeSizeLastFD; - - if (TypeSizeLastFD != TypeSize) { - if (RemainingInAlignment && - LastFD && LastFD->isBitField() && - LastFD->getBitWidthValue(Context)) { - // If previous field was a bitfield with some remaining unfilled - // bits, pad the field so current field starts on its type boundary. - uint64_t FieldOffset = - getDataSizeInBits() - UnfilledBitsInLastByte; - uint64_t NewSizeInBits = RemainingInAlignment + FieldOffset; - setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, - Context.getTargetInfo().getCharAlign())); - setSize(std::max(getSizeInBits(), getDataSizeInBits())); - RemainingInAlignment = 0; - } - - uint64_t UnpaddedFieldOffset = - getDataSizeInBits() - UnfilledBitsInLastByte; - FieldAlign = std::max(FieldAlign, FieldAlignLastFD); - - // The maximum field alignment overrides the aligned attribute. - if (!MaxFieldAlignment.isZero()) { - unsigned MaxFieldAlignmentInBits = - Context.toBits(MaxFieldAlignment); - FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits); - } - - uint64_t NewSizeInBits = - llvm::RoundUpToAlignment(UnpaddedFieldOffset, FieldAlign); - setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, - Context.getTargetInfo().getCharAlign())); - UnfilledBitsInLastByte = getDataSizeInBits() - NewSizeInBits; - setSize(std::max(getSizeInBits(), getDataSizeInBits())); - } - if (FD->isBitField()) { - uint64_t FieldSize = FD->getBitWidthValue(Context); - assert (FieldSize > 0 && "LayoutFields - ms_struct layout"); - if (RemainingInAlignment < FieldSize) - RemainingInAlignment = TypeSize - FieldSize; - else - RemainingInAlignment -= FieldSize; - } - } - else if (FD->isBitField()) { - uint64_t FieldSize = FD->getBitWidthValue(Context); - std::pair FieldInfo = - Context.getTypeInfo(FD->getType()); - uint64_t TypeSize = FieldInfo.first; - RemainingInAlignment = TypeSize - FieldSize; - } - LastFD = FD; - } - else if (!Context.getTargetInfo().useBitFieldTypeAlignment() && - Context.getTargetInfo().useZeroLengthBitfieldAlignment()) { + if (!Context.getTargetInfo().useBitFieldTypeAlignment() && + Context.getTargetInfo().useZeroLengthBitfieldAlignment()) { if (Field->isBitField() && Field->getBitWidthValue(Context) == 0) ZeroLengthBitfield = *Field; } LayoutField(*Field); } - if (IsMsStruct && RemainingInAlignment && - LastFD && LastFD->isBitField() && LastFD->getBitWidthValue(Context)) { - // If we ended a bitfield before the full length of the type then - // pad the struct out to the full length of the last type. - uint64_t FieldOffset = - getDataSizeInBits() - UnfilledBitsInLastByte; - uint64_t NewSizeInBits = RemainingInAlignment + FieldOffset; - setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, - Context.getTargetInfo().getCharAlign())); - setSize(std::max(getSizeInBits(), getDataSizeInBits())); - } } void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, @@ -1878,10 +1776,11 @@ void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, CharUnits TypeAlign = Context.getTypeAlignInChars(Type); // We're not going to use any of the unfilled bits in the last byte. - UnfilledBitsInLastByte = 0; + UnfilledBitsInLastUnit = 0; + LastBitfieldTypeSize = 0; uint64_t FieldOffset; - uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastByte; + uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit; if (IsUnion) { setDataSize(std::max(getDataSizeInBits(), FieldSize)); @@ -1896,7 +1795,7 @@ void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, Context.getTargetInfo().getCharAlign())); - UnfilledBitsInLastByte = getDataSizeInBits() - NewSizeInBits; + UnfilledBitsInLastUnit = getDataSizeInBits() - NewSizeInBits; } // Place this field at the current location. @@ -1914,47 +1813,37 @@ void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize, void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { bool FieldPacked = Packed || D->hasAttr(); - uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastByte; - uint64_t FieldOffset = IsUnion ? 0 : UnpaddedFieldOffset; uint64_t FieldSize = D->getBitWidthValue(Context); - std::pair FieldInfo = Context.getTypeInfo(D->getType()); uint64_t TypeSize = FieldInfo.first; unsigned FieldAlign = FieldInfo.second; - - // This check is needed for 'long long' in -m32 mode. - if (IsMsStruct && (TypeSize > FieldAlign) && - (Context.hasSameType(D->getType(), - Context.UnsignedLongLongTy) - || Context.hasSameType(D->getType(), Context.LongLongTy))) + + if (IsMsStruct) { + // The field alignment for integer types in ms_struct structs is + // always the size. FieldAlign = TypeSize; + // Ignore zero-length bitfields after non-bitfields in ms_struct structs. + if (!FieldSize && !LastBitfieldTypeSize) + FieldAlign = 1; + // If a bitfield is followed by a bitfield of a different size, don't + // pack the bits together in ms_struct structs. + if (LastBitfieldTypeSize != TypeSize) { + UnfilledBitsInLastUnit = 0; + LastBitfieldTypeSize = 0; + } + } + + uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit; + uint64_t FieldOffset = IsUnion ? 0 : UnpaddedFieldOffset; if (ZeroLengthBitfield) { - std::pair FieldInfo; - unsigned ZeroLengthBitfieldAlignment; - if (IsMsStruct) { - // If a zero-length bitfield is inserted after a bitfield, - // and the alignment of the zero-length bitfield is - // greater than the member that follows it, `bar', `bar' - // will be aligned as the type of the zero-length bitfield. - if (ZeroLengthBitfield != D) { - FieldInfo = Context.getTypeInfo(ZeroLengthBitfield->getType()); - ZeroLengthBitfieldAlignment = FieldInfo.second; - // Ignore alignment of subsequent zero-length bitfields. - if ((ZeroLengthBitfieldAlignment > FieldAlign) || (FieldSize == 0)) - FieldAlign = ZeroLengthBitfieldAlignment; - if (FieldSize) - ZeroLengthBitfield = 0; - } - } else { - // The alignment of a zero-length bitfield affects the alignment - // of the next member. The alignment is the max of the zero - // length bitfield's alignment and a target specific fixed value. - unsigned ZeroLengthBitfieldBoundary = - Context.getTargetInfo().getZeroLengthBitfieldBoundary(); - if (ZeroLengthBitfieldBoundary > FieldAlign) - FieldAlign = ZeroLengthBitfieldBoundary; - } + // The alignment of a zero-length bitfield affects the alignment + // of the next member. The alignment is the max of the zero + // length bitfield's alignment and a target specific fixed value. + unsigned ZeroLengthBitfieldBoundary = + Context.getTargetInfo().getZeroLengthBitfieldBoundary(); + if (ZeroLengthBitfieldBoundary > FieldAlign) + FieldAlign = ZeroLengthBitfieldBoundary; } if (FieldSize > TypeSize) { @@ -1982,6 +1871,13 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits); } + // ms_struct bitfields always have to start at a round alignment. + if (IsMsStruct && !LastBitfieldTypeSize) { + FieldOffset = llvm::RoundUpToAlignment(FieldOffset, FieldAlign); + UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset, + UnpackedFieldAlign); + } + // Check if we need to add padding to give the field the correct alignment. if (FieldSize == 0 || (MaxFieldAlignment.isZero() && @@ -1996,11 +1892,12 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { // Padding members don't affect overall alignment, unless zero length bitfield // alignment is enabled. - if (!D->getIdentifier() && !Context.getTargetInfo().useZeroLengthBitfieldAlignment()) + if (!D->getIdentifier() && + !Context.getTargetInfo().useZeroLengthBitfieldAlignment() && + !IsMsStruct) FieldAlign = UnpackedFieldAlign = 1; - if (!IsMsStruct) - ZeroLengthBitfield = 0; + ZeroLengthBitfield = 0; if (ExternalLayout) FieldOffset = updateExternalFieldOffset(D, FieldOffset); @@ -2017,11 +1914,29 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { // FIXME: I think FieldSize should be TypeSize here. setDataSize(std::max(getDataSizeInBits(), FieldSize)); } else { - uint64_t NewSizeInBits = FieldOffset + FieldSize; - - setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, - Context.getTargetInfo().getCharAlign())); - UnfilledBitsInLastByte = getDataSizeInBits() - NewSizeInBits; + if (IsMsStruct && FieldSize) { + // Under ms_struct, a bitfield always takes up space equal to the size + // of the type. We can't just change the alignment computation on the + // other codepath because of the way this interacts with #pragma pack: + // in a packed struct, we need to allocate misaligned space in the + // struct to hold the bitfield. + if (!UnfilledBitsInLastUnit) { + setDataSize(FieldOffset + TypeSize); + UnfilledBitsInLastUnit = TypeSize - FieldSize; + } else if (UnfilledBitsInLastUnit < FieldSize) { + setDataSize(getDataSizeInBits() + TypeSize); + UnfilledBitsInLastUnit = TypeSize - FieldSize; + } else { + UnfilledBitsInLastUnit -= FieldSize; + } + LastBitfieldTypeSize = TypeSize; + } else { + uint64_t NewSizeInBits = FieldOffset + FieldSize; + uint64_t BitfieldAlignment = Context.getTargetInfo().getCharAlign(); + setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, BitfieldAlignment)); + UnfilledBitsInLastUnit = getDataSizeInBits() - NewSizeInBits; + LastBitfieldTypeSize = 0; + } } // Update the size. @@ -2038,10 +1953,11 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D) { return; } - uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastByte; + uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit; // Reset the unfilled bits. - UnfilledBitsInLastByte = 0; + UnfilledBitsInLastUnit = 0; + LastBitfieldTypeSize = 0; bool FieldPacked = Packed || D->hasAttr(); CharUnits FieldOffset = @@ -2189,7 +2105,7 @@ void RecordLayoutBuilder::FinishLayout(const NamedDecl *D) { // Finally, round the size of the record up to the alignment of the // record itself. - uint64_t UnpaddedSize = getSizeInBits() - UnfilledBitsInLastByte; + uint64_t UnpaddedSize = getSizeInBits() - UnfilledBitsInLastUnit; uint64_t UnpackedSizeInBits = llvm::RoundUpToAlignment(getSizeInBits(), Context.toBits(UnpackedAlignment)); diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 9d2b23ed185c..d191073958cb 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -915,10 +915,6 @@ CollectRecordFields(const RecordDecl *record, llvm::DIFile tunit, // Field number for non-static fields. unsigned fieldNo = 0; - // Bookkeeping for an ms struct, which ignores certain fields. - bool IsMsStruct = record->isMsStruct(CGM.getContext()); - const FieldDecl *LastFD = 0; - // Static and non-static members should appear in the same order as // the corresponding declarations in the source program. for (RecordDecl::decl_iterator I = record->decls_begin(), @@ -926,13 +922,6 @@ CollectRecordFields(const RecordDecl *record, llvm::DIFile tunit, if (const VarDecl *V = dyn_cast(*I)) CollectRecordStaticField(V, elements, RecordTy); else if (FieldDecl *field = dyn_cast(*I)) { - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are - // completely ignored; we don't even count them. - if (CGM.getContext().ZeroBitfieldFollowsNonBitfield((field), LastFD)) - continue; - LastFD = field; - } CollectRecordNormalField(field, layout.getFieldOffset(fieldNo), tunit, elements, RecordTy); diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp index 3e13e9728019..c06cf2f1b22e 100644 --- a/clang/lib/CodeGen/CGExprConstant.cpp +++ b/clang/lib/CodeGen/CGExprConstant.cpp @@ -373,30 +373,16 @@ bool ConstStructBuilder::Build(InitListExpr *ILE) { unsigned FieldNo = 0; unsigned ElementNo = 0; - const FieldDecl *LastFD = 0; - bool IsMsStruct = RD->isMsStruct(CGM.getContext()); for (RecordDecl::field_iterator Field = RD->field_begin(), FieldEnd = RD->field_end(); Field != FieldEnd; ++Field, ++FieldNo) { - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are - // ignored: - if (CGM.getContext().ZeroBitfieldFollowsNonBitfield(*Field, LastFD)) { - --FieldNo; - continue; - } - LastFD = *Field; - } - // If this is a union, skip all the fields that aren't being initialized. if (RD->isUnion() && ILE->getInitializedFieldInUnion() != *Field) continue; // Don't emit anonymous bitfields, they just affect layout. - if (Field->isUnnamedBitfield()) { - LastFD = *Field; + if (Field->isUnnamedBitfield()) continue; - } // Get the initializer. A struct can include fields without initializers, // we just use explicit null values for them. @@ -472,31 +458,17 @@ void ConstStructBuilder::Build(const APValue &Val, const RecordDecl *RD, } unsigned FieldNo = 0; - const FieldDecl *LastFD = 0; - bool IsMsStruct = RD->isMsStruct(CGM.getContext()); uint64_t OffsetBits = CGM.getContext().toBits(Offset); for (RecordDecl::field_iterator Field = RD->field_begin(), FieldEnd = RD->field_end(); Field != FieldEnd; ++Field, ++FieldNo) { - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are - // ignored: - if (CGM.getContext().ZeroBitfieldFollowsNonBitfield(*Field, LastFD)) { - --FieldNo; - continue; - } - LastFD = *Field; - } - // If this is a union, skip all the fields that aren't being initialized. if (RD->isUnion() && Val.getUnionField() != *Field) continue; // Don't emit anonymous bitfields, they just affect layout. - if (Field->isUnnamedBitfield()) { - LastFD = *Field; + if (Field->isUnnamedBitfield()) continue; - } // Emit the value of the initializer. const APValue &FieldValue = diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index 30ab528ffbe4..8d692c5f69ed 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -78,9 +78,6 @@ public: /// Packed - Whether the resulting LLVM struct will be packed or not. bool Packed; - - /// IsMsStruct - Whether ms_struct is in effect or not - bool IsMsStruct; private: CodeGenTypes &Types; @@ -195,8 +192,7 @@ public: CGRecordLayoutBuilder(CodeGenTypes &Types) : BaseSubobjectType(0), IsZeroInitializable(true), IsZeroInitializableAsBase(true), - Packed(false), IsMsStruct(false), - Types(Types) { } + Packed(false), Types(Types) { } /// Layout - Will layout a RecordDecl. void Layout(const RecordDecl *D); @@ -207,8 +203,6 @@ public: void CGRecordLayoutBuilder::Layout(const RecordDecl *D) { Alignment = Types.getContext().getASTRecordLayout(D).getAlignment(); Packed = D->hasAttr(); - - IsMsStruct = D->isMsStruct(Types.getContext()); if (D->isUnion()) { LayoutUnion(D); @@ -764,20 +758,10 @@ bool CGRecordLayoutBuilder::LayoutFields(const RecordDecl *D) { return false; unsigned FieldNo = 0; - const FieldDecl *LastFD = 0; for (RecordDecl::field_iterator FI = D->field_begin(), FE = D->field_end(); FI != FE; ++FI, ++FieldNo) { FieldDecl *FD = *FI; - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are - // ignored: - if (Types.getContext().ZeroBitfieldFollowsNonBitfield(FD, LastFD)) { - --FieldNo; - continue; - } - LastFD = FD; - } // If this field is a bitfield, layout all of the consecutive // non-zero-length bitfields and the last zero-length bitfield; these will @@ -1028,8 +1012,6 @@ CGRecordLayout *CodeGenTypes::ComputeRecordLayout(const RecordDecl *D, const ASTRecordLayout &AST_RL = getContext().getASTRecordLayout(D); RecordDecl::field_iterator it = D->field_begin(); - const FieldDecl *LastFD = 0; - bool IsMsStruct = D->isMsStruct(getContext()); for (unsigned i = 0, e = AST_RL.getFieldCount(); i != e; ++i, ++it) { const FieldDecl *FD = *it; @@ -1039,25 +1021,12 @@ CGRecordLayout *CodeGenTypes::ComputeRecordLayout(const RecordDecl *D, unsigned FieldNo = RL->getLLVMFieldNo(FD); assert(AST_RL.getFieldOffset(i) == SL->getElementOffsetInBits(FieldNo) && "Invalid field offset!"); - LastFD = FD; continue; } - - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are - // ignored: - if (getContext().ZeroBitfieldFollowsNonBitfield(FD, LastFD)) { - --i; - continue; - } - LastFD = FD; - } // Ignore unnamed bit-fields. - if (!FD->getDeclName()) { - LastFD = FD; + if (!FD->getDeclName()) continue; - } // Don't inspect zero-length bitfields. if (FD->getBitWidthValue(getContext()) == 0) diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp index 3f0eaee53635..f299481ffe3a 100644 --- a/clang/lib/CodeGen/CodeGenTBAA.cpp +++ b/clang/lib/CodeGen/CodeGenTBAA.cpp @@ -203,18 +203,8 @@ CodeGenTBAA::CollectFields(uint64_t BaseOffset, const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); unsigned idx = 0; - const FieldDecl *LastFD = 0; - bool IsMsStruct = RD->isMsStruct(Context); for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end(); i != e; ++i, ++idx) { - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are ignored. - if (Context.ZeroBitfieldFollowsNonBitfield(*i, LastFD)) { - --idx; - continue; - } - LastFD = *i; - } uint64_t Offset = BaseOffset + Layout.getFieldOffset(idx) / Context.getCharWidth(); QualType FieldQTy = i->getType(); @@ -278,19 +268,8 @@ CodeGenTBAA::getTBAAStructTypeInfo(QualType QTy) { const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); SmallVector , 4> Fields; unsigned idx = 0; - const FieldDecl *LastFD = 0; - bool IsMsStruct = RD->isMsStruct(Context); for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end(); i != e; ++i, ++idx) { - if (IsMsStruct) { - // Zero-length bitfields following non-bitfield members are ignored. - if (Context.ZeroBitfieldFollowsNonBitfield(*i, LastFD)) { - --idx; - continue; - } - LastFD = *i; - } - QualType FieldQTy = i->getType(); llvm::MDNode *FieldNode; if (isTBAAPathStruct(FieldQTy)) diff --git a/clang/test/CodeGen/tbaa-struct.cpp b/clang/test/CodeGen/tbaa-struct.cpp index 6d593a3c1b24..9320a8375cb2 100644 --- a/clang/test/CodeGen/tbaa-struct.cpp +++ b/clang/test/CodeGen/tbaa-struct.cpp @@ -70,5 +70,5 @@ void copy5(struct six *a, struct six *b) { // CHECK: [[TS2]] = metadata !{i64 0, i64 1, metadata !{{.*}}, i64 4, i64 2, metadata !{{.*}}, i64 8, i64 4, metadata !{{.*}}, i64 12, i64 1, metadata !{{.*}}, i64 16, i64 4, metadata {{.*}}, i64 20, i64 4, metadata {{.*}}} // (offset, size) = (0,8) char; (0,2) char; (4,8) char // CHECK: [[TS3]] = metadata !{i64 0, i64 8, metadata !{{.*}}, i64 0, i64 2, metadata !{{.*}}, i64 4, i64 8, metadata !{{.*}}} -// CHECK: [[TS4]] = metadata !{i64 0, i64 1, metadata [[CHAR]], i64 1, i64 1, metadata [[CHAR]], i64 2, i64 1, metadata [[CHAR]]} +// CHECK: [[TS4]] = metadata !{i64 0, i64 1, metadata [[CHAR]], i64 1, i64 4, metadata [[INT]], i64 1, i64 1, metadata [[CHAR]], i64 2, i64 1, metadata [[CHAR]]} // CHECK: [[TS5]] = metadata !{i64 0, i64 1, metadata [[CHAR]], i64 4, i64 4, metadata [[INT]], i64 4, i64 1, metadata [[CHAR]], i64 5, i64 1, metadata [[CHAR]]} diff --git a/clang/test/CodeGen/tbaa.cpp b/clang/test/CodeGen/tbaa.cpp index afb8893d3e6a..77133e473a4e 100644 --- a/clang/test/CodeGen/tbaa.cpp +++ b/clang/test/CodeGen/tbaa.cpp @@ -250,6 +250,6 @@ char g14(struct six *a, struct six *b) { // PATH: [[TAG_D_b_a_f32]] = metadata !{metadata [[TYPE_D:!.*]], metadata [[TYPE_INT]], i64 12} // PATH: [[TYPE_D]] = metadata !{metadata !"_ZTS7StructD", metadata [[TYPE_SHORT]], i64 0, metadata [[TYPE_B]], i64 4, metadata [[TYPE_INT]], i64 28, metadata [[TYPE_CHAR]], i64 32} // PATH: [[TAG_five_b]] = metadata !{metadata [[TYPE_five:!.*]], metadata [[TYPE_CHAR]], i64 1} -// PATH: [[TYPE_five]] = metadata !{metadata !"_ZTS4five", metadata [[TYPE_CHAR]], i64 0, metadata [[TYPE_CHAR]], i64 1, metadata [[TYPE_CHAR]], i64 2} +// PATH: [[TYPE_five]] = metadata !{metadata !"_ZTS4five", metadata [[TYPE_CHAR]], i64 0, metadata [[TYPE_INT]], i64 1, metadata [[TYPE_CHAR]], i64 1, metadata [[TYPE_CHAR]], i64 2} // PATH: [[TAG_six_b]] = metadata !{metadata [[TYPE_six:!.*]], metadata [[TYPE_CHAR]], i64 4} // PATH: [[TYPE_six]] = metadata !{metadata !"_ZTS3six", metadata [[TYPE_CHAR]], i64 0, metadata [[TYPE_INT]], i64 4, metadata [[TYPE_CHAR]], i64 4, metadata [[TYPE_CHAR]], i64 5} diff --git a/clang/test/SemaCXX/ms_struct.cpp b/clang/test/SemaCXX/ms_struct.cpp new file mode 100644 index 000000000000..fd1ed904fa86 --- /dev/null +++ b/clang/test/SemaCXX/ms_struct.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-apple-darwin9 -std=c++11 %s +// expected-no-diagnostics + +#pragma ms_struct on + +struct A { + unsigned long a:4; + unsigned char b; + A(); +}; + +struct B : public A { + unsigned long c:16; + int d; + B(); +}; + +static_assert(__builtin_offsetof(B, d) == 12, + "We can't allocate the bitfield into the padding under ms_struct"); \ No newline at end of file