std.time.sleep integer overflow

[peteruhnak]

Zig Version

0.10.0-dev.4249+11dce7894

Steps to Reproduce

Not sure how to best categorize this, as it technically works as intended, but it is imho a footgun and a usability issue. User has to always keep this in mind and manually cast when using sleep lest they want a runtime error.

std.time.ns_per_s (and other constants there) have type comptime_int, however their value is almost max u32, so if they are multiplied by even small number (e.g. to sleep 5 seconds) that is not u64 (or comptime known), the multiplication overflows.

If the constants would be explicitly typed to u64 (such as example a()), then this would be avoided. As comptime_int doesn't have a guaranteed size, maybe this would be a relatively safe change?

const std = @import("std");

pub fn main() !void {
    a(5);
    std.log.debug("a() done", .{});
    b(5);
    std.log.debug("b() done", .{});
    c(5);
    std.log.debug("c() done", .{});
}

const ns_per_us: u64 = 1000;
const ns_per_ms: u64 = 1000 * ns_per_us;
const ns_per_s: u64 = 1000 * ns_per_ms;

fn a(sec: u32) void {
    std.time.sleep(sec * ns_per_s); // safe, implicitly coerced to u64
}

fn b(sec: u32) void {
    std.time.sleep(@as(u64, sec) * std.time.ns_per_s); // safe, but requires explicit coersion
}

fn c(sec: u32) void {
    std.time.sleep(sec * std.time.ns_per_s); // integer oveflow
}

Expected Behavior

debug: a() done
debug: b() done
debug: c() done

Actual Behavior

debug: a() done
debug: b() done
thread 38795 panic: integer overflow
/tmp/zig-stuff/async/src/main.zig:62:19: 0x20fef4 in c (async)
    std.time.sleep(sec * std.time.ns_per_s); // integer oveflow
                  ^
/tmp/zig-stuff/async/src/main.zig:45:6: 0x20fe06 in main (async)
    c(5);

[nektro]

as much as I agree that this can be confusing at first glance, consider that this is proposing that the result of math should be different based on if the result location has a known type:

const a: u32 = 5;
const b = std.time.ns_per_s;

const c = a * b;
const d: i64 = a * b;

so if this proposal were to go through, c != d would be true. which, might be something to think about, but requires a restructure of integer promotion rules.

personally, I'm not really decided yet as I write this out. curious on what other people think.

[Vexu]

Related #7967

[peteruhnak]

consider that this is proposing that the result of math should be different based on if the result location has a known type:

Maybe I wasn't clear enough.

What I am asking is to explicitly declare the type of these constants

https://github.com/ziglang/zig/blob/0e6285c/lib/std/time.zig#L119-L147

to be

// Divisions of a nanosecond.
pub const ns_per_us: u64 = 1000;
pub const ns_per_ms: u64 = 1000 * ns_per_us;
pub const ns_per_s: u64 = 1000 * ns_per_ms;

etc

[nektro]

good catch!

nit: i64 is needed to be passed to most std.time functions

otherwise you'll get errors around type u64 is not able to contain all values of type i64