zig icon indicating copy to clipboard operation
zig copied to clipboard

Inconsistent stage1/stage2 behavior for @rem

Open ehaas opened this issue 3 years ago • 2 comments

Zig Version

0.11.0-dev.3+0bbb00035

Steps to Reproduce and Observed Behavior

const std = @import("std");
test {
    var x: c_int = 5;
    var y: c_int = -2;
    try std.testing.expectEqual(@as(c_int, 1), @rem(x, y));
}

zig test -fstage1 test.zig produces panic: remainder division by zero or negative value (expected behavior according to https://ziglang.org/documentation/0.10.0/#rem)

zig test test.zig produces the correct answer according to the formula (@divTrunc(a, b) * b) + @rem(a, b) == a

Expected Behavior

Should panic due to negative divisor, or the docs should be updated to indicate that negative divisors are allowed.

Original discussion of negative denominators is here https://github.com/ziglang/zig/issues/217

ehaas avatar Oct 31 '22 22:10 ehaas

This safety check was removed in 5c9826630dfb8a2f59663a569bb8e452359c9524

Vexu avatar Nov 01 '22 18:11 Vexu

Depending on how it's lowered, that definition (where negative divisors are allowed) could lead to undefined behavior if the dividend is a signed int with the minimum value for the type, and the divisor is -1.

const std = @import("std");

test "@rem" {
    var x: i32 = std.math.minInt(i32);
    var y: i32 = -1;
    try std.testing.expectEqual(@as(i32, 0), @rem(x, y));
}

In debug mode on an x86 mac I get:

zig test test.zig
Test [1/1] test.@rem... Arithmetic exception at address 0x10183a9c4
./test.zig:6:32: 0x10183a9c4 in test.@rem (test)
    try std.testing.expectEqual(@as(i32, 0), @rem(x, y));
                               ^

In ReleaseSafe (the found value varies on each run):

zig test -OReleaseSafe test.zig
Test [1/1] test.@rem... expected 0, found 53712615
Test [1/1] test.@rem... FAIL (TestExpectedEqual)
/Users/ehaas/source/zig/lib/std/testing.zig:79:17: 0x10331282b in test.@rem (test)
                return error.TestExpectedEqual;
                ^
./test.zig:6:5: 0x103312833 in test.@rem (test)
    try std.testing.expectEqual(@as(i32, 0), @rem(x, y));
    ^

In ReleaseFast:

zig test -OReleaseFast test.zig
Test [1/1] test.@rem... expected 0, found 0
Test [1/1] test.@rem... FAIL (TestExpectedEqual)
0 passed; 0 skipped; 1 failed.

ehaas avatar Nov 01 '22 19:11 ehaas