From a5dd722bdf2f74a1a249fe6661d96a7236aee0f0 Mon Sep 17 00:00:00 2001 From: Anders Carlsson Date: Sat, 8 Aug 2009 19:38:24 +0000 Subject: [PATCH] Take #pragma pack into account when laying out structs. Fixes rdar://problem/7095436. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@78490 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/RecordLayoutBuilder.cpp | 56 +++++++++++++-------------- lib/AST/RecordLayoutBuilder.h | 3 +- lib/CodeGen/CGRecordLayoutBuilder.cpp | 20 +++++++++- lib/CodeGen/CGRecordLayoutBuilder.h | 5 ++- 4 files changed, 51 insertions(+), 33 deletions(-) diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index 8a18197a56..68350daa6c 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -22,8 +22,8 @@ using namespace clang; ASTRecordLayoutBuilder::ASTRecordLayoutBuilder(ASTContext &Ctx) - : Ctx(Ctx), Size(0), Alignment(8), StructPacking(0), NextOffset(0), - IsUnion(false), NonVirtualSize(0), NonVirtualAlignment(8) {} + : Ctx(Ctx), Size(0), Alignment(8), Packed(false), MaxFieldAlignment(0), + NextOffset(0), IsUnion(false), NonVirtualSize(0), NonVirtualAlignment(8) {} /// LayoutVtable - Lay out the vtable and set PrimaryBase. void ASTRecordLayoutBuilder::LayoutVtable(const CXXRecordDecl *RD) { @@ -198,10 +198,11 @@ void ASTRecordLayoutBuilder::LayoutBaseNonVirtually(const CXXRecordDecl *RD) { void ASTRecordLayoutBuilder::Layout(const RecordDecl *D) { IsUnion = D->isUnion(); - if (D->hasAttr()) - StructPacking = 1; + Packed = D->hasAttr(); + + // The #pragma pack attribute specifies the maximum field alignment. if (const PragmaPackAttr *PPA = D->getAttr()) - StructPacking = PPA->getAlignment(); + MaxFieldAlignment = PPA->getAlignment(); if (const AlignedAttr *AA = D->getAttr()) UpdateAlignment(AA->getAlignment()); @@ -242,8 +243,11 @@ void ASTRecordLayoutBuilder::Layout(const ObjCInterfaceDecl *D, NextOffset = Size; } - if (D->hasAttr()) - StructPacking = 1; + Packed = D->hasAttr(); + + // The #pragma pack attribute specifies the maximum field alignment. + if (const PragmaPackAttr *PPA = D->getAttr()) + MaxFieldAlignment = PPA->getAlignment(); if (const AlignedAttr *AA = D->getAttr()) UpdateAlignment(AA->getAlignment()); @@ -268,15 +272,12 @@ void ASTRecordLayoutBuilder::LayoutFields(const RecordDecl *D) { } void ASTRecordLayoutBuilder::LayoutField(const FieldDecl *D) { - unsigned FieldPacking = StructPacking; + bool FieldPacked = Packed; uint64_t FieldOffset = IsUnion ? 0 : Size; uint64_t FieldSize; unsigned FieldAlign; - // FIXME: Should this override struct packing? Probably we want to - // take the minimum? - if (D->hasAttr()) - FieldPacking = 1; + FieldPacked |= D->hasAttr(); if (const Expr *BitWidthExpr = D->getBitWidth()) { // TODO: Need to check this algorithm on other targets! @@ -285,18 +286,17 @@ void ASTRecordLayoutBuilder::LayoutField(const FieldDecl *D) { std::pair FieldInfo = Ctx.getTypeInfo(D->getType()); uint64_t TypeSize = FieldInfo.first; - - // Determine the alignment of this bitfield. The packing - // attributes define a maximum and the alignment attribute defines - // a minimum. - // FIXME: What is the right behavior when the specified alignment - // is smaller than the specified packing? + FieldAlign = FieldInfo.second; - if (FieldPacking) - FieldAlign = std::min(FieldAlign, FieldPacking); + + if (FieldPacked) + FieldAlign = 1; if (const AlignedAttr *AA = D->getAttr()) FieldAlign = std::max(FieldAlign, AA->getAlignment()); - + // The maximum field alignment overrides the aligned attribute. + if (MaxFieldAlignment) + FieldAlign = std::min(FieldAlign, MaxFieldAlignment); + // Check if we need to add padding to give the field the correct // alignment. if (FieldSize == 0 || (FieldOffset & (FieldAlign-1)) + FieldSize > TypeSize) @@ -324,17 +324,13 @@ void ASTRecordLayoutBuilder::LayoutField(const FieldDecl *D) { FieldAlign = FieldInfo.second; } - // Determine the alignment of this bitfield. The packing - // attributes define a maximum and the alignment attribute defines - // a minimum. Additionally, the packing alignment must be at least - // a byte for non-bitfields. - // - // FIXME: What is the right behavior when the specified alignment - // is smaller than the specified packing? - if (FieldPacking) - FieldAlign = std::min(FieldAlign, std::max(8U, FieldPacking)); + if (FieldPacked) + FieldAlign = 8; if (const AlignedAttr *AA = D->getAttr()) FieldAlign = std::max(FieldAlign, AA->getAlignment()); + // The maximum field alignment overrides the aligned attribute. + if (MaxFieldAlignment) + FieldAlign = std::min(FieldAlign, MaxFieldAlignment); // Round up the current record size to the field's alignment boundary. FieldOffset = (FieldOffset + (FieldAlign-1)) & ~(FieldAlign-1); diff --git a/lib/AST/RecordLayoutBuilder.h b/lib/AST/RecordLayoutBuilder.h index 75c6b51921..488f6662c0 100644 --- a/lib/AST/RecordLayoutBuilder.h +++ b/lib/AST/RecordLayoutBuilder.h @@ -30,7 +30,8 @@ class ASTRecordLayoutBuilder { unsigned Alignment; llvm::SmallVector FieldOffsets; - unsigned StructPacking; + bool Packed; + unsigned MaxFieldAlignment; uint64_t NextOffset; bool IsUnion; diff --git a/lib/CodeGen/CGRecordLayoutBuilder.cpp b/lib/CodeGen/CGRecordLayoutBuilder.cpp index d3699bb992..4576b808f0 100644 --- a/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -27,13 +27,15 @@ using namespace clang; using namespace CodeGen; void CGRecordLayoutBuilder::Layout(const RecordDecl *D) { + Alignment = Types.getContext().getASTRecordLayout(D).getAlignment() / 8; + if (D->isUnion()) { LayoutUnion(D); return; } Packed = D->hasAttr(); - + if (LayoutFields(D)) return; @@ -115,6 +117,21 @@ bool CGRecordLayoutBuilder::LayoutField(const FieldDecl *D, const llvm::Type *Ty = Types.ConvertTypeForMemRecursive(D->getType()); unsigned TypeAlignment = getTypeAlignment(Ty); + // If the type alignment is larger then the struct alignment, we must use + // a packed struct. + if (TypeAlignment > Alignment) { + assert(!Packed && "Alignment is wrong even with packed struct!"); + return false; + } + + if (const RecordType *RT = D->getType()->getAs()) { + const RecordDecl *RD = cast(RT->getDecl()); + if (const PragmaPackAttr *PPA = RD->getAttr()) { + if (PPA->getAlignment() != TypeAlignment * 8 && !Packed) + return false; + } + } + // Round up the field offset to the alignment of the field type. uint64_t AlignedNextFieldOffsetInBytes = llvm::RoundUpToAlignment(NextFieldOffsetInBytes, TypeAlignment); @@ -193,6 +210,7 @@ void CGRecordLayoutBuilder::LayoutUnion(const RecordDecl *D) { bool CGRecordLayoutBuilder::LayoutFields(const RecordDecl *D) { assert(!D->isUnion() && "Can't call LayoutFields on a union!"); + assert(Alignment && "Did not set alignment!"); const ASTRecordLayout &Layout = Types.getContext().getASTRecordLayout(D); diff --git a/lib/CodeGen/CGRecordLayoutBuilder.h b/lib/CodeGen/CGRecordLayoutBuilder.h index bca0b5c142..ff551a4f13 100644 --- a/lib/CodeGen/CGRecordLayoutBuilder.h +++ b/lib/CodeGen/CGRecordLayoutBuilder.h @@ -36,6 +36,9 @@ class CGRecordLayoutBuilder { /// Packed - Whether the resulting LLVM struct will be packed or not. bool Packed; + /// Alignment - Contains the alignment of the RecordDecl. + unsigned Alignment; + /// AlignmentAsLLVMStruct - Will contain the maximum alignment of all the /// LLVM types. unsigned AlignmentAsLLVMStruct; @@ -69,7 +72,7 @@ class CGRecordLayoutBuilder { llvm::SmallVector LLVMBitFields; CGRecordLayoutBuilder(CodeGenTypes &Types) - : Types(Types), Packed(false), AlignmentAsLLVMStruct(1) + : Types(Types), Packed(false), Alignment(0), AlignmentAsLLVMStruct(1) , BitsAvailableInLastField(0), NextFieldOffsetInBytes(0) { } /// Layout - Will layout a RecordDecl. -- 2.40.0