ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.88k stars 2.48k forks source link

translate-c exposing undefined behavior in C code with signed integer arithmetic #4265

Closed jeffkdev closed 4 years ago

jeffkdev commented 4 years ago

Use case is this file: https://github.com/smcameron/open-simplex-noise-in-c/blob/master/open-simplex-noise.c#L195 Crashes on line 195 with "Illegal Operation" in debug mode only. Ran translate-c, below is a snippet of what it generates:


// Manually modified to work
pub export fn create_seed_working(arg_seed: i64,) i64 {
    var seed = arg_seed;
    seed = ((seed *% @as(c_longlong, 6364136223846793005)) +% @as(c_longlong, 1442695040888963407));
    seed = ((seed *% @as(c_longlong, 6364136223846793005)) +% @as(c_longlong, 1442695040888963407));
    seed = ((seed *% @as(c_longlong, 6364136223846793005)) +% @as(c_longlong, 1442695040888963407));
    return seed;
}

// What translate-c generate from :
//  seed = seed * 6364136223846793005LL + 1442695040888963407LL;
//  seed = seed * 6364136223846793005LL + 1442695040888963407LL;
//  seed = seed * 6364136223846793005LL + 1442695040888963407LL;
pub export fn create_seed_broken(arg_seed: i64,) i64 {
    var seed = arg_seed;
    seed = ((seed * @as(c_longlong, 6364136223846793005)) + @as(c_longlong, 1442695040888963407));
    seed = ((seed * @as(c_longlong, 6364136223846793005)) + @as(c_longlong, 1442695040888963407));
    seed = ((seed * @as(c_longlong, 6364136223846793005)) + @as(c_longlong, 1442695040888963407));
    return seed;
}

pub fn main() void {
    const works = create_seed_working(123);
    const broken = create_seed_broken(456);
}

Basically the C code depends on the defined wrap-around behavior for unsigned integers, but the generated Zig code uses the standard operators. Can be reproduced by running: zig translate-c open-simplex-noise.c

0.5.0 +a6f6d8d2f

daurnimator commented 4 years ago

The failure with "Illegal Operation" is a 2nd issue that should also be solved.

andrewrk commented 4 years ago

The C code has Undefined Behavior. Those are signed ints. Zig does honor wrapping arithmetic for unsigned ints.

jeffkdev commented 4 years ago

Doh! Sorry about that - completely misread the C type. Very nice of Zig to catch this undefined behavior :)