Coder Social home page Coder Social logo

Comments (8)

kkysen avatar kkysen commented on September 20, 2024 1

I'll re-open it for now; I'll look into it more when I get back from RustConf and have time.

from c2rust.

kkysen avatar kkysen commented on September 20, 2024 1

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, Robin

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.

from c2rust.

mewmew avatar mewmew commented on September 20, 2024

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 * instead of wrapping_mul in convert_binary_operator (

c_ast::BinOp::Multiply if is_unsigned_integral_type => {
)

            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 uint32_t as type of the cur_rand_seed variable (thus alleviating the need of the cast expression), the c2rust output of v0.18.0+409 was still incorrect.

Extracts from C source:
https://github.com/mewspring/c2rust_issues/blob/79fa28e70470a984e4a99b71fccf9e22309d9198/issue-wrapping_mul_source_fix/issue-wrapping_mul_v0.18.0%2B409/rnd.c#L4

uint32_t cur_rand_seed = 0;

https://github.com/mewspring/c2rust_issues/blob/79fa28e70470a984e4a99b71fccf9e22309d9198/issue-wrapping_mul_source_fix/issue-wrapping_mul_v0.18.0%2B409/rnd.c#L13

	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 uint32_t type of cur_rand_seed does not seem to be enough to trigger the is_unsigned_integral_type case of convert_binary_operator, and as such, the wrapping_mul operand is not used even though it should be.

from c2rust.

kkysen avatar kkysen commented on September 20, 2024

Hi @mewmew, I wasn't able to reproduce this. I ran the gen_rust.shs from your repo with c2rust compiled from master (C2Rust v0.18.0+409 (0e9e4048b 2023-09-08)), but everything used .wrapping_mul, not *. I'll close this for now, but if you can find a test case that successfully reproduces the issue, you can re-open it.

from c2rust.

mewmew avatar mewmew commented on September 20, 2024

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:

Step 6/25 : RUN rustup show
 ---> Running in 96584e402e34
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/user123/.rustup

installed toolchains
--------------------

nightly-2022-08-08-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu (default)

active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (default)
rustc 1.74.0-nightly (030e4d382 2023-09-10)



Step 7/25 : RUN c2rust --help
 ---> Running in fdb8f91e0685
C2Rust v0.18.0+409 (0e9e4048b 2023-09-08)
The C2Rust Project Developers <[email protected]>



Step 19/25 : RUN c2rust transpile --emit-build-files compile_commands.json -o out --binary main -- -I/usr/lib/clang/16/include
 ---> Running in ecf9e1812c04
Transpiling main.c
Transpiling rnd.c
Removing intermediate container ecf9e1812c04
 ---> 979670a377d8
Step 20/25 : RUN cat out/src/rnd.rs
 ---> Running in 631e857af489
use ::libc;
extern "C" {
    fn abs(_: libc::c_int) -> libc::c_int;
}
pub type __int32_t = libc::c_int;
pub type __uint32_t = libc::c_uint;
pub type int32_t = __int32_t;
pub type uint32_t = __uint32_t;
#[no_mangle]
pub static mut cur_rand_seed: int32_t = 0 as libc::c_int;
#[no_mangle]
pub unsafe extern "C" fn set_rand_seed(mut s: int32_t) {
    cur_rand_seed = s;
}
#[no_mangle]
pub unsafe extern "C" fn get_rand_seed() -> int32_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 * cur_rand_seed as uint32_t).wrapping_add(INCREMENT)
        as int32_t;
    let mut ret: int32_t = abs(cur_rand_seed);
    return ret;
}



Step 24/25 : RUN c2rust transpile --emit-build-files compile_commands.json -o out --binary main -- -I/usr/lib/clang/16/include
 ---> Running in 8f9b19f7cbd9
Transpiling main.c
Transpiling rnd.c
Removing intermediate container 8f9b19f7cbd9
 ---> 76519b9517ae
Step 25/25 : RUN cat out/src/rnd.rs
 ---> Running in de3cc82b65d0
use ::libc;
extern "C" {
    fn abs(_: libc::c_int) -> libc::c_int;
}
pub type __int32_t = libc::c_int;
pub type __uint32_t = libc::c_uint;
pub type int32_t = __int32_t;
pub type uint32_t = __uint32_t;
#[no_mangle]
pub static mut cur_rand_seed: uint32_t = 0 as libc::c_int as uint32_t;
#[no_mangle]
pub unsafe extern "C" fn set_rand_seed(mut s: uint32_t) {
    cur_rand_seed = s;
}
#[no_mangle]
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 * cur_rand_seed).wrapping_add(INCREMENT);
    let mut ret: uint32_t = abs(cur_rand_seed as int32_t) as uint32_t;
    return ret;
}

In particular, note that the multiplication operand * is used instead of wrapping_mul in both outputs:

    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,
Robin

from c2rust.

mewmew avatar mewmew commented on September 20, 2024

I'll close this for now, but if you can find a test case that successfully reproduces the issue, you can re-open it.

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)).

from c2rust.

mewmew avatar mewmew commented on September 20, 2024

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!

from c2rust.

mewmew avatar mewmew commented on September 20, 2024

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,
Robin

from c2rust.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.