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

incorrect cast in os.time.milliTimestamp #1648

Closed
andrewrk opened this Issue Oct 9, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@andrewrk
Copy link
Member

commented Oct 9, 2018

test failure:

Test 412/648 std-macosx-x86_64-Debug-bare os.time.timestamp...attempt to cast negative value to unsigned integer
/Users/travis/build/ziglang/zig/std/os/time.zig:0:31: 0x10fea8e2c in _milliTimestampDarwin (test.o)
/Users/travis/build/ziglang/zig/std/os/time.zig:261:34: 0x10feed2dd in _std-macosx-x86_64-Debug-bare os.time.timestamp (test.o)
/Users/travis/build/ziglang/zig/build/lib/zig/std/special/test_runner.zig:13:25: 0x110128d05 in _main.0 (test.o)
/Users/travis/build/ziglang/zig/build/lib/zig/std/special/bootstrap.zig:102:22: 0x110128a50 in _main (test.o)
???:?:?: 0x7fffa49e7235 in ??? (???)

relevant code:

fn milliTimestampDarwin() u64 {
    //Sources suggest MacOS 10.12 has support for
    //  posix clock_gettime.
    var tv: darwin.timeval = undefined;
    var err = darwin.gettimeofday(&tv, null);
    debug.assert(err == 0);
    const sec_ms = @intCast(u64, tv.tv_sec) * ms_per_s;
    const usec_ms = @divFloor(@intCast(u64, tv.tv_usec), us_per_s / ms_per_s);
    return u64(sec_ms) + u64(usec_ms);
}

This code is doing an @intCast of a isize to u64. darwin.timeval is defined like this:

pub const timeval = extern struct {
    tv_sec: isize,
    tv_usec: isize,
};

Looks like the fact that the values can be negative is intentional, and so this cast is not valid.

cc @tgschultz

@andrewrk andrewrk added this to the 0.4.0 milestone Oct 9, 2018

@tgschultz

This comment has been minimized.

Copy link
Contributor

commented Oct 10, 2018

Huh, for some reason it hadn't occurred to me that gettimeofday() could return a time before the epoch.

tgschultz added a commit to tgschultz/zig that referenced this issue Oct 10, 2018

@andrewrk

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

Just got it again:

2019-01-30T03:48:37.2363230Z Test 465/702 std-macosx-x86_64-Debug-bare os.time.timestamp...attempt to cast negative value to unsigned integer
2019-01-30T03:48:37.3517990Z /Users/vsts/agent/2.144.2/work/1/s/std/os/time.zig:0:31: 0x10b71a10c in _milliTimestampDarwin (test.o)
2019-01-30T03:48:37.3536040Z /Users/vsts/agent/2.144.2/work/1/s/std/os/time.zig:271:34: 0x10b76618d in _std-macosx-x86_64-Debug-bare os.time.timestamp (test.o)
2019-01-30T03:48:37.3750520Z /Users/vsts/agent/2.144.2/work/1/s/build/release/lib/zig/std/special/test_runner.zig:13:25: 0x10b999645 in _main.0 (test.o)
2019-01-30T03:48:37.3902400Z /Users/vsts/agent/2.144.2/work/1/s/build/release/lib/zig/std/special/bootstrap.zig:105:22: 0x10b999397 in _main (test.o)
2019-01-30T03:48:37.3902730Z ???:?:?: 0x7fff7be0d015 in ??? (???)

https://dev.azure.com/ziglang/zig/_build/results?buildId=540

andrewrk added a commit that referenced this issue Jan 30, 2019

@andrewrk

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

I believe a better fix to this will be to use clock_get_time instead of gettimeofday on darwin, like this: https://stackoverflow.com/a/6725161/432

@andrewrk

This comment has been minimized.

Copy link
Member Author

commented Jan 31, 2019

gettimeofday is actually fine for that function, which only needs millisecond granularity. I figured out the real problem:

 pub const timeval = extern struct {
-    tv_sec: isize,
-    tv_usec: isize,
+    tv_sec: c_long,
+    tv_usec: i32,
 };

The timeval struct was incorrectly declared. With this, everything is fixed.

@andrewrk andrewrk closed this in 7a4ed10 Jan 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.