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

Unsigned global set to -1 #1040

Closed
e00E opened this issue Sep 26, 2017 · 11 comments
Closed

Unsigned global set to -1 #1040

e00E opened this issue Sep 26, 2017 · 11 comments

Comments

@e00E
Copy link

e00E commented Sep 26, 2017

Input C/C++ Header

unsigned long long g_107 = 18446744073709551615UL;

Bindgen Invocation

bindgen failing.h -o failing.rs

Actual Results

Bindgen generates

pub const g_107: ::std::os::raw::c_ulonglong = -1;

which when compiled with rustc --crate-type lib --test -o out failing2.rs gives

error[E0600]: cannot apply unary operator `-` to type `u64`
 --> failing2.rs:3:48
  |
3 | pub const g_107: ::std::os::raw::c_ulonglong = -1;
  |                                                ^^

error: aborting due to previous error

Expected Results

Not sure, but 18446744073709551615 does not look like -1 in the first place.
Found with csmith fuzzing.

@pepyakin
Copy link
Contributor

Not sure, but 18446744073709551615 does not look like -1 in the first place.
Found with csmith fuzzing.

In fact it is!
hex(18446744073709551615) = 0xFFFF_FFFF_FFFF_FFFF which is -1 in signed interpretation. See Two's complement.

This happens because bindgen doesn't check for a signedness of type, it just emits as signed var with int_expr.

@highfive
Copy link

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

@fitzgen
Copy link
Member

fitzgen commented Sep 26, 2017

Thanks for the bug report @e00E ! Did you use C-Smith and C-Reduce?

This should be easy enough to fix by making signed and unsigned versions of the codegen helpers that @pepyakin linked to.

@e00E
Copy link
Author

e00E commented Sep 26, 2017

I did use csmith but not C-reduce. I manually cut out everything except the part responsible for the error.

@Jlonghi
Copy link

Jlonghi commented Sep 28, 2017

Hi I'd like to start contributing and take this on as my first bug!
@highfive: assign me

@highfive
Copy link

Hey @josh0588! Thanks for your interest in working on this issue. It's now assigned to you!

@fitzgen
Copy link
Member

fitzgen commented Sep 28, 2017

@josh0588 if you run into any questions, don't hesitate to ask :)

@Jlonghi
Copy link

Jlonghi commented Sep 30, 2017

@fitzgen thanks, it appears there already is a signed and unsigned version of the codegen helpers 'uint_ expr' and 'int_expr? but the match construct only calls the int_expr

@fitzgen
Copy link
Member

fitzgen commented Oct 2, 2017

@josh0588 ah, I forgot about that! Looks like we jsut need to call it at the right time then :)

@fitzgen
Copy link
Member

fitzgen commented Oct 12, 2017

Hi @josh0588 -- still making progress here? Any questions I can answer for you?

fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Oct 13, 2017
We were not checking signed-ness and emitting the appropriate types.

Fixes rust-lang#1040
@fitzgen
Copy link
Member

fitzgen commented Oct 13, 2017

I'm taking this due to inactivity and because it is being hit by csmith all the time, preventing us from digging too much deeper.

@josh0588, if you're interested in hacking on another issue, let me know and I can help you find something :)

bors-servo pushed a commit that referenced this issue Oct 13, 2017
Handle unsigned integer constants greater than u32::MAX in codegen

We were not checking signed-ness and emitting the appropriate types.

Fixes #1040

r? @pepyakin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants