Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions libbindgen/src/codegen/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions libbindgen/src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
30 changes: 26 additions & 4 deletions libbindgen/src/ir/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,43 @@ 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff is great :)

}
}

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 {
Expand Down
136 changes: 136 additions & 0 deletions libbindgen/tests/expectations/tests/16-byte-alignment.rs
Original file line number Diff line number Diff line change
@@ -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 }
}
33 changes: 33 additions & 0 deletions libbindgen/tests/headers/16-byte-alignment.h
Original file line number Diff line number Diff line change
@@ -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)));