diff options
author | Nick Fitzgerald <fitzgen@gmail.com> | 2017-11-01 14:36:08 -0700 |
---|---|---|
committer | Nick Fitzgerald <fitzgen@gmail.com> | 2017-11-02 09:51:09 -0700 |
commit | 79dbe8b8062076f8ac3f76c0eaf4846dcb8c4638 (patch) | |
tree | c7fbf6bbb0134e13a1ebaea38c342f02daaaa69c /src | |
parent | f059edee4a6b165323623af0c993d3c3889ed2c1 (diff) |
Detect `#pragma pack(...)` and make `pack(n)` where `n > 1` opaque
This is a bandaid for #537. It does *not* fix the underlying issue, which
requires `#[repr(packed = "N")]` support in Rust. However, it does make sure
that we don't generate type definitions with the wrong layout, or fail our
generated layout tests.
Diffstat (limited to 'src')
-rw-r--r-- | src/codegen/mod.rs | 8 | ||||
-rw-r--r-- | src/codegen/struct_layout.rs | 9 | ||||
-rw-r--r-- | src/ir/comp.rs | 67 | ||||
-rw-r--r-- | src/ir/ty.rs | 2 |
4 files changed, 66 insertions, 20 deletions
diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 2cbde732..50de296e 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 ca46947d..32b48965 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<Layout>, @@ -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 7265660c..8ba3f04a 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<Layout>) -> 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, "<tr><td>packed</td><td>true</td></tr>")?; + if self.packed_attr { + writeln!(out, "<tr><td>packed_attr</td><td>true</td></tr>")?; } if self.is_forward_declaration { @@ -1528,15 +1551,17 @@ impl DotAttributes for CompInfo { } impl IsOpaque for CompInfo { - type Extra = (); + type Extra = Option<Layout>; - fn is_opaque(&self, ctx: &BindgenContext, _: &()) -> bool { - // Early return to avoid extra computation + fn is_opaque(&self, ctx: &BindgenContext, layout: &Option<Layout>) -> 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 9a51c2b3..e14860c2 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, } |