Auto merge of #413 - emilio:debug-array-unaligned, r=fitzgen

codegen: Avoid generating invalid Rust code when a struct is not properly aligned.

r? @fitzgen

cc #412

5 files changed, 211 insertions, 15 deletions

diff --git a/libbindgen/src/codegen/helpers.rs b/libbindgen/src/codegen/helpers.rs
index b4cc75f5..06dadab0 100644
--- a/libbindgen/src/codegen/helpers.rs
+++ b/libbindgen/src/codegen/helpers.rs
@@ -49,20 +49,22 @@ impl BlobTyBuilder {
     }
 
     pub fn build(self) -> P<ast::Ty> {
-        use std::cmp;
+        let opaque = self.layout.opaque();
 
-        let ty_name = match self.layout.align {
-            8 => "u64",
-            4 => "u32",
-            2 => "u16",
-            1 | _ => "u8",
-        };
-        let data_len = if ty_name == "u8" {
-            self.layout.size
-        } else {
-            self.layout.size / cmp::max(self.layout.align, 1)
+        // FIXME(emilio, #412): We fall back to byte alignment, but there are
+        // some things that legitimately are more than 8-byte aligned.
+        //
+        // Eventually we should be able to `unwrap` here, but...
+        let ty_name = match opaque.known_rust_type_for_array() {
+            Some(ty) => ty,
+            None => {
+                warn!("Found unknown alignment on code generation!");
+                "u8"
+            }
         };
 
+        let data_len = opaque.array_size().unwrap_or(self.layout.size);
+
         let inner_ty = aster::AstBuilder::new().ty().path().id(ty_name).build();
         if data_len == 1 {
             inner_ty
diff --git a/libbindgen/src/ir/comp.rs b/libbindgen/src/ir/comp.rs
index 0d1c653e..968bf879 100644
--- a/libbindgen/src/ir/comp.rs
+++ b/libbindgen/src/ir/comp.rs
@@ -893,6 +893,9 @@ impl<'a> CanDeriveCopy<'a> for CompInfo {
 
         if self.kind == CompKind::Union {
             if !ctx.options().unstable_rust {
+                // NOTE: If there's no template parameters we can derive copy
+                // unconditionally, since arrays are magical for rustc, and
+                // __BindgenUnionField always implements copy.
                 return true;
             }
 
diff --git a/libbindgen/src/ir/layout.rs b/libbindgen/src/ir/layout.rs
index 3a07f7e8..033fff62 100644
--- a/libbindgen/src/ir/layout.rs
+++ b/libbindgen/src/ir/layout.rs
@@ -46,12 +46,35 @@ impl Layout {
 /// When we are treating a type as opaque, it is just a blob with a `Layout`.
 pub struct Opaque(pub Layout);
 
+impl Opaque {
+    /// Return the known rust type we should use to create a correctly-aligned
+    /// field with this layout.
+    pub fn known_rust_type_for_array(&self) -> Option<&'static str> {
+        Some(match self.0.align {
+            8 => "u64",
+            4 => "u32",
+            2 => "u16",
+            1 => "u8",
+            _ => return None,
+        })
+    }
+
+    /// Return the array size that an opaque type for this layout should have if
+    /// we know the correct type for it, or `None` otherwise.
+    pub fn array_size(&self) -> Option<usize> {
+        if self.known_rust_type_for_array().is_some() {
+            Some(self.0.size / cmp::max(self.0.align, 1))
+        } else {
+            None
+        }
+    }
+}
+
 impl CanDeriveDebug for Opaque {
     type Extra = ();
 
     fn can_derive_debug(&self, _: &BindgenContext, _: ()) -> bool {
-        let size_divisor = cmp::max(1, self.0.align);
-        self.0.size / size_divisor <= RUST_DERIVE_IN_ARRAY_LIMIT
+        self.array_size().map_or(false, |size| size <= RUST_DERIVE_IN_ARRAY_LIMIT)
     }
 }
 
@@ -59,8 +82,7 @@ impl<'a> CanDeriveCopy<'a> for Opaque {
     type Extra = ();
 
     fn can_derive_copy(&self, _: &BindgenContext, _: ()) -> bool {
-        let size_divisor = cmp::max(1, self.0.align);
-        self.0.size / size_divisor <= RUST_DERIVE_IN_ARRAY_LIMIT
+        self.array_size().map_or(false, |size| size <= RUST_DERIVE_IN_ARRAY_LIMIT)
     }
 
     fn can_derive_copy_in_array(&self, ctx: &BindgenContext, _: ()) -> bool {
diff --git a/libbindgen/tests/expectations/tests/16-byte-alignment.rs b/libbindgen/tests/expectations/tests/16-byte-alignment.rs
new file mode 100644
index 00000000..b28c0537
--- /dev/null
+++ b/libbindgen/tests/expectations/tests/16-byte-alignment.rs
@@ -0,0 +1,136 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+#[repr(C)]
+pub struct __BindgenUnionField<T>(::std::marker::PhantomData<T>);
+impl <T> __BindgenUnionField<T> {
+    #[inline]
+    pub fn new() -> Self { __BindgenUnionField(::std::marker::PhantomData) }
+    #[inline]
+    pub unsafe fn as_ref(&self) -> &T { ::std::mem::transmute(self) }
+    #[inline]
+    pub unsafe fn as_mut(&mut self) -> &mut T { ::std::mem::transmute(self) }
+}
+impl <T> ::std::default::Default for __BindgenUnionField<T> {
+    #[inline]
+    fn default() -> Self { Self::new() }
+}
+impl <T> ::std::clone::Clone for __BindgenUnionField<T> {
+    #[inline]
+    fn clone(&self) -> Self { Self::new() }
+}
+impl <T> ::std::marker::Copy for __BindgenUnionField<T> { }
+impl <T> ::std::fmt::Debug for __BindgenUnionField<T> {
+    fn fmt(&self, fmt: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
+        fmt.write_str("__BindgenUnionField")
+    }
+}
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct rte_ipv4_tuple {
+    pub src_addr: u32,
+    pub dst_addr: u32,
+    pub __bindgen_anon_1: rte_ipv4_tuple__bindgen_ty_1,
+}
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct rte_ipv4_tuple__bindgen_ty_1 {
+    pub __bindgen_anon_1: __BindgenUnionField<rte_ipv4_tuple__bindgen_ty_1__bindgen_ty_1>,
+    pub sctp_tag: __BindgenUnionField<u32>,
+    pub bindgen_union_field: u32,
+}
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct rte_ipv4_tuple__bindgen_ty_1__bindgen_ty_1 {
+    pub dport: u16,
+    pub sport: u16,
+}
+#[test]
+fn bindgen_test_layout_rte_ipv4_tuple__bindgen_ty_1__bindgen_ty_1() {
+    assert_eq!(::std::mem::size_of::<rte_ipv4_tuple__bindgen_ty_1__bindgen_ty_1>()
+               , 4usize);
+    assert_eq!(::std::mem::align_of::<rte_ipv4_tuple__bindgen_ty_1__bindgen_ty_1>()
+               , 2usize);
+}
+impl Clone for rte_ipv4_tuple__bindgen_ty_1__bindgen_ty_1 {
+    fn clone(&self) -> Self { *self }
+}
+#[test]
+fn bindgen_test_layout_rte_ipv4_tuple__bindgen_ty_1() {
+    assert_eq!(::std::mem::size_of::<rte_ipv4_tuple__bindgen_ty_1>() ,
+               4usize);
+    assert_eq!(::std::mem::align_of::<rte_ipv4_tuple__bindgen_ty_1>() ,
+               4usize);
+}
+impl Clone for rte_ipv4_tuple__bindgen_ty_1 {
+    fn clone(&self) -> Self { *self }
+}
+#[test]
+fn bindgen_test_layout_rte_ipv4_tuple() {
+    assert_eq!(::std::mem::size_of::<rte_ipv4_tuple>() , 12usize);
+    assert_eq!(::std::mem::align_of::<rte_ipv4_tuple>() , 4usize);
+}
+impl Clone for rte_ipv4_tuple {
+    fn clone(&self) -> Self { *self }
+}
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct rte_ipv6_tuple {
+    pub src_addr: [u8; 16usize],
+    pub dst_addr: [u8; 16usize],
+    pub __bindgen_anon_1: rte_ipv6_tuple__bindgen_ty_1,
+}
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct rte_ipv6_tuple__bindgen_ty_1 {
+    pub __bindgen_anon_1: __BindgenUnionField<rte_ipv6_tuple__bindgen_ty_1__bindgen_ty_1>,
+    pub sctp_tag: __BindgenUnionField<u32>,
+    pub bindgen_union_field: u32,
+}
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct rte_ipv6_tuple__bindgen_ty_1__bindgen_ty_1 {
+    pub dport: u16,
+    pub sport: u16,
+}
+#[test]
+fn bindgen_test_layout_rte_ipv6_tuple__bindgen_ty_1__bindgen_ty_1() {
+    assert_eq!(::std::mem::size_of::<rte_ipv6_tuple__bindgen_ty_1__bindgen_ty_1>()
+               , 4usize);
+    assert_eq!(::std::mem::align_of::<rte_ipv6_tuple__bindgen_ty_1__bindgen_ty_1>()
+               , 2usize);
+}
+impl Clone for rte_ipv6_tuple__bindgen_ty_1__bindgen_ty_1 {
+    fn clone(&self) -> Self { *self }
+}
+#[test]
+fn bindgen_test_layout_rte_ipv6_tuple__bindgen_ty_1() {
+    assert_eq!(::std::mem::size_of::<rte_ipv6_tuple__bindgen_ty_1>() ,
+               4usize);
+    assert_eq!(::std::mem::align_of::<rte_ipv6_tuple__bindgen_ty_1>() ,
+               4usize);
+}
+impl Clone for rte_ipv6_tuple__bindgen_ty_1 {
+    fn clone(&self) -> Self { *self }
+}
+#[test]
+fn bindgen_test_layout_rte_ipv6_tuple() {
+    assert_eq!(::std::mem::size_of::<rte_ipv6_tuple>() , 36usize);
+    assert_eq!(::std::mem::align_of::<rte_ipv6_tuple>() , 4usize);
+}
+impl Clone for rte_ipv6_tuple {
+    fn clone(&self) -> Self { *self }
+}
+#[repr(C)]
+#[derive(Copy)]
+pub struct rte_thash_tuple {
+    pub v4: __BindgenUnionField<rte_ipv4_tuple>,
+    pub v6: __BindgenUnionField<rte_ipv6_tuple>,
+    pub bindgen_union_field: [u8; 48usize],
+}
+impl Clone for rte_thash_tuple {
+    fn clone(&self) -> Self { *self }
+}
diff --git a/libbindgen/tests/headers/16-byte-alignment.h b/libbindgen/tests/headers/16-byte-alignment.h
new file mode 100644
index 00000000..7a7f7548
--- /dev/null
+++ b/libbindgen/tests/headers/16-byte-alignment.h
@@ -0,0 +1,33 @@
+
+typedef unsigned char uint8_t;
+typedef unsigned short uint16_t;
+typedef unsigned int uint32_t;
+
+struct rte_ipv4_tuple {
+        uint32_t        src_addr;
+        uint32_t        dst_addr;
+        union {
+                struct {
+                        uint16_t dport;
+                        uint16_t sport;
+                };
+                uint32_t        sctp_tag;
+        };
+};
+
+struct rte_ipv6_tuple {
+        uint8_t         src_addr[16];
+        uint8_t         dst_addr[16];
+        union {
+                struct {
+                        uint16_t dport;
+                        uint16_t sport;
+                };
+                uint32_t        sctp_tag;
+        };
+};
+
+union rte_thash_tuple {
+        struct rte_ipv4_tuple   v4;
+        struct rte_ipv6_tuple   v6;
+} __attribute__((aligned(16)));