ty: More struct layout fixes for overaligned integers / aliases. This fixes most of the test-cases discussed in #3453. The remaining one is related to bitfields and needs a bit more thought (I think we currently don't pad bitfields based on the clang-reported offset and maybe should).
diff --git a/bindgen-tests/tests/expectations/tests/issue-3406.rs b/bindgen-tests/tests/expectations/tests/issue-3406.rs index f200006..a755589 100644 --- a/bindgen-tests/tests/expectations/tests/issue-3406.rs +++ b/bindgen-tests/tests/expectations/tests/issue-3406.rs
@@ -1,5 +1,35 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] +#[derive(Default)] +pub struct __IncompleteArrayField<T>(::std::marker::PhantomData<T>, [T; 0]); +impl<T> __IncompleteArrayField<T> { + #[inline] + pub const fn new() -> Self { + __IncompleteArrayField(::std::marker::PhantomData, []) + } + #[inline] + pub fn as_ptr(&self) -> *const T { + self as *const _ as *const T + } + #[inline] + pub fn as_mut_ptr(&mut self) -> *mut T { + self as *mut _ as *mut T + } + #[inline] + pub unsafe fn as_slice(&self, len: usize) -> &[T] { + ::std::slice::from_raw_parts(self.as_ptr(), len) + } + #[inline] + pub unsafe fn as_mut_slice(&mut self, len: usize) -> &mut [T] { + ::std::slice::from_raw_parts_mut(self.as_mut_ptr(), len) + } +} +impl<T> ::std::fmt::Debug for __IncompleteArrayField<T> { + fn fmt(&self, fmt: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { + fmt.write_str("__IncompleteArrayField") + } +} +#[repr(C)] #[repr(align(16))] #[derive(Debug, Default, Copy, Clone)] pub struct Inner { @@ -75,3 +105,75 @@ ["Offset of field: Outer4::before"][::std::mem::offset_of!(Outer4, before) - 0usize]; ["Offset of field: Outer4::inner"][::std::mem::offset_of!(Outer4, inner) - 16usize]; }; +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Inner2 { + pub a: ::std::os::raw::c_longlong, + pub b: ::std::os::raw::c_longlong, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of Inner2"][::std::mem::size_of::<Inner2>() - 16usize]; + ["Alignment of Inner2"][::std::mem::align_of::<Inner2>() - 8usize]; + ["Offset of field: Inner2::a"][::std::mem::offset_of!(Inner2, a) - 0usize]; + ["Offset of field: Inner2::b"][::std::mem::offset_of!(Inner2, b) - 8usize]; +}; +pub type AlignedInner2 = Inner2; +#[repr(C)] +#[repr(align(16))] +#[derive(Debug, Default, Copy, Clone)] +pub struct Outer5 { + pub before: ::std::os::raw::c_longlong, + pub __bindgen_padding_0: [u8; 8usize], + pub inner: [AlignedInner2; 1usize], +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of Outer5"][::std::mem::size_of::<Outer5>() - 32usize]; + ["Alignment of Outer5"][::std::mem::align_of::<Outer5>() - 16usize]; + ["Offset of field: Outer5::before"][::std::mem::offset_of!(Outer5, before) - 0usize]; + ["Offset of field: Outer5::inner"][::std::mem::offset_of!(Outer5, inner) - 16usize]; +}; +#[repr(C)] +#[repr(align(16))] +#[derive(Debug, Default)] +pub struct Outer6 { + pub before: ::std::os::raw::c_longlong, + pub __bindgen_padding_0: [u8; 8usize], + pub inner: __IncompleteArrayField<AlignedInner2>, + pub tail: ::std::os::raw::c_char, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of Outer6"][::std::mem::size_of::<Outer6>() - 32usize]; + ["Alignment of Outer6"][::std::mem::align_of::<Outer6>() - 16usize]; + ["Offset of field: Outer6::before"][::std::mem::offset_of!(Outer6, before) - 0usize]; + ["Offset of field: Outer6::inner"][::std::mem::offset_of!(Outer6, inner) - 16usize]; + ["Offset of field: Outer6::tail"][::std::mem::offset_of!(Outer6, tail) - 16usize]; +}; +pub const AlignedEnum_Value: AlignedEnum = 1; +pub type AlignedEnum = ::std::os::raw::c_uint; +#[repr(C)] +#[repr(align(16))] +#[derive(Debug, Copy, Clone)] +pub struct Outer7 { + pub before: ::std::os::raw::c_longlong, + pub __bindgen_padding_0: [u8; 8usize], + pub inner: AlignedEnum, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of Outer7"][::std::mem::size_of::<Outer7>() - 32usize]; + ["Alignment of Outer7"][::std::mem::align_of::<Outer7>() - 16usize]; + ["Offset of field: Outer7::before"][::std::mem::offset_of!(Outer7, before) - 0usize]; + ["Offset of field: Outer7::inner"][::std::mem::offset_of!(Outer7, inner) - 16usize]; +}; +impl Default for Outer7 { + fn default() -> Self { + let mut s = ::std::mem::MaybeUninit::<Self>::uninit(); + unsafe { + ::std::ptr::write_bytes(s.as_mut_ptr(), 0, 1); + s.assume_init() + } + } +}
diff --git a/bindgen-tests/tests/headers/issue-3406.h b/bindgen-tests/tests/headers/issue-3406.h index 1b87c4b..34ba644 100644 --- a/bindgen-tests/tests/headers/issue-3406.h +++ b/bindgen-tests/tests/headers/issue-3406.h
@@ -25,3 +25,24 @@ int before; NestedAlignedInt inner; }; + +struct Inner2 { long long a, b; }; +typedef struct Inner2 AlignedInner2 __attribute__((aligned(16))); + +struct Outer5 { + long long before; + AlignedInner2 inner[1]; +}; + +struct Outer6 { + long long before; + AlignedInner2 inner[0]; + char tail; +}; + +enum __attribute__((aligned(16))) AlignedEnum { Value = 1 }; + +struct Outer7 { + long long before; + enum AlignedEnum inner; +};
diff --git a/bindgen/ir/ty.rs b/bindgen/ir/ty.rs index 0519fd3..7eb81f5 100644 --- a/bindgen/ir/ty.rs +++ b/bindgen/ir/ty.rs
@@ -15,6 +15,7 @@ use crate::clang::{self, Cursor}; use crate::parse::{ParseError, ParseResult}; use std::borrow::Cow; +use std::cell::Cell; use std::io; pub use super::int::IntKind; @@ -28,12 +29,15 @@ pub(crate) struct Type { /// The name of the type, or None if it was an unnamed struct or union. name: Option<String>, - /// The layout of the type, if known. - layout: Option<Layout>, + /// This is, originally, the layout clang tells us about, if known. But we might clobber it + /// before returning it out if we are a typedef or so, see the `layout()` function. + layout: Cell<Option<Layout>>, /// The inner kind of the type kind: TypeKind, /// Whether this type is const-qualified. is_const: bool, + /// Avoids checking the nested type layout over and over. + checked_inner_layout: Cell<bool>, } /// The maximum number of items in an array for which Rust implements common @@ -61,9 +65,10 @@ ) -> Self { Type { name, - layout, + layout: Cell::new(layout), kind, is_const, + checked_inner_layout: Cell::new(false), } } @@ -216,36 +221,38 @@ } } + // HACK(emilio): Rust can't represent over-aligned typedefs / enums, so prefer the inner type's + // layout if available, to get struct layout correct at least... + fn check_inner_layout(&self, ctx: &BindgenContext) -> Option<Layout> { + debug_assert!(!self.checked_inner_layout.get()); + let has_layout = self.layout.get().is_some(); + match self.kind { + TypeKind::Enum(ref e) => ctx.resolve_type(e.repr()?).layout(ctx), + TypeKind::Alias(inner) | TypeKind::ResolvedTypeRef(inner) => { + ctx.resolve_type(inner).layout(ctx) + } + TypeKind::Comp(ref ci) if !has_layout => ci.layout(ctx), + TypeKind::Pointer(..) if !has_layout => Some(Layout::new( + ctx.target_pointer_size(), + ctx.target_pointer_size(), + )), + TypeKind::Array(inner, len) => { + let layout = ctx.resolve_type(inner).layout(ctx)?; + Some(Layout::new(len * layout.size, layout.align)) + } + _ => None, + } + } + /// What is the layout of this type? pub(crate) fn layout(&self, ctx: &BindgenContext) -> Option<Layout> { - if let TypeKind::Alias(inner) | TypeKind::ResolvedTypeRef(inner) = - self.kind - { - // HACK(emilio): Rust can't represent over-aligned typedefs, so prefer the inner type's - // layout if available, to get struct layout correct at least... - if let Some(l) = ctx.resolve_type(inner).layout(ctx) { - return Some(l); + if !self.checked_inner_layout.get() { + if let Some(inner) = self.check_inner_layout(ctx) { + self.layout.set(Some(inner)); } + self.checked_inner_layout.set(true); } - self.layout.or_else(|| { - match self.kind { - TypeKind::Comp(ref ci) => ci.layout(ctx), - TypeKind::Array(inner, 0) => Some(Layout::new( - 0, - ctx.resolve_type(inner).layout(ctx)?.align, - )), - // FIXME(emilio): This is a hack for anonymous union templates. - // Use the actual pointer size! - TypeKind::Pointer(..) => Some(Layout::new( - ctx.target_pointer_size(), - ctx.target_pointer_size(), - )), - TypeKind::ResolvedTypeRef(inner) => { - ctx.resolve_type(inner).layout(ctx) - } - _ => None, - } - }) + self.layout.get() } /// Whether this named type is an invalid C++ identifier. This is done to @@ -371,7 +378,7 @@ TypeKind::TemplateInstantiation(ref inst) => { inst.is_opaque(ctx, item) } - TypeKind::Comp(ref comp) => comp.is_opaque(ctx, &self.layout), + TypeKind::Comp(ref comp) => comp.is_opaque(ctx, &self.layout(ctx)), TypeKind::ResolvedTypeRef(to) => to.is_opaque(ctx, &()), _ => false, } @@ -415,7 +422,7 @@ where W: io::Write, { - if let Some(ref layout) = self.layout { + if let Some(ref layout) = self.layout(ctx) { writeln!( out, "<tr><td>size</td><td>{}</td></tr>