Skip to content

ir: Avoid generating out-of-range values in constants. #276

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

Merged
merged 1 commit into from
Nov 17, 2016
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
6 changes: 3 additions & 3 deletions libbindgen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ impl CodeGenerator for Item {
result: &mut CodegenResult,
_extra: &()) {
if self.is_hidden(ctx) || result.seen(self.id()) {
debug!("<Item as CodeGenerator>::codegen: Ignoring hidden or seen: self = {:?}", self);
debug!("<Item as CodeGenerator>::codegen: Ignoring hidden or seen: \
self = {:?}", self);
return;
}

Expand Down Expand Up @@ -328,8 +329,7 @@ impl CodeGenerator for Var {
.build(ty)
}
VarType::Int(val) => {
const_item.build(helpers::ast_ty::int_expr(val))
.build(ty)
const_item.build(helpers::ast_ty::int_expr(val)).build(ty)
}
VarType::String(ref bytes) => {
// Account the trailing zero.
Expand Down
5 changes: 5 additions & 0 deletions libbindgen/src/ir/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,9 @@ impl IntKind {
Custom { is_signed, .. } => is_signed,
}
}

/// Whether this type's signedness matches the value.
pub fn signedness_matches(&self, val: i64) -> bool {
val >= 0 || self.is_signed()
}
}
3 changes: 2 additions & 1 deletion libbindgen/src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1032,7 +1032,8 @@ impl ClangItemParser for Item {
// It's harmless, but if we restrict that, then
// tests/headers/nsStyleAutoArray.hpp crashes.
if let Err(ParseError::Recurse) = result {
warn!("Unknown type, assuming named template type: id = {:?}; spelling = {}",
warn!("Unknown type, assuming named template type: \
id = {:?}; spelling = {}",
id,
ty.spelling());
Ok(Self::named_type_with_id(id,
Expand Down
4 changes: 3 additions & 1 deletion libbindgen/src/ir/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,9 @@ impl TypeCollector for Type {
}
// FIXME: Pending types!
ref other @ _ => {
debug!("<Type as TypeCollector>::collect_types: Ignoring: {:?}", other);
debug!("<Type as TypeCollector>::collect_types: Ignoring: \
{:?}",
other);
}
}
}
Expand Down
35 changes: 21 additions & 14 deletions libbindgen/src/ir/var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use super::ty::{FloatKind, TypeKind};
/// The type for a constant variable.
#[derive(Debug)]
pub enum VarType {
/// An boolean.
/// A boolean.
Bool(bool),
/// An integer.
Int(i64),
Expand Down Expand Up @@ -196,26 +196,33 @@ impl ClangSubItemParser for Var {
let canonical_ty = ctx.safe_resolve_type(ty)
.and_then(|t| t.safe_canonical_type(ctx));

let is_bool = canonical_ty.map_or(false, |t| t.is_bool());
let is_integer = canonical_ty.map_or(false, |t| t.is_integer());
let is_float = canonical_ty.map_or(false, |t| t.is_float());

// TODO: We could handle `char` more gracefully.
// TODO: Strings, though the lookup is a bit more hard (we need
// to look at the canonical type of the pointee too, and check
// is char, u8, or i8 I guess).
let value = if is_bool {
cursor.evaluate().as_int()
.map(|val| VarType::Bool(val != 0))
} else if is_integer {
cursor.evaluate()
.as_int()
.map(|val| val as i64)
.or_else(|| {
let tu = ctx.translation_unit();
get_integer_literal_from_cursor(&cursor, tu)
})
.map(VarType::Int)
let value = if is_integer {
let kind = match *canonical_ty.unwrap().kind() {
TypeKind::Int(kind) => kind,
_ => unreachable!(),
};

let mut val =
cursor.evaluate().as_int().map(|val| val as i64);
if val.is_none() || !kind.signedness_matches(val.unwrap()) {
let tu = ctx.translation_unit();
val = get_integer_literal_from_cursor(&cursor, tu);
}
Copy link
Member

Choose a reason for hiding this comment

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

I slightly preferred all this with the slightly more concise method chaining, but either way works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it was a bit trickier to write it that way, because we may be overriding a Some(value out of range) with None, so I preferred to be more explicit.


val.map(|val| {
if kind == IntKind::Bool {
VarType::Bool(val != 0)
} else {
VarType::Int(val)
}
})
} else if is_float {
cursor.evaluate()
.as_double()
Expand Down
3 changes: 3 additions & 0 deletions libbindgen/tests/expectations/tests/constant-evaluate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ pub const bar: _bindgen_ty_1 = _bindgen_ty_1::bar;
#[repr(u32)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum _bindgen_ty_1 { foo = 4, bar = 8, }
pub type EasyToOverflow = ::std::os::raw::c_ulonglong;
pub const k: EasyToOverflow = 2147483648;
pub const k_expr: EasyToOverflow = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be desirable. Build would succeed, but existence of this may lead to weird bug. This should either be removed, or be linked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're getting zero from clang in this case unfortunately, so there's little I can do :(

pub const BAZ: ::std::os::raw::c_longlong = 24;
pub const fuzz: f64 = 51.;
pub const BAZZ: ::std::os::raw::c_char = 53;
Expand Down
5 changes: 5 additions & 0 deletions libbindgen/tests/headers/constant-evaluate.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ enum {
bar = 8,
};

typedef unsigned long long EasyToOverflow;
const EasyToOverflow k = 0x80000000;

const EasyToOverflow k_expr = 1ULL << 60;

const long long BAZ = (1 << foo) | bar;
const double fuzz = (1 + 50.0f);
const char BAZZ = '5';
Expand Down