pr26544: Bitfield layout with pragma pack and attributes "packed" and "aligned

[vbyakovlcl]

Summary

Fix clang/gcc incompatibility of bitfields layout in the presence of pragma packed and attributes aligned and packed.

[DmitryPolukhin]

John, this patch extends check that I added recently and looks good to me but could you please also take a look.

[rjmccall]

LGTM.

lib/AST/RecordLayoutBuilder.cpp

void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
   unsigned ExplicitFieldAlign = D->getMaxAlignment();
   if (ExplicitFieldAlign) {
     FieldAlign = std::max(FieldAlign, ExplicitFieldAlign);
     UnpackedFieldAlign = std::max(UnpackedFieldAlign, ExplicitFieldAlign);
   }
 
   // But, if there's a #pragma pack in play, that takes precedent over
   // even the 'aligned' attribute, for non-zero-width bitfields.
-  if (!MaxFieldAlignment.isZero() && FieldSize) {
-    unsigned MaxFieldAlignmentInBits = Context.toBits(MaxFieldAlignment);
-    FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  if (!MaxFieldAlignment.isZero() && FieldSize) {
     UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+    if (FieldPacked)
+      FieldAlign = UnpackedFieldAlign;
+    else
+      FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
   }
 
   // But, ms_struct just ignores all of that in unions, even explicit
   // alignment attributes.
   if (IsMsStruct && IsUnion) {
     FieldAlign = UnpackedFieldAlign = 1;
   }
 
if (IsMsStruct) {
     bool AllowPadding = MaxFieldAlignment.isZero();
 
     // Compute the real offset.
     if (FieldSize == 0 || 
         (AllowPadding &&
          (FieldOffset & (FieldAlign-1)) + FieldSize > TypeSize)) {
       FieldOffset = llvm::alignTo(FieldOffset, FieldAlign);
     } else if (ExplicitFieldAlign &&
+               (MaxFieldAlignmentInBits == 0 ||
+                ExplicitFieldAlign <= MaxFieldAlignmentInBits) &&
                Context.getTargetInfo().useExplicitBitFieldAlignment()) {
       // TODO: figure it out what needs to be done on targets that don't honor
       // bit-field type alignment like ARM APCS ABI.
       FieldOffset = llvm::alignTo(FieldOffset, ExplicitFieldAlign);
     }
 
     // Repeat the computation for diagnostic purposes.
     if (FieldSize == 0 ||
         (AllowPadding &&
          (UnpackedFieldOffset & (UnpackedFieldAlign-1)) + FieldSize > TypeSize))
       UnpackedFieldOffset =
           llvm::alignTo(UnpackedFieldOffset, UnpackedFieldAlign);
     else if (ExplicitFieldAlign &&
+             (MaxFieldAlignmentInBits == 0 ||
+              ExplicitFieldAlign <= MaxFieldAlignmentInBits) &&
              Context.getTargetInfo().useExplicitBitFieldAlignment())
       UnpackedFieldOffset =
           llvm::alignTo(UnpackedFieldOffset, ExplicitFieldAlign);
   }
 
   // If we're using external layout, give the external layout a chance
   // to override this information.
   if (UseExternalLayout)

test/Sema/bitfield-layout_1.c

+// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=i686-apple-darwin9
+// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=arm-linux-gnueabihf
+// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=aarch64-linux-gnu
+// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=x86_64-pc-linux-gnu
+// expected-no-diagnostics
+
+#define CHECK_SIZE(name, size) \
+  extern int name##_1[sizeof(name) == size ? 1 : -1];
+
+
+struct  __attribute__((packed)) {
+  int a;
+  int b : 4;
+  int c : 32;
+} s0;
+CHECK_SIZE(s0,9)
+
+#pragma pack (1)
+struct {
+  int a;
+  int b : 4;
+  int c : 32;
+} s1;
+CHECK_SIZE(s1,9)
+
+#pragma pack (2)
+struct {
+  int a;
+  int b : 4;
+  int c : 32;
+} s2;
+CHECK_SIZE(s2,10)
+
+#pragma pack (2)
+struct __attribute__((packed)) {
+  int a;
+  int b : 4;
+  int c : 32;
+} s3;
+CHECK_SIZE(s3,10)
+
+#pragma pack (4)
+struct  __attribute__((packed)) {
+  int a;
+  int b : 4;
+  int c : 32;
+} s4;
+CHECK_SIZE(s4,12)
+
+#pragma pack (16)
+struct {
+  int a;
+  int __attribute__((packed)) b : 4;
+  int __attribute__((packed)) c : 32;
+} s41;
+CHECK_SIZE(s41,12)
+
+#pragma pack (16)
+struct {
+  int a;
+  int b : 4;
+  int c : 32;
+} s5;
+CHECK_SIZE(s5,12)
+
+#pragma pack (1)
+struct  __attribute__((aligned(4))) {
+  int a;
+  int b : 4;
+  int c : 32;
+} s6;
+CHECK_SIZE(s6,12)
+
+#pragma pack (2)
+struct {
+  char a;
+  int b : 4;
+  int c : 32;
+  char s;
+} s7;
+CHECK_SIZE(s7,8)
+
+#pragma pack (1)
+struct {
+  char a;
+  int b : 4;
+  int c : 28;
+  char s;
+} s8;
+CHECK_SIZE(s8,6)
+
+#pragma pack (8)
+struct {
+  char a;
+  int b : 4;
+  int c : 28;
+  char s;
+} s9;
+CHECK_SIZE(s9,8)
+
+#pragma pack (8)
+struct {
+  char a;
+  char s;
+} s10;
+CHECK_SIZE(s10,2)
+
+#pragma pack(4)
+struct {
+  char a;
+  int b : 4;
+  int c : 28;
+  char s1;
+  char s2;
+  char s3;
+} s11;
+CHECK_SIZE(s11,8)
+
+#pragma pack(4)
+struct {
+  short s1;
+  int a1 : 17;
+  int a2 : 17;
+  int a3 : 30;
+  short s2;
+} s12;
+CHECK_SIZE(s12,12)
+
+#pragma pack(4)
+struct {
+  char c1;
+  int i1 : 17;
+  int i2 : 17;
+  int i3 : 30;
+  char c2;
+} s13;
+CHECK_SIZE(s13,12)
+
+#pragma pack(2)
+struct {
+  char a;
+  int s;
+} s14;
+CHECK_SIZE(s14,6)
+
+#pragma pack(4)
+struct {
+  char a;
+  short s;
+} s15;
+CHECK_SIZE(s15,4)
+
+#pragma pack(2)
+struct {
+  char a;
+  int b : 4;
+  int c : 28;
+  char s1;
+  char s2;
+  char s3;
+} s16;
+CHECK_SIZE(s16,8)
+
+#pragma pack (16)
+struct {
+  int __attribute__((packed)) a;
+  int __attribute__((packed)) b : 4;
+  int __attribute__((packed)) c : 32;
+} s17;
+CHECK_SIZE(s17,12)
+
+#pragma pack (16)
+struct {
+  int __attribute__((aligned(8))) a;
+  int __attribute__((aligned(8))) b : 4;
+  int __attribute__((aligned(8))) c : 32;
+} s18;
+CHECK_SIZE(s18,24)
+
+#pragma pack (16)
+struct {
+  int __attribute__((aligned(1))) a;
+  int __attribute__((aligned(1))) b : 4;
+  int __attribute__((aligned(1))) c : 32;
+} s19;
+CHECK_SIZE(s19,12)
+
+#pragma pack (1)
+struct  __attribute__((aligned(8))) {
+  int a;
+  int b : 4;
+  int c : 32;
+} s20;
+CHECK_SIZE(s20,16)
+
+#pragma pack (2)
+struct {
+  int __attribute__((aligned(8))) a;
+  int __attribute__((aligned(8))) b : 4;
+  int __attribute__((aligned(8))) c : 32;
+} s21;
+CHECK_SIZE(s21,10)