Auto merge of #736 - emilio:bitfields-revamp, r=fitzgen

ir: Fix a bunch of bitfield correctness issues.

In particular, the "flush the allocation unit" logic is only valid for
ms_structs (that is, MSVC).

It's slightly annoying to have this different behavior, but it'd work just fine
if we'd turn that on for MSVC.

This patch doesn't do that, yet at least, and adds tests for all the weird
bitfield alignments around.

Fixes #726 (and another set of hidden issues by the old code).

Author: bors-servo

bindgen-integration/cpp/Test.cc

@@ -56,4 +56,24 @@ Third::assert(int first, bool second, ItemKind third)
         kind == third;
 }
 
+bool
+Fourth::assert(MyEnum tag, unsigned long ptr)
+{
+    return this->tag == tag && this->ptr == ptr;
+}
+
+bool
+Date2::assert(unsigned short nWeekDay,
+              unsigned short nMonthDay,
+              unsigned short nMonth,
+              unsigned short nYear,
+              unsigned short byte)
+{
+  return this->nWeekDay == nWeekDay &&
+      this->nMonthDay == nMonthDay &&
+      this->nMonth == nMonth &&
+      this->nYear == nYear &&
+      this->byte == byte;
+}
+
 } // namespace bitfields

bindgen-integration/cpp/Test.h

@@ -49,7 +49,7 @@ struct Second {
 };
 
 enum ItemKind {
-    ITEM_KIND_UNO,
+    ITEM_KIND_UNO = 0,
     ITEM_KIND_DOS,
     ITEM_KIND_TRES,
 };
@@ -63,6 +63,35 @@ struct Third {
     bool assert(int first, bool second, ItemKind third);
 };
 
+enum MyEnum {
+    ONE = 0,
+    TWO,
+    THREE,
+    FOUR,
+};
+
+struct Fourth {
+    MyEnum tag: 2;
+    unsigned long ptr: 48;
+
+    /// Returns true if the bitfields match the arguments, false otherwise.
+    bool assert(MyEnum tag, unsigned long ptr);
+};
+
+struct Date2 {
+    unsigned short nWeekDay  : 3;    // 0..7   (3 bits)
+    unsigned short nMonthDay : 6;    // 0..31  (6 bits)
+    unsigned short nMonth    : 5;    // 0..12  (5 bits)
+    unsigned short nYear     : 8;    // 0..100 (8 bits)
+    unsigned char byte : 8;
+
+    bool assert(unsigned short nWeekDay,
+                unsigned short nMonthDay,
+                unsigned short nMonth,
+                unsigned short nYear,
+                unsigned short byte);
+};
+
 } // namespace bitfields
 
 struct AutoRestoreBool {

bindgen-integration/src/lib.rs

@@ -102,11 +102,46 @@ fn test_bitfields_third() {
     });
 }
 
+#[test]
+fn test_bitfields_fourth() {
+    let mut fourth: bindings::bitfields::Fourth = unsafe {
+        mem::zeroed()
+    };
+    assert!(unsafe {
+        fourth.assert(bindings::bitfields::MyEnum::ONE, 0)
+    });
+
+    fourth.set_tag(bindings::bitfields::MyEnum::THREE);
+    fourth.set_ptr(0xdeadbeef);
+    assert!(unsafe {
+        fourth.assert(bindings::bitfields::MyEnum::THREE, 0xdeadbeef)
+    });
+}
+
+#[test]
+fn test_bitfields_date2() {
+    let mut date: bindings::bitfields::Date2 = unsafe {
+        mem::zeroed()
+    };
+    assert!(unsafe {
+        date.assert(0, 0, 0, 0, 0)
+    });
+
+    date.set_nWeekDay(6); // saturdays are the best
+    date.set_nMonthDay(20);
+    date.set_nMonth(11);
+    date.set_nYear(95);
+    date.set_byte(255);
+    assert!(unsafe {
+        date.assert(6, 20, 11, 95, 255)
+    });
+}
+
 #[test]
 fn test_bitfield_constructors() {
+    use std::mem;
     let mut first = bindings::bitfields::First {
-        _bitfield_1: bindings::bitfields::First::new_bitfield_1(1),
-        _bitfield_2: bindings::bitfields::First::new_bitfield_2(2, 3),
+        _bitfield_1: unsafe { mem::transmute(bindings::bitfields::First::new_bitfield_1(1, 2, 3)) },
         __bindgen_align: [],
     };
     assert!(unsafe {

src/ir/comp.rs

@@ -490,49 +490,67 @@ fn bitfields_to_allocation_units<E, I>(ctx: &BindgenContext,
     let mut unit_align = 0;
     let mut bitfields_in_unit = vec![];
 
+    // TODO(emilio): Determine this from attributes or pragma ms_struct
+    // directives. Also, perhaps we should check if the target is MSVC?
+    const is_ms_struct: bool = false;
+
     for bitfield in raw_bitfields {
         let bitfield_width = bitfield.bitfield().unwrap() as usize;
-        let bitfield_align = ctx.resolve_type(bitfield.ty())
-            .layout(ctx)
-            .expect("Bitfield without layout? Gah!")
-            .align;
-
-        if unit_size_in_bits != 0 &&
-           (bitfield_width == 0 ||
-            bitfield_width > unfilled_bits_in_unit) {
-            // We've reached the end of this allocation unit, so flush it
-            // and its bitfields.
-            unit_size_in_bits = align_to(unit_size_in_bits,
-                                                 bitfield_align);
-            flush_allocation_unit(fields,
-                                  bitfield_unit_count,
-                                  unit_size_in_bits,
-                                  unit_align,
-                                  mem::replace(&mut bitfields_in_unit, vec![]));
-
-            // Now we're working on a fresh bitfield allocation unit, so reset
-            // the current unit size and alignment.
-            unit_size_in_bits = 0;
-            unit_align = 0;
+        let bitfield_layout =
+            ctx.resolve_type(bitfield.ty())
+                .layout(ctx)
+                .expect("Bitfield without layout? Gah!");
+        let bitfield_size = bitfield_layout.size;
+        let bitfield_align = bitfield_layout.align;
+
+        let mut offset = unit_size_in_bits;
+        if is_ms_struct {
+            if unit_size_in_bits != 0 &&
+               (bitfield_width == 0 ||
+                bitfield_width > unfilled_bits_in_unit) {
+                // We've reached the end of this allocation unit, so flush it
+                // and its bitfields.
+                unit_size_in_bits = align_to(unit_size_in_bits, unit_align * 8);
+                flush_allocation_unit(fields,
+                                      bitfield_unit_count,
+                                      unit_size_in_bits,
+                                      unit_align,
+                                      mem::replace(&mut bitfields_in_unit, vec![]));
+
+                // 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;
+                }
+            }
+        } else {
+            if offset != 0 &&
+                (bitfield_width == 0 ||
+                 (offset & (bitfield_align * 8 - 1)) + bitfield_width > bitfield_size * 8) {
+                offset = align_to(offset, bitfield_align * 8);
+            }
         }
 
         // Only keep named bitfields around. Unnamed bitfields (with > 0
         // bitsize) are used for padding. Because the `Bitfield` struct stores
         // the bit-offset into its allocation unit where its bits begin, we
         // don't need any padding bits hereafter.
         if bitfield.name().is_some() {
-            bitfields_in_unit.push(Bitfield::new(unit_size_in_bits, bitfield));
+            bitfields_in_unit.push(Bitfield::new(offset, bitfield));
         }
 
-        unit_size_in_bits += bitfield_width;
-
         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
         // have seen so far, and use that to compute how many bits are still
         // available in the unit.