comp: Fix bitfields to allow underaligned fields after them to take p…

…adding space. Fixes #1947. There are two separate issues here: First, the change in comp.rs ensures that we don't round up the amount of storage to the alignment of the bitfield. That generates the "expected" output in #1947 (`__BindgenBitfieldUnit<[u8; 3], u16>`). But that's still not enough to fix that test-case because __BindgenBitfieldUnit would be aligned and have padding, and Rust won't put the extra field in the padding. In order to ensure the bitfield starts at the right alignment, but that Rust can put stuff in the extra field, we need to make a breaking change and split the generated fields in two: One preceding that guarantees alignment, and the actual storage, bit-aligned. This keeps the existing behavior while fixing that test-case.
rust-lang · Dec 20, 2020 · a4558a2 · a4558a2
1 parent 4ce4b93
commit a4558a2
Show file tree

Hide file tree

Showing 41 changed files with 770 additions and 524 deletions.
diff --git a/bindgen-integration/src/lib.rs b/bindgen-integration/src/lib.rs
@@ -175,16 +175,19 @@ fn test_bitfields_seventh() {
 fn test_bitfield_constructors() {
     use std::mem;
     let mut first = bindings::bitfields::First {
+        _bitfield_align_1: [],
         _bitfield_1: bindings::bitfields::First::new_bitfield_1(1, 2, 3),
     };
     assert!(unsafe { first.assert(1, 2, 3) });
 
     let mut second = bindings::bitfields::Second {
+        _bitfield_align_1: [],
         _bitfield_1: bindings::bitfields::Second::new_bitfield_1(1337, true),
     };
     assert!(unsafe { second.assert(1337, true) });
 
     let mut third = bindings::bitfields::Third {
+        _bitfield_align_1: [],
         _bitfield_1: bindings::bitfields::Third::new_bitfield_1(
             42,
             false,

diff --git a/src/codegen/bitfield_unit.rs b/src/codegen/bitfield_unit.rs
@@ -1,18 +1,17 @@
 #[repr(C)]
 #[derive(Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
-pub struct __BindgenBitfieldUnit<Storage, Align> {
+pub struct __BindgenBitfieldUnit<Storage> {
     storage: Storage,
-    align: [Align; 0],
 }
 
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align> {
+impl<Storage> __BindgenBitfieldUnit<Storage> {
     #[inline]
     pub const fn new(storage: Storage) -> Self {
-        Self { storage, align: [] }
+        Self { storage }
     }
 }
 
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align>
+impl<Storage> __BindgenBitfieldUnit<Storage>
 where
     Storage: AsRef<[u8]> + AsMut<[u8]>,
 {

diff --git a/src/codegen/bitfield_unit_tests.rs b/src/codegen/bitfield_unit_tests.rs
@@ -22,12 +22,10 @@
 //! ```
 
 use super::bitfield_unit::__BindgenBitfieldUnit;
-use std::mem;
 
 #[test]
 fn bitfield_unit_get_bit() {
-    let unit =
-        __BindgenBitfieldUnit::<[u8; 2], u64>::new([0b10011101, 0b00011101]);
+    let unit = __BindgenBitfieldUnit::<[u8; 2]>::new([0b10011101, 0b00011101]);
 
     let mut bits = vec![];
     for i in 0..16 {
@@ -50,7 +48,7 @@ fn bitfield_unit_get_bit() {
 #[test]
 fn bitfield_unit_set_bit() {
     let mut unit =
-        __BindgenBitfieldUnit::<[u8; 2], u64>::new([0b00000000, 0b00000000]);
+        __BindgenBitfieldUnit::<[u8; 2]>::new([0b00000000, 0b00000000]);
 
     for i in 0..16 {
         if i % 3 == 0 {
@@ -63,7 +61,7 @@ fn bitfield_unit_set_bit() {
     }
 
     let mut unit =
-        __BindgenBitfieldUnit::<[u8; 2], u64>::new([0b11111111, 0b11111111]);
+        __BindgenBitfieldUnit::<[u8; 2]>::new([0b11111111, 0b11111111]);
 
     for i in 0..16 {
         if i % 3 == 0 {
@@ -76,43 +74,6 @@ fn bitfield_unit_set_bit() {
     }
 }
 
-#[test]
-fn bitfield_unit_align() {
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 1], u8>>(),
-        mem::align_of::<u8>()
-    );
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 1], u16>>(),
-        mem::align_of::<u16>()
-    );
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 1], u32>>(),
-        mem::align_of::<u32>()
-    );
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 1], u64>>(),
-        mem::align_of::<u64>()
-    );
-
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 8], u8>>(),
-        mem::align_of::<u8>()
-    );
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 8], u16>>(),
-        mem::align_of::<u16>()
-    );
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 8], u32>>(),
-        mem::align_of::<u32>()
-    );
-    assert_eq!(
-        mem::align_of::<__BindgenBitfieldUnit<[u8; 8], u64>>(),
-        mem::align_of::<u64>()
-    );
-}
-
 macro_rules! bitfield_unit_get {
     (
         $(
@@ -123,7 +84,7 @@ macro_rules! bitfield_unit_get {
         fn bitfield_unit_get() {
             $({
                 let expected = $expected;
-                let unit = __BindgenBitfieldUnit::<_, u64>::new($storage);
+                let unit = __BindgenBitfieldUnit::<_>::new($storage);
                 let actual = unit.get($start, $len);
 
                 println!();
@@ -223,7 +184,7 @@ macro_rules! bitfield_unit_set {
         #[test]
         fn bitfield_unit_set() {
             $(
-                let mut unit = __BindgenBitfieldUnit::<[u8; 4], u64>::new([0, 0, 0, 0]);
+                let mut unit = __BindgenBitfieldUnit::<[u8; 4]>::new([0, 0, 0, 0]);
                 unit.set($start, $len, $val);
                 let actual = unit.get(0, 32);
 

diff --git a/src/codegen/helpers.rs b/src/codegen/helpers.rs
@@ -120,16 +120,9 @@ pub fn bitfield_unit(ctx: &BindgenContext, layout: Layout) -> TokenStream {
         tokens.append_all(quote! { root:: });
     }
 
-    let align = match layout.align {
-        n if n >= 8 => quote! { u64 },
-        4 => quote! { u32 },
-        2 => quote! { u16 },
-        _ => quote! { u8  },
-    };
-
     let size = layout.size;
     tokens.append_all(quote! {
-        __BindgenBitfieldUnit<[u8; #size], #align>
+        __BindgenBitfieldUnit<[u8; #size]>
     });
 
     tokens

diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
@@ -1437,6 +1437,21 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
             }
         };
 
+        {
+            let align_field_name = format!("_bitfield_align_{}", self.nth());
+            let align_field_ident = ctx.rust_ident(&align_field_name);
+            let align_ty = match self.layout().align {
+                n if n >= 8 => quote! { u64 },
+                4 => quote! { u32 },
+                2 => quote! { u16 },
+                _ => quote! { u8  },
+            };
+            let align_field = quote! {
+                pub #align_field_ident: [#align_ty; 0],
+            };
+            fields.extend(Some(align_field));
+        }
+
         let unit_field_name = format!("_bitfield_{}", self.nth());
         let unit_field_ident = ctx.rust_ident(&unit_field_name);
 

diff --git a/src/ir/comp.rs b/src/ir/comp.rs
@@ -575,7 +575,7 @@ where
         } else {
             bytes_from_bits_pow2(unit_align_in_bits)
         };
-        let size = align_to(unit_size_in_bits, align * 8) / 8;
+        let size = align_to(unit_size_in_bits, 8) / 8;
         let layout = Layout::new(size, align);
         fields.extend(Some(Field::Bitfields(BitfieldUnit {
             nth: *bitfield_unit_count,

diff --git a/tests/expectations/tests/bitfield-32bit-overflow.rs b/tests/expectations/tests/bitfield-32bit-overflow.rs
@@ -7,17 +7,16 @@
 
 #[repr(C)]
 #[derive(Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
-pub struct __BindgenBitfieldUnit<Storage, Align> {
+pub struct __BindgenBitfieldUnit<Storage> {
     storage: Storage,
-    align: [Align; 0],
 }
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align> {
+impl<Storage> __BindgenBitfieldUnit<Storage> {
     #[inline]
     pub const fn new(storage: Storage) -> Self {
-        Self { storage, align: [] }
+        Self { storage }
     }
 }
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align>
+impl<Storage> __BindgenBitfieldUnit<Storage>
 where
     Storage: AsRef<[u8]> + AsMut<[u8]>,
 {
@@ -95,7 +94,8 @@ where
 #[repr(C, packed)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct MuchBitfield {
-    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 5usize], u8>,
+    pub _bitfield_align_1: [u8; 0],
+    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 5usize]>,
 }
 #[test]
 fn bindgen_test_layout_MuchBitfield() {
@@ -575,11 +575,9 @@ impl MuchBitfield {
         m30: ::std::os::raw::c_char,
         m31: ::std::os::raw::c_char,
         m32: ::std::os::raw::c_char,
-    ) -> __BindgenBitfieldUnit<[u8; 5usize], u8> {
-        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<
-            [u8; 5usize],
-            u8,
-        > = Default::default();
+    ) -> __BindgenBitfieldUnit<[u8; 5usize]> {
+        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 5usize]> =
+            Default::default();
         __bindgen_bitfield_unit.set(0usize, 1u8, {
             let m0: u8 = unsafe { ::std::mem::transmute(m0) };
             m0 as u64

diff --git a/tests/expectations/tests/bitfield-large.rs b/tests/expectations/tests/bitfield-large.rs
@@ -7,17 +7,16 @@
 
 #[repr(C)]
 #[derive(Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
-pub struct __BindgenBitfieldUnit<Storage, Align> {
+pub struct __BindgenBitfieldUnit<Storage> {
     storage: Storage,
-    align: [Align; 0],
 }
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align> {
+impl<Storage> __BindgenBitfieldUnit<Storage> {
     #[inline]
     pub const fn new(storage: Storage) -> Self {
-        Self { storage, align: [] }
+        Self { storage }
     }
 }
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align>
+impl<Storage> __BindgenBitfieldUnit<Storage>
 where
     Storage: AsRef<[u8]> + AsMut<[u8]>,
 {
@@ -96,7 +95,8 @@ where
 #[repr(align(16))]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct HasBigBitfield {
-    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 16usize], u64>,
+    pub _bitfield_align_1: [u64; 0],
+    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 16usize]>,
 }
 #[test]
 fn bindgen_test_layout_HasBigBitfield() {
@@ -126,13 +126,9 @@ impl HasBigBitfield {
         }
     }
     #[inline]
-    pub fn new_bitfield_1(
-        x: i128,
-    ) -> __BindgenBitfieldUnit<[u8; 16usize], u64> {
-        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<
-            [u8; 16usize],
-            u64,
-        > = Default::default();
+    pub fn new_bitfield_1(x: i128) -> __BindgenBitfieldUnit<[u8; 16usize]> {
+        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 16usize]> =
+            Default::default();
         __bindgen_bitfield_unit.set(0usize, 128u8, {
             let x: u128 = unsafe { ::std::mem::transmute(x) };
             x as u64
@@ -144,7 +140,8 @@ impl HasBigBitfield {
 #[repr(align(16))]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct HasTwoBigBitfields {
-    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 16usize], u64>,
+    pub _bitfield_align_1: [u64; 0],
+    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 16usize]>,
 }
 #[test]
 fn bindgen_test_layout_HasTwoBigBitfields() {
@@ -190,11 +187,9 @@ impl HasTwoBigBitfields {
     pub fn new_bitfield_1(
         x: i128,
         y: i128,
-    ) -> __BindgenBitfieldUnit<[u8; 16usize], u64> {
-        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<
-            [u8; 16usize],
-            u64,
-        > = Default::default();
+    ) -> __BindgenBitfieldUnit<[u8; 16usize]> {
+        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 16usize]> =
+            Default::default();
         __bindgen_bitfield_unit.set(0usize, 80u8, {
             let x: u128 = unsafe { ::std::mem::transmute(x) };
             x as u64

diff --git a/tests/expectations/tests/bitfield-linux-32.rs b/tests/expectations/tests/bitfield-linux-32.rs
@@ -7,17 +7,16 @@
 
 #[repr(C)]
 #[derive(Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
-pub struct __BindgenBitfieldUnit<Storage, Align> {
+pub struct __BindgenBitfieldUnit<Storage> {
     storage: Storage,
-    align: [Align; 0],
 }
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align> {
+impl<Storage> __BindgenBitfieldUnit<Storage> {
     #[inline]
     pub const fn new(storage: Storage) -> Self {
-        Self { storage, align: [] }
+        Self { storage }
     }
 }
-impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align>
+impl<Storage> __BindgenBitfieldUnit<Storage>
 where
     Storage: AsRef<[u8]> + AsMut<[u8]>,
 {
@@ -96,7 +95,8 @@ where
 #[derive(Debug, Default, Copy, Clone)]
 pub struct Test {
     pub foo: u64,
-    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 8usize], u64>,
+    pub _bitfield_align_1: [u64; 0],
+    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 8usize]>,
 }
 #[test]
 fn bindgen_test_layout_Test() {
@@ -147,11 +147,9 @@ impl Test {
     pub fn new_bitfield_1(
         x: u64,
         y: u64,
-    ) -> __BindgenBitfieldUnit<[u8; 8usize], u64> {
-        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<
-            [u8; 8usize],
-            u64,
-        > = Default::default();
+    ) -> __BindgenBitfieldUnit<[u8; 8usize]> {
+        let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 8usize]> =
+            Default::default();
         __bindgen_bitfield_unit.set(0usize, 56u8, {
             let x: u64 = unsafe { ::std::mem::transmute(x) };
             x as u64