Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make bitfield unit allocation fallible #1141

Merged
merged 1 commit into from
Nov 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,13 @@ impl FieldMethods for RawField {

/// Convert the given ordered set of raw fields into a list of either plain data
/// members, and/or bitfield units containing multiple bitfields.
///
/// If we do not have the layout for a bitfield's type, then we can't reliably
/// compute its allocation unit. In such cases, we return an error.
fn raw_fields_to_fields_and_bitfield_units<I>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add to the doc comment when do we return errors?

ctx: &BindgenContext,
raw_fields: I,
) -> Vec<Field>
) -> Result<Vec<Field>, ()>
where
I: IntoIterator<Item = RawField>,
{
Expand Down Expand Up @@ -503,15 +506,15 @@ where
&mut bitfield_unit_count,
&mut fields,
bitfields,
);
)?;
}

assert!(
raw_fields.next().is_none(),
"The above loop should consume all items in `raw_fields`"
);

fields
Ok(fields)
}

/// Given a set of contiguous raw bitfields, group and allocate them into
Expand All @@ -521,7 +524,8 @@ fn bitfields_to_allocation_units<E, I>(
bitfield_unit_count: &mut usize,
fields: &mut E,
raw_bitfields: I,
) where
) -> Result<(), ()>
where
E: Extend<Field>,
I: IntoIterator<Item = RawField>,
{
Expand Down Expand Up @@ -572,7 +576,7 @@ fn bitfields_to_allocation_units<E, I>(
let bitfield_width = bitfield.bitfield_width().unwrap() as usize;
let bitfield_layout = ctx.resolve_type(bitfield.ty())
.layout(ctx)
.expect("Bitfield without layout? Gah!");
.ok_or(())?;
let bitfield_size = bitfield_layout.size;
let bitfield_align = bitfield_layout.align;

Expand Down Expand Up @@ -649,6 +653,8 @@ fn bitfields_to_allocation_units<E, I>(
bitfields_in_unit,
);
}

Ok(())
}

/// A compound structure's fields are initially raw, and have bitfields that
Expand All @@ -662,6 +668,7 @@ fn bitfields_to_allocation_units<E, I>(
enum CompFields {
BeforeComputingBitfieldUnits(Vec<RawField>),
AfterComputingBitfieldUnits(Vec<Field>),
ErrorComputingBitfieldUnits,
}

impl Default for CompFields {
Expand All @@ -676,7 +683,7 @@ impl CompFields {
CompFields::BeforeComputingBitfieldUnits(ref mut raws) => {
raws.push(raw);
}
CompFields::AfterComputingBitfieldUnits(_) => {
_ => {
panic!(
"Must not append new fields after computing bitfield allocation units"
);
Expand All @@ -689,22 +696,36 @@ impl CompFields {
CompFields::BeforeComputingBitfieldUnits(ref mut raws) => {
mem::replace(raws, vec![])
}
CompFields::AfterComputingBitfieldUnits(_) => {
_ => {
panic!("Already computed bitfield units");
}
};

let fields_and_units =
let result =
raw_fields_to_fields_and_bitfield_units(ctx, raws);
mem::replace(
self,
CompFields::AfterComputingBitfieldUnits(fields_and_units),
);

match result {
Ok(fields_and_units) => {
mem::replace(
self,
CompFields::AfterComputingBitfieldUnits(fields_and_units));
}
Err(()) => {
mem::replace(
self,
CompFields::ErrorComputingBitfieldUnits
);
}
}
}

fn deanonymize_fields(&mut self, ctx: &BindgenContext, methods: &[Method]) {
let fields = match *self {
CompFields::AfterComputingBitfieldUnits(ref mut fields) => fields,
CompFields::ErrorComputingBitfieldUnits => {
// Nothing to do here.
return;
}
CompFields::BeforeComputingBitfieldUnits(_) => {
panic!("Not yet computed bitfield units.");
}
Expand Down Expand Up @@ -787,6 +808,7 @@ impl Trace for CompFields {
T: Tracer,
{
match *self {
CompFields::ErrorComputingBitfieldUnits => {}
CompFields::BeforeComputingBitfieldUnits(ref fields) => {
for f in fields {
tracer.visit_kind(f.ty().into(), EdgeKind::Field);
Expand Down Expand Up @@ -1046,6 +1068,7 @@ impl CompInfo {
/// Get this type's set of fields.
pub fn fields(&self) -> &[Field] {
match self.fields {
CompFields::ErrorComputingBitfieldUnits => &[],
CompFields::AfterComputingBitfieldUnits(ref fields) => fields,
CompFields::BeforeComputingBitfieldUnits(_) => {
panic!("Should always have computed bitfield units first");
Expand Down Expand Up @@ -1558,6 +1581,14 @@ impl IsOpaque for CompInfo {
return true
}

// When we do not have the layout for a bitfield's type (for example, it
// is a type parameter), then we can't compute bitfield units. We are
// left with no choice but to make the whole struct opaque, or else we
// might generate structs with incorrect sizes and alignments.
if let CompFields::ErrorComputingBitfieldUnits = self.fields {
return true;
}

// 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.
Expand Down
15 changes: 15 additions & 0 deletions tests/expectations/tests/templatized-bitfield.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]



/// We don't get a layout for this bitfield, since we don't know what `T` will
/// be, so we cannot allocate bitfield units. The best thing we can do is make
/// the struct opaque.
#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct TemplatizedBitfield {
pub _address: u8,
}
7 changes: 7 additions & 0 deletions tests/headers/templatized-bitfield.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// We don't get a layout for this bitfield, since we don't know what `T` will
/// be, so we cannot allocate bitfield units. The best thing we can do is make
/// the struct opaque.
template <class T>
class TemplatizedBitfield {
T t : 6;
};