Hi Anders, Does this have to use an extra variable UnfilledBitsInLastByte? Can't the code just round up DataSize once it is done laying out a single class?
- Daniel On Sun, Nov 22, 2009 at 11:13 AM, Anders Carlsson <[email protected]> wrote: > Author: andersca > Date: Sun Nov 22 13:13:51 2009 > New Revision: 89611 > > URL: http://llvm.org/viewvc/llvm-project?rev=89611&view=rev > Log: > When laying out bitfields, make sure that the data size is always aligned to > a byte. This fixes PR5580. > > Modified: > cfe/trunk/lib/AST/RecordLayoutBuilder.cpp > cfe/trunk/lib/AST/RecordLayoutBuilder.h > cfe/trunk/test/SemaCXX/class-layout.cpp > > Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=89611&r1=89610&r2=89611&view=diff > > ============================================================================== > --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original) > +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Sun Nov 22 13:13:51 2009 > @@ -22,9 +22,9 @@ > using namespace clang; > > ASTRecordLayoutBuilder::ASTRecordLayoutBuilder(ASTContext &Ctx) > - : Ctx(Ctx), Size(0), Alignment(8), Packed(false), MaxFieldAlignment(0), > - DataSize(0), IsUnion(false), NonVirtualSize(0), NonVirtualAlignment(8), > - PrimaryBase(0), PrimaryBaseWasVirtual(false) {} > + : Ctx(Ctx), Size(0), Alignment(8), Packed(false), > UnfilledBitsInLastByte(0), > + MaxFieldAlignment(0), DataSize(0), IsUnion(false), NonVirtualSize(0), > + NonVirtualAlignment(8), PrimaryBase(0), PrimaryBaseWasVirtual(false) {} > > /// LayoutVtable - Lay out the vtable and set PrimaryBase. > void ASTRecordLayoutBuilder::LayoutVtable(const CXXRecordDecl *RD) { > @@ -521,7 +521,7 @@ > > void ASTRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { > bool FieldPacked = Packed || D->hasAttr<PackedAttr>(); > - uint64_t FieldOffset = IsUnion ? 0 : DataSize; > + uint64_t FieldOffset = IsUnion ? 0 : (DataSize - UnfilledBitsInLastByte); > uint64_t FieldSize = D->getBitWidth()->EvaluateAsInt(Ctx).getZExtValue(); > > std::pair<uint64_t, unsigned> FieldInfo = Ctx.getTypeInfo(D->getType()); > @@ -549,14 +549,19 @@ > // Place this field at the current location. > FieldOffsets.push_back(FieldOffset); > > - // Reserve space for this field. > - if (IsUnion) > - Size = std::max(Size, FieldSize); > - else > - Size = FieldOffset + FieldSize; > + // Update DataSize to include the last byte containing (part of) the > bitfield. > + if (IsUnion) { > + // FIXME: I think FieldSize should be TypeSize here. > + DataSize = std::max(DataSize, FieldSize); > + } else { > + uint64_t NewSizeInBits = FieldOffset + FieldSize; > + > + DataSize = llvm::RoundUpToAlignment(NewSizeInBits, 8); > + UnfilledBitsInLastByte = DataSize - NewSizeInBits; > + } > > - // Update the data size. > - DataSize = Size; > + // Update the size. > + Size = std::max(Size, DataSize); > > // Remember max struct/class alignment. > UpdateAlignment(FieldAlign); > @@ -568,6 +573,9 @@ > return; > } > > + // Reset the unfilled bits. > + UnfilledBitsInLastByte = 0; > + > bool FieldPacked = Packed || D->hasAttr<PackedAttr>(); > uint64_t FieldOffset = IsUnion ? 0 : DataSize; > uint64_t FieldSize; > > Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.h?rev=89611&r1=89610&r2=89611&view=diff > > ============================================================================== > --- cfe/trunk/lib/AST/RecordLayoutBuilder.h (original) > +++ cfe/trunk/lib/AST/RecordLayoutBuilder.h Sun Nov 22 13:13:51 2009 > @@ -27,12 +27,21 @@ > class ASTRecordLayoutBuilder { > ASTContext &Ctx; > > + /// Size - The current size of the record layout. > uint64_t Size; > + > + /// Alignment - The current alignment of the record layout. > unsigned Alignment; > + > llvm::SmallVector<uint64_t, 16> FieldOffsets; > > /// Packed - Whether the record is packed or not. > bool Packed; > + > + /// 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; > > /// MaxFieldAlignment - The maximum allowed field alignment. This is set by > /// #pragma pack. > > Modified: cfe/trunk/test/SemaCXX/class-layout.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/class-layout.cpp?rev=89611&r1=89610&r2=89611&view=diff > > ============================================================================== > --- cfe/trunk/test/SemaCXX/class-layout.cpp (original) > +++ cfe/trunk/test/SemaCXX/class-layout.cpp Sun Nov 22 13:13:51 2009 > @@ -47,3 +47,20 @@ > struct H : G { }; > > SA(6, sizeof(H) == 1); > + > +// PR5580 > +namespace PR5580 { > + > +class A { bool iv0 : 1; }; > +SA(7, sizeof(A) == 1); > + > +class B : A { bool iv0 : 1; }; > +SA(8, sizeof(B) == 2); > + > +struct C { bool iv0 : 1; }; > +SA(9, sizeof(C) == 1); > + > +struct D : C { bool iv0 : 1; }; > +SA(10, sizeof(D) == 2); > + > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
