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

integer overflow in CTFE implementation of rotate_left #61406

Closed
RalfJung opened this issue May 31, 2019 · 4 comments · Fixed by #61454
Closed

integer overflow in CTFE implementation of rotate_left #61406

RalfJung opened this issue May 31, 2019 · 4 comments · Fixed by #61454
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

The following code ICEs when compiled with a rustc with debug-assertions enabled:

const X: i16 = 0i16.rotate_left(124);

The problem is in the implementation of rotate_left, specifically in this line:

let inv_shift_bits = (width_bits - raw_shift_bits) % width_bits;

The subtraction overflows here.

Cc @oli-obk

@Centril Centril added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 31, 2019
@nikic
Copy link
Contributor

nikic commented May 31, 2019

Fix should be:

// replace
let inv_shift_bits = (width_bits - raw_shift_bits) % width_bits;
// with
let inv_shift_bits = (width_bits - shift_bits) % width_bits;

@RalfJung RalfJung added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels May 31, 2019
@hgallagher1993
Copy link
Contributor

Ya I'll take this on 👍

@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label May 31, 2019
@RalfJung
Copy link
Member Author

RalfJung commented Jun 1, 2019

@hgallagher1993 awesome! @nikic above already pretty much gave the mentoring instructions, except for one thing: please also add a test case. Or rather, please extend the existing test case at src/test/ui/consts/const-int-rotate.rs.

Let me know if you need any more assistance.

@RalfJung
Copy link
Member Author

RalfJung commented Jun 2, 2019

@lzutao @hgallagher1993 looks like you are both on this now? Not sure how that happened...

bors added a commit that referenced this issue Jun 4, 2019
Fix integer overflow in rotate_left

Closes #61406
r? @RalfJung
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants