[PATCH] Fixing a compiler assertion with zero-width bit-fields in packed structs

[Yunzhong Gao]

ygao added you to the CC list for the revision "Fixing a compiler assertion with zero-width bit-fields in packed structs".

Hi,
The following patch fixed a compiler assertion when compiling the following test case:
```
/* test.c
 * $ clang -S test.c
 */
struct __attribute__((packed)) f { 
  int v1:9; 
  int :0; 
  int v2:17;
};
struct f f1 = {};
/* end of test.c */
```

According to the GNU docs, the zero-sized bitfields should not be affected by the packed attribute
Reference: http://gcc.gnu.org/onlinedocs/gcc/Type-Attributes.html
```
packed
  This attribute, attached to struct or union type definition, specifies that each member (other than zero-width bit-fields)
  of the structure or union is placed to minimize the memory required. When attached to an enum definition, it indicates
  that the smallest integral type should be used. 
```

Also, the C++11 memory model requires that the members after a zero-width bit-fields be accessible as a separate
object, which implies that they be aligned at least to byte boundaries.

I am under the impression that the packed attribute is not related to the target ABI spec and hence there
should be no need to check the return value of "Context.getTargetInfo().useZeroLengthBitfieldAlignment()"
in this part of the code. Please let me know if that is not the case.

Many thanks!
- Gao.

http://llvm-reviews.chandlerc.com/D2693

Files:
  lib/AST/RecordLayoutBuilder.cpp
  test/Sema/bitfield-layout.c

Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -1523,8 +1523,8 @@
   // Remember the alignment we would have used if the field were not packed.
   unsigned UnpackedFieldAlign = FieldAlign;
 
-  // Ignore the field alignment if the field is packed.
-  if (FieldPacked)
+  // Ignore the field alignment if the field is packed unless it has zero-size.
+  if (FieldPacked && FieldSize != 0)
     FieldAlign = 1;
 
   // But, if there's an 'aligned' attribute on the field, honor that.
Index: test/Sema/bitfield-layout.c
===================================================================
--- test/Sema/bitfield-layout.c
+++ test/Sema/bitfield-layout.c
@@ -9,6 +9,21 @@
 CHECK_SIZE(struct, a, 5)
 CHECK_ALIGN(struct, a, 1)
 
+// Zero-width bit-fields with packed
+struct __attribute__((packed)) a2 { short x : 9; char : 0; int y : 17; };
+CHECK_SIZE(struct, a2, 5)
+CHECK_ALIGN(struct, a2, 1)
+
+// Zero-width bit-fields at the end of packed struct
+struct __attribute__((packed)) a3 { short x : 9; int : 0; };
+CHECK_SIZE(struct, a3, 4)
+CHECK_ALIGN(struct, a3, 1)
+
+// For comparison, non-zero-width bit-fields at the end of packed struct
+struct __attribute__((packed)) a4 { short x : 9; int : 1; };
+CHECK_SIZE(struct, a4, 2)
+CHECK_ALIGN(struct, a4, 1)
+
 union b {char x; int : 0; char y;};
 CHECK_SIZE(union, b, 1)
 CHECK_ALIGN(union, b, 1)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2693.2.patch
Type: text/x-patch
Size: 1479 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140204/9be541e9/attachment.bin>