Auto merge of #1130 - fitzgen:issue-1076, r=pepyakin

Unnamed bit-fields and alignment

r? @pepyakin or @emilio

Author: bors-servo

src/ir/comp.rs

@@ -595,12 +595,8 @@ fn bitfields_to_allocation_units<E, I>(
 
                 // Now we're working on a fresh bitfield allocation unit, so reset
                 // the current unit size and alignment.
-                #[allow(unused_assignments)]
-                {
-                    unit_size_in_bits = 0;
-                    offset = 0;
-                    unit_align = 0;
-                }
+                offset = 0;
+                unit_align = 0;
             }
         } else {
             if offset != 0 &&
@@ -612,20 +608,28 @@ fn bitfields_to_allocation_units<E, I>(
             }
         }
 
+        // According to the x86[-64] ABI spec: "Unnamed bit-fields’ types do not
+        // affect the alignment of a structure or union". This makes sense: such
+        // bit-fields are only used for padding, and we can't perform an
+        // un-aligned read of something we can't read because we can't even name
+        // it.
+        if bitfield.name().is_some() {
+            max_align = cmp::max(max_align, bitfield_align);
+
+            // NB: The `bitfield_width` here is completely, absolutely
+            // intentional.  Alignment of the allocation unit is based on the
+            // maximum bitfield width, not (directly) on the bitfields' types'
+            // alignment.
+            unit_align = cmp::max(unit_align, bitfield_width);
+        }
+
         // Always keep all bitfields around. While unnamed bitifields are used
         // for padding (and usually not needed hereafter), large unnamed
         // bitfields over their types size cause weird allocation size behavior from clang.
         // Therefore, all bitfields needed to be kept around in order to check for this
         // and make the struct opaque in this case
         bitfields_in_unit.push(Bitfield::new(offset, bitfield));
 
-        max_align = cmp::max(max_align, bitfield_align);
-
-        // NB: The `bitfield_width` here is completely, absolutely intentional.
-        // Alignment of the allocation unit is based on the maximum bitfield
-        // width, not (directly) on the bitfields' types' alignment.
-        unit_align = cmp::max(unit_align, bitfield_width);
-
         unit_size_in_bits = offset + bitfield_width;
 
         // Compute what the physical unit's final size would be given what we

tests/expectations/tests/issue-1034.rs

@@ -4,10 +4,11 @@
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
 
 
-#[repr(C, packed)]
+#[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct S2 {
-    pub _bitfield_1: u16,
+    pub _bitfield_1: u8,
+    pub __bindgen_padding_0: u8,
 }
 #[test]
 fn bindgen_test_layout_S2() {
@@ -24,7 +25,7 @@ fn bindgen_test_layout_S2() {
 }
 impl S2 {
     #[inline]
-    pub fn new_bitfield_1() -> u16 {
+    pub fn new_bitfield_1() -> u8 {
         0
     }
 }

tests/expectations/tests/issue-1076-unnamed-bitfield-alignment.rs

@@ -0,0 +1,31 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+
+
+#[repr(C)]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct S1 {
+    pub _bitfield_1: [u8; 2usize],
+    pub __bindgen_padding_0: u8,
+}
+#[test]
+fn bindgen_test_layout_S1() {
+    assert_eq!(
+        ::std::mem::size_of::<S1>(),
+        3usize,
+        concat!("Size of: ", stringify!(S1))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<S1>(),
+        1usize,
+        concat!("Alignment of ", stringify!(S1))
+    );
+}
+impl S1 {
+    #[inline]
+    pub fn new_bitfield_1() -> u16 {
+        0
+    }
+}

tests/headers/issue-1076-unnamed-bitfield-alignment.h

@@ -0,0 +1,4 @@
+struct S1 {
+    signed : 15;
+    unsigned : 6
+};