From 30a4722622883459c87d5de7a5ffae37849805d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 21 Jan 2017 14:56:11 +0100 Subject: [PATCH] codegen: Avoid generating invalid Rust code when a struct is not properly aligned. --- libbindgen/src/codegen/helpers.rs | 24 ++-- libbindgen/src/ir/comp.rs | 3 + libbindgen/src/ir/layout.rs | 30 +++- .../expectations/tests/16-byte-alignment.rs | 136 ++++++++++++++++++ libbindgen/tests/headers/16-byte-alignment.h | 33 +++++ 5 files changed, 211 insertions(+), 15 deletions(-) create mode 100644 libbindgen/tests/expectations/tests/16-byte-alignment.rs create mode 100644 libbindgen/tests/headers/16-byte-alignment.h diff --git a/libbindgen/src/codegen/helpers.rs b/libbindgen/src/codegen/helpers.rs index c09f007160..78fd7b6956 100644 --- a/libbindgen/src/codegen/helpers.rs +++ b/libbindgen/src/codegen/helpers.rs @@ -49,20 +49,22 @@ impl BlobTyBuilder { } pub fn build(self) -> P { - 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 6dfa1ece23..ac41b92900 100644 --- a/libbindgen/src/ir/comp.rs +++ b/libbindgen/src/ir/comp.rs @@ -914,6 +914,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 3a07f7e8c1..033fff6297 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 { + 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 0000000000..b28c0537e6 --- /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(::std::marker::PhantomData); +impl __BindgenUnionField { + #[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 ::std::default::Default for __BindgenUnionField { + #[inline] + fn default() -> Self { Self::new() } +} +impl ::std::clone::Clone for __BindgenUnionField { + #[inline] + fn clone(&self) -> Self { Self::new() } +} +impl ::std::marker::Copy for __BindgenUnionField { } +impl ::std::fmt::Debug for __BindgenUnionField { + 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, + pub sctp_tag: __BindgenUnionField, + 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::() + , 4usize); + assert_eq!(::std::mem::align_of::() + , 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::() , + 4usize); + assert_eq!(::std::mem::align_of::() , + 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::() , 12usize); + assert_eq!(::std::mem::align_of::() , 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, + pub sctp_tag: __BindgenUnionField, + 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::() + , 4usize); + assert_eq!(::std::mem::align_of::() + , 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::() , + 4usize); + assert_eq!(::std::mem::align_of::() , + 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::() , 36usize); + assert_eq!(::std::mem::align_of::() , 4usize); +} +impl Clone for rte_ipv6_tuple { + fn clone(&self) -> Self { *self } +} +#[repr(C)] +#[derive(Copy)] +pub struct rte_thash_tuple { + pub v4: __BindgenUnionField, + pub v6: __BindgenUnionField, + 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 0000000000..7a7f7548eb --- /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)));