codegen: Properly track alignment of unions.

This makes us not unnecessarily add repr(align) to unions. Closes rust-lang#1498.
emilio · Mar 4, 2019 · 23d9229 · 23d9229
1 parent 3dd3ce7
commit 23d9229
Show file tree

Hide file tree

Showing 36 changed files with 179 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -101,7 +101,12 @@ Released 2019/03/04
 * Default rust target was changed to 1.33, which means that bindgen can get much
   more often the layout of structs right. [#1529][]
 
+## Fixed
+
+* Bindgen will output repr(align) just when needed for unions. [#1498][]
+
 [#1529]: https://github.com/rust-lang-nursery/rust-bindgen/issues/1529
+[#1498]: https://github.com/rust-lang-nursery/rust-bindgen/issues/1498
 
 --------------------------------------------------------------------------------
 

diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
@@ -1589,6 +1589,7 @@ impl CodeGenerator for CompInfo {
             // TODO(emilio): It'd be nice to unify this with the struct path
             // above somehow.
             let layout = layout.expect("Unable to get layout information?");
+            struct_layout.saw_union(layout);
 
             if struct_layout.requires_explicit_align(layout) {
                 explicit_align = Some(layout.align);
@@ -1600,8 +1601,6 @@ impl CodeGenerator for CompInfo {
                     _bindgen_union_align: #ty ,
                 }
             } else {
-                struct_layout.saw_union(layout);
-
                 quote! {
                     pub bindgen_union_field: #ty ,
                 }

diff --git a/src/codegen/struct_layout.rs b/src/codegen/struct_layout.rs
@@ -85,9 +85,9 @@ impl<'a> StructLayoutTracker<'a> {
         name: &'a str,
     ) -> Self {
         StructLayoutTracker {
-            name: name,
-            ctx: ctx,
-            comp: comp,
+            name,
+            ctx,
+            comp,
             is_packed: comp.is_packed(ctx, &ty.layout(ctx)),
             latest_offset: 0,
             padding_count: 0,

diff --git a/tests/expectations/tests/16-byte-alignment.rs b/tests/expectations/tests/16-byte-alignment.rs
@@ -15,7 +15,6 @@ pub struct rte_ipv4_tuple {
     pub __bindgen_anon_1: rte_ipv4_tuple__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union rte_ipv4_tuple__bindgen_ty_1 {
     pub __bindgen_anon_1: rte_ipv4_tuple__bindgen_ty_1__bindgen_ty_1,
@@ -149,7 +148,6 @@ pub struct rte_ipv6_tuple {
     pub __bindgen_anon_1: rte_ipv6_tuple__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union rte_ipv6_tuple__bindgen_ty_1 {
     pub __bindgen_anon_1: rte_ipv6_tuple__bindgen_ty_1__bindgen_ty_1,

diff --git a/tests/expectations/tests/anon_struct_in_union.rs b/tests/expectations/tests/anon_struct_in_union.rs
@@ -13,7 +13,6 @@ pub struct s {
     pub u: s__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union s__bindgen_ty_1 {
     pub field: s__bindgen_ty_1_inner,

diff --git a/tests/expectations/tests/anon_union.rs b/tests/expectations/tests/anon_union.rs
@@ -33,7 +33,6 @@ pub struct TErrorResult_DOMExceptionInfo {
     _unused: [u8; 0],
 }
 #[repr(C)]
-#[repr(align(8))]
 pub union TErrorResult__bindgen_ty_1 {
     pub mMessage: *mut TErrorResult_Message,
     pub mDOMExceptionInfo: *mut TErrorResult_DOMExceptionInfo,

diff --git a/tests/expectations/tests/class.rs b/tests/expectations/tests/class.rs
@@ -335,7 +335,6 @@ impl Default for IncompleteArrayNonCopiable {
     }
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union Union {
     pub d: f32,

diff --git a/tests/expectations/tests/class_with_inner_struct.rs b/tests/expectations/tests/class_with_inner_struct.rs
@@ -54,7 +54,6 @@ fn bindgen_test_layout_A_Segment() {
     );
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union A__bindgen_ty_1 {
     pub f: ::std::os::raw::c_int,
@@ -89,7 +88,6 @@ impl Default for A__bindgen_ty_1 {
     }
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union A__bindgen_ty_2 {
     pub d: ::std::os::raw::c_int,
@@ -233,7 +231,6 @@ pub struct C {
     pub __bindgen_anon_1: C__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union C__bindgen_ty_1 {
     pub mFunc: C__bindgen_ty_1__bindgen_ty_1,

diff --git a/tests/expectations/tests/derive-debug-mangle-name.rs b/tests/expectations/tests/derive-debug-mangle-name.rs
@@ -15,7 +15,6 @@ pub struct perf_event_attr {
     pub __bindgen_anon_1: perf_event_attr__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union perf_event_attr__bindgen_ty_1 {
     pub b: ::std::os::raw::c_int,

diff --git a/tests/expectations/tests/derive-partialeq-anonfield.rs b/tests/expectations/tests/derive-partialeq-anonfield.rs
@@ -14,7 +14,6 @@ pub struct rte_mbuf {
     pub __bindgen_anon_1: rte_mbuf__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(1))]
 #[derive(Copy, Clone)]
 pub union rte_mbuf__bindgen_ty_1 {
     _bindgen_union_align: [u8; 0usize],

diff --git a/tests/expectations/tests/derive-partialeq-pointer.rs b/tests/expectations/tests/derive-partialeq-pointer.rs
@@ -41,7 +41,6 @@ pub struct c {
     pub __bindgen_anon_1: c__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(1))]
 #[derive(Copy, Clone)]
 pub union c__bindgen_ty_1 {
     _bindgen_union_align: u8,

diff --git a/tests/expectations/tests/derive-partialeq-union.rs b/tests/expectations/tests/derive-partialeq-union.rs
@@ -9,7 +9,6 @@
 
 /// Deriving PartialEq for rust unions is not supported.
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union ShouldNotDerivePartialEq {
     pub a: ::std::os::raw::c_char,

diff --git a/tests/expectations/tests/issue-1285.rs b/tests/expectations/tests/issue-1285.rs
@@ -12,7 +12,6 @@ pub struct foo {
     pub bar: foo__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 pub union foo__bindgen_ty_1 {
     pub a: ::std::os::raw::c_uint,
     pub b: ::std::os::raw::c_ushort,

diff --git a/tests/expectations/tests/issue-1498.rs b/tests/expectations/tests/issue-1498.rs
@@ -0,0 +1,153 @@
+/* automatically generated by rust-bindgen */
+
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+#[repr(C, packed)]
+#[derive(Copy, Clone)]
+pub struct rte_memseg {
+    ///< Start physical address.
+    pub phys_addr: u64,
+    pub __bindgen_anon_1: rte_memseg__bindgen_ty_1,
+    ///< Length of the segment.
+    pub len: usize,
+    ///< The pagesize of underlying memory
+    pub hugepage_sz: u64,
+    ///< NUMA socket ID.
+    pub socket_id: i32,
+    ///< Number of channels.
+    pub nchannel: u32,
+    ///< Number of ranks.
+    pub nrank: u32,
+}
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub union rte_memseg__bindgen_ty_1 {
+    ///< Start virtual address.
+    pub addr: *mut ::std::os::raw::c_void,
+    ///< Makes sure addr is always 64 bits
+    pub addr_64: u64,
+    _bindgen_union_align: u64,
+}
+#[test]
+fn bindgen_test_layout_rte_memseg__bindgen_ty_1() {
+    assert_eq!(
+        ::std::mem::size_of::<rte_memseg__bindgen_ty_1>(),
+        8usize,
+        concat!("Size of: ", stringify!(rte_memseg__bindgen_ty_1))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<rte_memseg__bindgen_ty_1>(),
+        8usize,
+        concat!("Alignment of ", stringify!(rte_memseg__bindgen_ty_1))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<rte_memseg__bindgen_ty_1>())).addr as *const _ as usize },
+        0usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg__bindgen_ty_1),
+            "::",
+            stringify!(addr)
+        )
+    );
+    assert_eq!(
+        unsafe {
+            &(*(::std::ptr::null::<rte_memseg__bindgen_ty_1>())).addr_64 as *const _ as usize
+        },
+        0usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg__bindgen_ty_1),
+            "::",
+            stringify!(addr_64)
+        )
+    );
+}
+impl Default for rte_memseg__bindgen_ty_1 {
+    fn default() -> Self {
+        unsafe { ::std::mem::zeroed() }
+    }
+}
+#[test]
+fn bindgen_test_layout_rte_memseg() {
+    assert_eq!(
+        ::std::mem::size_of::<rte_memseg>(),
+        44usize,
+        concat!("Size of: ", stringify!(rte_memseg))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<rte_memseg>(),
+        1usize,
+        concat!("Alignment of ", stringify!(rte_memseg))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<rte_memseg>())).phys_addr as *const _ as usize },
+        0usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg),
+            "::",
+            stringify!(phys_addr)
+        )
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<rte_memseg>())).len as *const _ as usize },
+        16usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg),
+            "::",
+            stringify!(len)
+        )
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<rte_memseg>())).hugepage_sz as *const _ as usize },
+        24usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg),
+            "::",
+            stringify!(hugepage_sz)
+        )
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<rte_memseg>())).socket_id as *const _ as usize },
+        32usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg),
+            "::",
+            stringify!(socket_id)
+        )
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<rte_memseg>())).nchannel as *const _ as usize },
+        36usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg),
+            "::",
+            stringify!(nchannel)
+        )
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<rte_memseg>())).nrank as *const _ as usize },
+        40usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(rte_memseg),
+            "::",
+            stringify!(nrank)
+        )
+    );
+}
+impl Default for rte_memseg {
+    fn default() -> Self {
+        unsafe { ::std::mem::zeroed() }
+    }
+}
diff --git a/tests/expectations/tests/issue-493.rs b/tests/expectations/tests/issue-493.rs
@@ -83,7 +83,6 @@ pub struct basic_string___short {
     pub __data_: *mut basic_string_value_type,
 }
 #[repr(C)]
-#[repr(align(1))]
 pub union basic_string___short__bindgen_ty_1 {
     pub __size_: ::std::os::raw::c_uchar,
     pub __lx: basic_string_value_type,

diff --git a/tests/expectations/tests/jsval_layout_opaque.rs b/tests/expectations/tests/jsval_layout_opaque.rs
@@ -176,7 +176,6 @@ pub enum JSWhyMagic {
     JS_WHY_MAGIC_COUNT = 18,
 }
 #[repr(C)]
-#[repr(align(8))]
 #[derive(Copy, Clone)]
 pub union jsval_layout {
     pub asBits: u64,
@@ -259,7 +258,6 @@ pub struct jsval_layout__bindgen_ty_2 {
     pub payload: jsval_layout__bindgen_ty_2__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union jsval_layout__bindgen_ty_2__bindgen_ty_1 {
     pub i32: i32,

diff --git a/tests/expectations/tests/layout_eth_conf.rs b/tests/expectations/tests/layout_eth_conf.rs
@@ -1830,7 +1830,6 @@ impl Default for rte_eth_conf__bindgen_ty_1 {
     }
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union rte_eth_conf__bindgen_ty_2 {
     pub vmdq_dcb_tx_conf: rte_eth_vmdq_dcb_tx_conf,

diff --git a/tests/expectations/tests/layout_mbuf.rs b/tests/expectations/tests/layout_mbuf.rs
@@ -180,7 +180,6 @@ pub struct rte_mbuf {
 /// or non-atomic) is controlled by the CONFIG_RTE_MBUF_REFCNT_ATOMIC
 /// config option.
 #[repr(C)]
-#[repr(align(2))]
 #[derive(Copy, Clone)]
 pub union rte_mbuf__bindgen_ty_1 {
     ///< Atomically accessed refcnt
@@ -230,7 +229,6 @@ impl Default for rte_mbuf__bindgen_ty_1 {
     }
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union rte_mbuf__bindgen_ty_2 {
     ///< L2/L3/L4 and tunnel information.
@@ -415,7 +413,6 @@ impl Default for rte_mbuf__bindgen_ty_2 {
     }
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union rte_mbuf__bindgen_ty_3 {
     ///< RSS hash result if RSS enabled
@@ -435,7 +432,6 @@ pub struct rte_mbuf__bindgen_ty_3__bindgen_ty_1 {
     pub hi: u32,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union rte_mbuf__bindgen_ty_3__bindgen_ty_1__bindgen_ty_1 {
     pub __bindgen_anon_1: rte_mbuf__bindgen_ty_3__bindgen_ty_1__bindgen_ty_1__bindgen_ty_1,
@@ -672,7 +668,6 @@ impl Default for rte_mbuf__bindgen_ty_3 {
     }
 }
 #[repr(C)]
-#[repr(align(8))]
 #[derive(Copy, Clone)]
 pub union rte_mbuf__bindgen_ty_4 {
     ///< Can be used for external metadata
@@ -720,7 +715,6 @@ impl Default for rte_mbuf__bindgen_ty_4 {
     }
 }
 #[repr(C)]
-#[repr(align(8))]
 #[derive(Copy, Clone)]
 pub union rte_mbuf__bindgen_ty_5 {
     ///< combined for easy fetch

diff --git a/tests/expectations/tests/struct_with_anon_union.rs b/tests/expectations/tests/struct_with_anon_union.rs
@@ -13,7 +13,6 @@ pub struct foo {
     pub bar: foo__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union foo__bindgen_ty_1 {
     pub a: ::std::os::raw::c_uint,

diff --git a/tests/expectations/tests/struct_with_anon_unnamed_union.rs b/tests/expectations/tests/struct_with_anon_unnamed_union.rs
@@ -13,7 +13,6 @@ pub struct foo {
     pub __bindgen_anon_1: foo__bindgen_ty_1,
 }
 #[repr(C)]
-#[repr(align(4))]
 #[derive(Copy, Clone)]
 pub union foo__bindgen_ty_1 {
     pub a: ::std::os::raw::c_uint,