struct_layout: Fix field offset computation for packed(n) structs. This can cause unnecessary padding to be computed otherwise at the end of the struct. With repr(packed(n)), a field can have padding to adjacent fields as long as its alignment is less than n. So reuse the code we have to align to a field layout, aligning to the struct layout instead. Fixes #1934
diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 15aea22..0d93c49 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs
@@ -1650,7 +1650,7 @@ let ty = item.expect_type(); let layout = ty.layout(ctx); - let mut packed = self.is_packed(ctx, &layout); + let mut packed = self.is_packed(ctx, layout.as_ref()); let canonical_name = item.canonical_name(ctx); let canonical_ident = ctx.rust_ident(&canonical_name);
diff --git a/src/codegen/struct_layout.rs b/src/codegen/struct_layout.rs index 54dfd86..4536e88 100644 --- a/src/codegen/struct_layout.rs +++ b/src/codegen/struct_layout.rs
@@ -18,6 +18,7 @@ ctx: &'a BindgenContext, comp: &'a CompInfo, is_packed: bool, + known_type_layout: Option<Layout>, latest_offset: usize, padding_count: usize, latest_field_layout: Option<Layout>, @@ -86,11 +87,14 @@ ty: &'a Type, name: &'a str, ) -> Self { + let known_type_layout = ty.layout(ctx); + let is_packed = comp.is_packed(ctx, known_type_layout.as_ref()); StructLayoutTracker { name, ctx, comp, - is_packed: comp.is_packed(ctx, &ty.layout(ctx)), + is_packed, + known_type_layout, latest_offset: 0, padding_count: 0, latest_field_layout: None, @@ -180,23 +184,32 @@ let will_merge_with_bitfield = self.align_to_latest_field(field_layout); + let padding_bytes = match field_offset { + Some(offset) if offset / 8 > self.latest_offset => { + offset / 8 - self.latest_offset + } + _ => { + if will_merge_with_bitfield || field_layout.align == 0 { + 0 + } else if !self.is_packed { + self.padding_bytes(field_layout) + } else if let Some(l) = self.known_type_layout { + self.padding_bytes(l) + } else { + 0 + } + } + }; + + self.latest_offset += padding_bytes; + let padding_layout = if self.is_packed { None } else { - let padding_bytes = match field_offset { - Some(offset) if offset / 8 > self.latest_offset => { - offset / 8 - self.latest_offset - } - _ if will_merge_with_bitfield || field_layout.align == 0 => 0, - _ => self.padding_bytes(field_layout), - }; - // Otherwise the padding is useless. let need_padding = padding_bytes >= field_layout.align || field_layout.align > MAX_GUARANTEED_ALIGN; - self.latest_offset += padding_bytes; - debug!( "Offset: <padding>: {} -> {}", self.latest_offset - padding_bytes,
diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 22c124f..1d257d2 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs
@@ -1576,7 +1576,7 @@ pub fn is_packed( &self, ctx: &BindgenContext, - layout: &Option<Layout>, + layout: Option<&Layout>, ) -> bool { if self.packed_attr { return true; @@ -1584,7 +1584,7 @@ // Even though `libclang` doesn't expose `#pragma packed(...)`, we can // detect it through its effects. - if let Some(ref parent_layout) = *layout { + if let Some(parent_layout) = layout { if self.fields().iter().any(|f| match *f { Field::Bitfields(ref unit) => { unit.layout().align > parent_layout.align @@ -1739,7 +1739,7 @@ // // See https://github.com/rust-lang/rust-bindgen/issues/537 and // https://github.com/rust-lang/rust/issues/33158 - if self.is_packed(ctx, layout) && + if self.is_packed(ctx, layout.as_ref()) && layout.map_or(false, |l| l.align > 1) { warn!("Found a type that is both packed and aligned to greater than \
diff --git a/tests/expectations/tests/divide-by-zero-in-struct-layout.rs b/tests/expectations/tests/divide-by-zero-in-struct-layout.rs index 1366548..1484719 100644 --- a/tests/expectations/tests/divide-by-zero-in-struct-layout.rs +++ b/tests/expectations/tests/divide-by-zero-in-struct-layout.rs
@@ -130,7 +130,6 @@ pub struct WithBitfieldAndPacked { pub _bitfield_1: __BindgenBitfieldUnit<[u8; 0usize], u8>, pub a: ::std::os::raw::c_uint, - pub __bindgen_padding_0: u8, } impl WithBitfieldAndPacked { #[inline]
diff --git a/tests/expectations/tests/packed-n-with-padding.rs b/tests/expectations/tests/packed-n-with-padding.rs new file mode 100644 index 0000000..13cb030 --- /dev/null +++ b/tests/expectations/tests/packed-n-with-padding.rs
@@ -0,0 +1,48 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C, packed(2))] +#[derive(Debug, Default, Copy, Clone)] +pub struct Packed { + pub a: ::std::os::raw::c_char, + pub b: ::std::os::raw::c_short, + pub c: ::std::os::raw::c_char, + pub d: ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_Packed() { + assert_eq!( + ::std::mem::size_of::<Packed>(), + 10usize, + concat!("Size of: ", stringify!(Packed)) + ); + assert_eq!( + ::std::mem::align_of::<Packed>(), + 2usize, + concat!("Alignment of ", stringify!(Packed)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::<Packed>())).a as *const _ as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Packed), "::", stringify!(a)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::<Packed>())).b as *const _ as usize }, + 2usize, + concat!("Offset of field: ", stringify!(Packed), "::", stringify!(b)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::<Packed>())).c as *const _ as usize }, + 4usize, + concat!("Offset of field: ", stringify!(Packed), "::", stringify!(c)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::<Packed>())).d as *const _ as usize }, + 6usize, + concat!("Offset of field: ", stringify!(Packed), "::", stringify!(d)) + ); +}
diff --git a/tests/headers/packed-n-with-padding.h b/tests/headers/packed-n-with-padding.h new file mode 100644 index 0000000..8a6233b --- /dev/null +++ b/tests/headers/packed-n-with-padding.h
@@ -0,0 +1,8 @@ +#pragma pack(push, 2) +struct Packed { + char a; + short b; + char c; + int d; +}; +#pragma pack(pop)