-
Notifications
You must be signed in to change notification settings - Fork 208
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
regression from v0.18.0 to v.0.18.0+409 with regards to wrapping_mul #1024
Comments
At first, I thought this issue was related to the cast expression not being identified as unsigned in the test case (https://github.com/mewspring/c2rust_issues/blob/79fa28e70470a984e4a99b71fccf9e22309d9198/issue-wrapping_mul/issue-wrapping_mul_v0.18.0%2B409/rnd.c#L13) cur_rand_seed = (int32_t)((uint32_t)(MULTIPLIER * ((uint32_t)cur_rand_seed) + INCREMENT)); This would result in the use of
c_ast::BinOp::Multiply if is_unsigned_integral_type => {
Ok(mk().method_call_expr(lhs, mk().path_segment("wrapping_mul"), vec![rhs]))
}
c_ast::BinOp::Multiply => {
Ok(mk().binary_expr(BinOp::Mul(Default::default()), lhs, rhs))
} However, a follow-up test case (https://github.com/mewspring/c2rust_issues/tree/master/issue-wrapping_mul_source_fix) showed that this was not the source of the issue. Even when updating the C source code to use Extracts from C source: uint32_t cur_rand_seed = 0; cur_rand_seed = MULTIPLIER * cur_rand_seed + INCREMENT; Diff between v0.18.0 and v0.18.0+409 output (note, v0.18.0 output is correct): diff -u -r issue-wrapping_mul_v0.18.0/out/src/rnd.rs issue-wrapping_mul_v0.18.0+409/out/src/rnd.rs
--- issue-wrapping_mul_v0.18.0/out/src/rnd.rs 2023-09-10 15:20:23.629045015 +0200
+++ issue-wrapping_mul_v0.18.0+409/out/src/rnd.rs 2023-09-10 15:21:22.242377869 +0200
@@ -16,7 +16,7 @@
pub unsafe extern "C" fn get_rand_seed() -> uint32_t {
let INCREMENT: uint32_t = 1 as libc::c_int as uint32_t;
let MULTIPLIER: uint32_t = 0x15a4e35 as libc::c_int as uint32_t;
- cur_rand_seed = MULTIPLIER.wrapping_mul(cur_rand_seed).wrapping_add(INCREMENT);
+ cur_rand_seed = (MULTIPLIER * cur_rand_seed).wrapping_add(INCREMENT);
let mut ret: uint32_t = abs(cur_rand_seed as int32_t) as uint32_t;
return ret;
} For some reason, the |
Hi @mewmew, I wasn't able to reproduce this. I ran the |
Hi @kkysen, Thanks for taking a look at this. And also, of course for working on c2rust. It's quite an incredible transpiler that you've built! Here is a Docker image that reproduces the issue, built from the official Arch Linux base-devel docker image. docker pull mewbaz/c2rust-git-issue-wrapping_mul Relevant docker out:
In particular, note that the multiplication operand cur_rand_seed = (MULTIPLIER * cur_rand_seed as uint32_t).wrapping_add(INCREMENT)
as int32_t; cur_rand_seed = (MULTIPLIER * cur_rand_seed).wrapping_add(INCREMENT); With kindness, |
There doesn't seem to be a button to re-open issues (for outside collaborators), once an issue has been closed by immunant. Please re-open this issue as it is reproducible given the above docker image (#1024 (comment)). |
I'll re-open it for now; I'll look into it more when I get back from RustConf and have time. |
Sounds good. Enjoy RustConf! |
Hi @kkysen, Hope you're well and enjoying the start of Spring. I was wondering if you had a chance to look at this after you returned from RustCon? Just wanted to confirm whether you could reproduce the issue with the above Docker images? Cheers, |
Thanks! I hope you're well, too! Sorry, I totally forgot about this, that was a while ago. I'll try to look into it soon. Sorry to keep you waiting so long. |
A regression was observed between v0.18.0 vs. v.0.18.0+409 (0e9e404) of c2rust with regards to
wrapping_mul
.A test case has been uploaded to https://github.com/mewspring/c2rust_issues/tree/master/issue-wrapping_mul which showcases the regression.
In particular, v0.18.0 uses
wrapping_mul
to handle multiplication ofuint32_t
expressions, whereas v0.18.0+409 fails to usewrapping_mul
foruint32_t
expressions and instead uses the*
operations which traps on overflow (i.e.panicked at 'attempt to multiply with overflow'
).Diff between v0.18.0 and v0.18.0+409 Rust output.
Steps to reproduce
v.0.18.0
Transpile C code to Rust (with c2rust v.0.18.0 installed)
git clone https://github.com/mewspring/c2rust_issues cd c2rust_issues/issue-wrapping_mul/issue-wrapping_mul_v0.18.0 ./gen_rust.sh
Run C program
Run Rust program
v.0.18.0+409
Transpile C code to Rust (with c2rust v.0.18.0+409 installed)
git clone https://github.com/mewspring/c2rust_issues cd c2rust_issues/issue-wrapping_mul/issue-wrapping_mul_v0.18.0+409 ./gen_rust.sh
Run C program
Run Rust program
The text was updated successfully, but these errors were encountered: