[cfe-commits] r89611 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp lib/AST/RecordLayoutBuilder.h test/SemaCXX/class-layout.cpp

[Daniel Dunbar]

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 <andersca at mac.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>