diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 2cbde73262..50de296e56 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1394,7 +1394,9 @@ impl CodeGenerator for CompInfo { let used_template_params = item.used_template_params(ctx); - let mut packed = self.packed(); + let ty = item.expect_type(); + let layout = ty.layout(ctx); + let mut packed = self.is_packed(ctx, &layout); // generate tuple struct if struct or union is a forward declaration, // skip for now if template parameters are needed. @@ -1431,7 +1433,7 @@ impl CodeGenerator for CompInfo { let is_opaque = item.is_opaque(ctx, &()); let mut fields = vec![]; let mut struct_layout = - StructLayoutTracker::new(ctx, self, &canonical_name); + StructLayoutTracker::new(ctx, self, ty, &canonical_name); if !is_opaque { if item.has_vtable_ptr(ctx) { @@ -1618,7 +1620,7 @@ impl CodeGenerator for CompInfo { if let Some(comment) = item.comment(ctx) { attributes.push(attributes::doc(comment)); } - if packed { + if packed && !is_opaque { attributes.push(attributes::repr_list(&["C", "packed"])); } else { attributes.push(attributes::repr("C")); diff --git a/src/codegen/struct_layout.rs b/src/codegen/struct_layout.rs index ca46947d1d..32b4896506 100644 --- a/src/codegen/struct_layout.rs +++ b/src/codegen/struct_layout.rs @@ -16,6 +16,7 @@ pub struct StructLayoutTracker<'a> { name: &'a str, ctx: &'a BindgenContext, comp: &'a CompInfo, + is_packed: bool, latest_offset: usize, padding_count: usize, latest_field_layout: Option, @@ -81,12 +82,14 @@ impl<'a> StructLayoutTracker<'a> { pub fn new( ctx: &'a BindgenContext, comp: &'a CompInfo, + ty: &'a Type, name: &'a str, ) -> Self { StructLayoutTracker { name: name, ctx: ctx, comp: comp, + is_packed: comp.is_packed(ctx, &ty.layout(ctx)), latest_offset: 0, padding_count: 0, latest_field_layout: None, @@ -180,7 +183,7 @@ impl<'a> StructLayoutTracker<'a> { let will_merge_with_bitfield = self.align_to_latest_field(field_layout); - let padding_layout = if self.comp.packed() { + let padding_layout = if self.is_packed { None } else { let padding_bytes = match field_offset { @@ -269,7 +272,7 @@ impl<'a> StructLayoutTracker<'a> { self.latest_field_layout.unwrap().align) || layout.align > mem::size_of::<*mut ()>()) { - let layout = if self.comp.packed() { + let layout = if self.is_packed { Layout::new(padding_bytes, 1) } else if self.last_field_was_bitfield || layout.align > mem::size_of::<*mut ()>() @@ -316,7 +319,7 @@ impl<'a> StructLayoutTracker<'a> { /// /// This is just to avoid doing the same check also in pad_field. fn align_to_latest_field(&mut self, new_field_layout: Layout) -> bool { - if self.comp.packed() { + if self.is_packed { // Skip to align fields when packed. return false; } diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 7265660c56..8ba3f04a00 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -974,8 +974,8 @@ pub struct CompInfo { /// size_t) has_non_type_template_params: bool, - /// Whether this struct layout is packed. - packed: bool, + /// Whether we saw `__attribute__((packed))` on or within this type. + packed_attr: bool, /// Used to know if we've found an opaque attribute that could cause us to /// generate a type with invalid layout. This is explicitly used to avoid us @@ -1007,7 +1007,7 @@ impl CompInfo { has_destructor: false, has_nonempty_base: false, has_non_type_template_params: false, - packed: false, + packed_attr: false, found_unknown_attr: false, is_forward_declaration: false, } @@ -1261,7 +1261,7 @@ impl CompInfo { } } CXCursor_PackedAttr => { - ci.packed = true; + ci.packed_attr = true; } CXCursor_TemplateTypeParameter => { let param = Item::type_param(None, cur, ctx) @@ -1439,8 +1439,31 @@ impl CompInfo { } /// Is this compound type packed? - pub fn packed(&self) -> bool { - self.packed + pub fn is_packed(&self, ctx: &BindgenContext, layout: &Option) -> bool { + if self.packed_attr { + return true + } + + // Even though `libclang` doesn't expose `#pragma packed(...)`, we can + // detect it through its effects. + if let Some(ref parent_layout) = *layout { + if self.fields().iter().any(|f| match *f { + Field::Bitfields(ref unit) => { + unit.layout().align > parent_layout.align + } + Field::DataMember(ref data) => { + let field_ty = ctx.resolve_type(data.ty()); + field_ty.layout(ctx).map_or(false, |field_ty_layout| { + field_ty_layout.align > parent_layout.align + }) + } + }) { + info!("Found a struct that was defined within `#pragma packed(...)`"); + return true; + } + } + + false } /// Returns true if compound type has been forward declared @@ -1504,8 +1527,8 @@ impl DotAttributes for CompInfo { )?; } - if self.packed { - writeln!(out, "packedtrue")?; + if self.packed_attr { + writeln!(out, "packed_attrtrue")?; } if self.is_forward_declaration { @@ -1528,15 +1551,17 @@ impl DotAttributes for CompInfo { } impl IsOpaque for CompInfo { - type Extra = (); + type Extra = Option; - fn is_opaque(&self, ctx: &BindgenContext, _: &()) -> bool { - // Early return to avoid extra computation + fn is_opaque(&self, ctx: &BindgenContext, layout: &Option) -> bool { if self.has_non_type_template_params { return true } - self.fields().iter().any(|f| match *f { + // Bitfields with a width that is larger than their unit's width have + // some strange things going on, and the best we can do is make the + // whole struct opaque. + if self.fields().iter().any(|f| match *f { Field::DataMember(_) => { false }, @@ -1548,7 +1573,23 @@ impl IsOpaque for CompInfo { bf.width() / 8 > bitfield_layout.size as u32 }) } - }) + }) { + return true; + } + + // We don't have `#[repr(packed = "N")]` in Rust yet, so the best we can + // do is make this struct opaque. + // + // See https://github.com/rust-lang-nursery/rust-bindgen/issues/537 and + // https://github.com/rust-lang/rust/issues/33158 + if self.is_packed(ctx, layout) && layout.map_or(false, |l| l.align > 1) { + warn!("Found a type that is both packed and aligned to greater than \ + 1; Rust doesn't have `#[repr(packed = \"N\")]` yet, so we \ + are treating it as opaque"); + return true; + } + + false } } diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 9a51c2b373..e14860c26b 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -378,7 +378,7 @@ impl IsOpaque for Type { TypeKind::TemplateInstantiation(ref inst) => { inst.is_opaque(ctx, item) } - TypeKind::Comp(ref comp) => comp.is_opaque(ctx, &()), + TypeKind::Comp(ref comp) => comp.is_opaque(ctx, &self.layout), TypeKind::ResolvedTypeRef(to) => to.is_opaque(ctx, &()), _ => false, } 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 1f738e869b..9fb429af12 100644 --- a/tests/expectations/tests/divide-by-zero-in-struct-layout.rs +++ b/tests/expectations/tests/divide-by-zero-in-struct-layout.rs @@ -30,11 +30,12 @@ impl WithBitfieldAndAttrPacked { 0 } } -#[repr(C)] +#[repr(C, packed)] #[derive(Debug, Default, Copy, Clone)] pub struct WithBitfieldAndPacked { pub _bitfield_1: [u8; 0usize], pub a: ::std::os::raw::c_uint, + pub __bindgen_padding_0: u8, } impl WithBitfieldAndPacked { #[inline] diff --git a/tests/expectations/tests/layout.rs b/tests/expectations/tests/layout.rs index eedf1e2b62..2790446c1a 100644 --- a/tests/expectations/tests/layout.rs +++ b/tests/expectations/tests/layout.rs @@ -5,49 +5,8 @@ #[repr(C)] -#[derive(Default)] -pub struct __IncompleteArrayField(::std::marker::PhantomData); -impl __IncompleteArrayField { - #[inline] - pub fn new() -> Self { - __IncompleteArrayField(::std::marker::PhantomData) - } - #[inline] - pub unsafe fn as_ptr(&self) -> *const T { - ::std::mem::transmute(self) - } - #[inline] - pub unsafe fn as_mut_ptr(&mut self) -> *mut T { - ::std::mem::transmute(self) - } - #[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 ::std::fmt::Debug for __IncompleteArrayField { - fn fmt(&self, fmt: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { - fmt.write_str("__IncompleteArrayField") - } -} -impl ::std::clone::Clone for __IncompleteArrayField { - #[inline] - fn clone(&self) -> Self { - Self::new() - } -} -impl ::std::marker::Copy for __IncompleteArrayField {} -#[repr(C, packed)] -#[derive(Debug, Default)] pub struct header { - pub proto: ::std::os::raw::c_char, - pub size: ::std::os::raw::c_uint, - pub data: __IncompleteArrayField<::std::os::raw::c_uchar>, - pub __bindgen_padding_0: [u8; 11usize], + pub _bindgen_opaque_blob: [u8; 16usize], } #[test] fn bindgen_test_layout_header() { @@ -57,3 +16,8 @@ fn bindgen_test_layout_header() { concat!("Size of: ", stringify!(header)) ); } +impl Default for header { + fn default() -> Self { + unsafe { ::std::mem::zeroed() } + } +} diff --git a/tests/headers/issue-537.h b/tests/headers/issue-537.h new file mode 100644 index 0000000000..9a1072a7b6 --- /dev/null +++ b/tests/headers/issue-537.h @@ -0,0 +1,35 @@ +/// This should not be opaque; we can see the attributes and can pack the +/// struct. +struct AlignedToOne { + int i; +} __attribute__ ((packed,aligned(1))); + +/// This should be opaque because although we can see the attributes, Rust +/// doesn't have `#[repr(packed = "N")]` yet. +struct AlignedToTwo { + int i; +} __attribute__ ((packed,aligned(2))); + +#pragma pack(1) + +/// This should not be opaque because although `libclang` doesn't give us the +/// `#pragma pack(1)`, we can detect that alignment is 1 and add +/// `#[repr(packed)]` to the struct ourselves. +struct PackedToOne { + int x; + int y; +}; + +#pragma pack() + +#pragma pack(2) + +/// In this case, even if we can detect the weird alignment triggered by +/// `#pragma pack(2)`, we can't do anything about it because Rust doesn't have +/// `#[repr(packed = "N")]`. Therefore, we must make it opaque. +struct PackedToTwo { + int x; + int y; +}; + +#pragma pack()