summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorNick Fitzgerald <fitzgen@gmail.com>2017-11-01 14:36:08 -0700
committerNick Fitzgerald <fitzgen@gmail.com>2017-11-02 09:51:09 -0700
commit79dbe8b8062076f8ac3f76c0eaf4846dcb8c4638 (patch)
treec7fbf6bbb0134e13a1ebaea38c342f02daaaa69c /src
parentf059edee4a6b165323623af0c993d3c3889ed2c1 (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.rs8
-rw-r--r--src/codegen/struct_layout.rs9
-rw-r--r--src/ir/comp.rs67
-rw-r--r--src/ir/ty.rs2
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,
}