diff --git a/bindgen-tests/tests/expectations/tests/packed_embeds_aligned.rs b/bindgen-tests/tests/expectations/tests/packed_embeds_aligned.rs new file mode 100644 index 0000000000..5d09c56d78 --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/packed_embeds_aligned.rs @@ -0,0 +1,126 @@ +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] +#[repr(C)] +#[repr(align(2))] +#[derive(Debug, Default, Copy, Clone)] +pub struct inner1 { + pub a: ::std::os::raw::c_char, + pub b: ::std::os::raw::c_char, +} +#[test] +fn bindgen_test_layout_inner1() { + const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 2usize, + concat!("Size of: ", stringify!(inner1)), + ); + assert_eq!( + ::std::mem::align_of::(), + 2usize, + concat!("Alignment of ", stringify!(inner1)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).a) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(inner1), "::", stringify!(a)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).b) as usize - ptr as usize }, + 1usize, + concat!("Offset of field: ", stringify!(inner1), "::", stringify!(b)), + ); +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct inner2 { + pub a: inner1, + pub b: inner1, +} +#[test] +fn bindgen_test_layout_inner2() { + const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 4usize, + concat!("Size of: ", stringify!(inner2)), + ); + assert_eq!( + ::std::mem::align_of::(), + 2usize, + concat!("Alignment of ", stringify!(inner2)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).a) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(inner2), "::", stringify!(a)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).b) as usize - ptr as usize }, + 2usize, + concat!("Offset of field: ", stringify!(inner2), "::", stringify!(b)), + ); +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct outer1 { + pub a: ::std::os::raw::c_short, + pub b: inner1, +} +#[test] +fn bindgen_test_layout_outer1() { + const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 4usize, + concat!("Size of: ", stringify!(outer1)), + ); + assert_eq!( + ::std::mem::align_of::(), + 2usize, + concat!("Alignment of ", stringify!(outer1)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).a) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(outer1), "::", stringify!(a)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).b) as usize - ptr as usize }, + 2usize, + concat!("Offset of field: ", stringify!(outer1), "::", stringify!(b)), + ); +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct outer2 { + pub a: ::std::os::raw::c_short, + pub b: inner2, +} +#[test] +fn bindgen_test_layout_outer2() { + const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 6usize, + concat!("Size of: ", stringify!(outer2)), + ); + assert_eq!( + ::std::mem::align_of::(), + 2usize, + concat!("Alignment of ", stringify!(outer2)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).a) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(outer2), "::", stringify!(a)), + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).b) as usize - ptr as usize }, + 2usize, + concat!("Offset of field: ", stringify!(outer2), "::", stringify!(b)), + ); +} diff --git a/bindgen-tests/tests/headers/packed_embeds_aligned.h b/bindgen-tests/tests/headers/packed_embeds_aligned.h new file mode 100644 index 0000000000..348a97cc6d --- /dev/null +++ b/bindgen-tests/tests/headers/packed_embeds_aligned.h @@ -0,0 +1,19 @@ +struct inner1 { + char a; + char b; +} __attribute__((aligned(2))); + +struct inner2 { + struct inner1 a; + struct inner1 b; +} __attribute__((aligned(2))); + +struct outer1 { + short a; + struct inner1 b; +} __attribute__((packed, aligned(2))); + +struct outer2 { + short a; + struct inner2 b; +} __attribute__((packed, aligned(2))); diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index b126a847b4..3b9572b6a4 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -2202,8 +2202,13 @@ impl CodeGenerator for CompInfo { // "packed" attr is redundant, and do not include it if so. if packed && !is_opaque && - !(explicit_align.is_some() && - self.already_packed(ctx).unwrap_or(false)) + !((explicit_align.is_some() || self.may_contain_aligned_type()) && + self.already_packed( + ctx, + struct_layout.max_field_align(), + layout, + ) + .unwrap_or(false)) { let n = layout.map_or(1, |l| l.align); assert!(ctx.options().rust_features().repr_packed_n || n == 1); diff --git a/bindgen/codegen/struct_layout.rs b/bindgen/codegen/struct_layout.rs index 7349669871..780afe10b8 100644 --- a/bindgen/codegen/struct_layout.rs +++ b/bindgen/codegen/struct_layout.rs @@ -121,6 +121,10 @@ impl<'a> StructLayoutTracker<'a> { self.is_rust_union } + pub(crate) fn max_field_align(&self) -> usize { + self.max_field_align + } + pub(crate) fn saw_vtable(&mut self) { debug!("saw vtable for {}", self.name); diff --git a/bindgen/ir/comp.rs b/bindgen/ir/comp.rs index bd4d016261..cd77fcff01 100644 --- a/bindgen/ir/comp.rs +++ b/bindgen/ir/comp.rs @@ -10,7 +10,7 @@ use super::item::{IsOpaque, Item}; use super::layout::Layout; use super::template::TemplateParameters; use super::traversal::{EdgeKind, Trace, Tracer}; -use super::ty::RUST_DERIVE_IN_ARRAY_LIMIT; +use super::ty::{TypeKind, RUST_DERIVE_IN_ARRAY_LIMIT}; use crate::clang; use crate::codegen::struct_layout::{align_to, bytes_from_bits_pow2}; use crate::ir::derive::CanDeriveCopy; @@ -1049,6 +1049,10 @@ pub(crate) struct CompInfo { /// Used to indicate when a struct has been forward declared. Usually used /// in headers so that APIs can't modify them directly. is_forward_declaration: bool, + + /// Heuristic for whether this type is likely to contain any types with an + /// explicit alignment attribute + may_contain_aligned_type: bool, } impl CompInfo { @@ -1072,6 +1076,7 @@ impl CompInfo { packed_attr: false, found_unknown_attr: false, is_forward_declaration: false, + may_contain_aligned_type: false, } } @@ -1235,6 +1240,7 @@ impl CompInfo { ty: &clang::Type, location: Option, ctx: &mut BindgenContext, + outer_layout: Option<&Layout>, ) -> Result { use clang_sys::*; assert!( @@ -1252,6 +1258,7 @@ impl CompInfo { } let kind = kind?; + let mut max_field_align: Option = None; debug!("CompInfo::from_ty({:?}, {:?})", kind, cursor); @@ -1330,6 +1337,29 @@ impl CompInfo { ctx, ); + let field_comp_type = ctx.resolve_type(field_type); + + match field_comp_type.kind() { + TypeKind::ResolvedTypeRef(inner_type) => { + let inner_type = ctx.resolve_type(*inner_type); + if let Some(inner_type) = inner_type.as_comp() { + if inner_type.may_contain_aligned_type { + ci.may_contain_aligned_type = true; + } + } + } + _ => {} + } + + if let Ok(field_layout) = + cur.cur_type().fallible_layout(ctx) + { + max_field_align = Some(cmp::max( + max_field_align.unwrap_or(0), + field_layout.align, + )); + } + let comment = cur.raw_comment(); let annotations = Annotations::new(&cur); let name = cur.spelling(); @@ -1418,6 +1448,14 @@ impl CompInfo { Some((inner, ty, public, offset)); } } + if let Ok(field_layout) = + cur.cur_type().fallible_layout(ctx) + { + max_field_align = Some(cmp::max( + max_field_align.unwrap_or(0), + field_layout.align, + )); + } } CXCursor_PackedAttr => { ci.packed_attr = true; @@ -1566,6 +1604,15 @@ impl CompInfo { CXChildVisit_Continue }); + if let Some(outer_layout) = outer_layout { + if let Some(max_field_align) = max_field_align { + if outer_layout.align > max_field_align || max_field_align >= 16 + { + ci.may_contain_aligned_type = true; + } + } + } + if let Some((ty, _, public, offset)) = maybe_anonymous_struct_field { let field = RawField::new(None, ty, None, None, None, public, offset); @@ -1646,7 +1693,12 @@ impl CompInfo { /// "packed" attribute without changing the layout. /// This is useful for types that need an "align(N)" attribute since rustc won't compile /// structs that have both of those attributes. - pub(crate) fn already_packed(&self, ctx: &BindgenContext) -> Option { + pub(crate) fn already_packed( + &self, + ctx: &BindgenContext, + max_field_align: usize, + outer_layout: Option, + ) -> Option { let mut total_size: usize = 0; for field in self.fields().iter() { @@ -1659,6 +1711,18 @@ impl CompInfo { total_size += layout.size; } + if let Some(outer_layout) = outer_layout { + if max_field_align != 0 && + outer_layout.size % max_field_align != 0 + { + return Some(false); + } + + if outer_layout.align < max_field_align { + return Some(false); + } + } + Some(true) } @@ -1667,6 +1731,16 @@ impl CompInfo { self.is_forward_declaration } + /// Returns true if compound type might have a child type with an explicit alignment attribute. + /// + /// Note, this is not 100% accurate because it is based only on Clang layout information, but + /// the final determination to place an alignment attribute happens during code generation and + /// requires information not available at the time this is set. However, this is good enough + /// for most cases. + pub(crate) fn may_contain_aligned_type(&self) -> bool { + self.may_contain_aligned_type + } + /// Compute this compound structure's bitfield allocation units. pub(crate) fn compute_bitfield_units( &mut self, diff --git a/bindgen/ir/ty.rs b/bindgen/ir/ty.rs index 2a24dd0291..27aedf40a3 100644 --- a/bindgen/ir/ty.rs +++ b/bindgen/ir/ty.rs @@ -52,6 +52,15 @@ impl Type { } } + /// Get the underlying `CompInfo` for this type as an immutable reference, or + /// `None` if this is some other kind of type. + pub(crate) fn as_comp(&self) -> Option<&CompInfo> { + match self.kind { + TypeKind::Comp(ref ci) => Some(ci), + _ => None, + } + } + /// Construct a new `Type`. pub(crate) fn new( name: Option, @@ -806,6 +815,7 @@ impl Type { ty, Some(location), ctx, + layout.as_ref(), ) .expect("C'mon"); TypeKind::Comp(complex) @@ -868,6 +878,7 @@ impl Type { ty, Some(location), ctx, + layout.as_ref(), ); match complex { Ok(complex) => TypeKind::Comp(complex), @@ -1134,6 +1145,7 @@ impl Type { ty, Some(location), ctx, + layout.as_ref(), ) .expect("Not a complex type?");