Skip to content

fix [x]u65529 and above overflowing#15537

Merged
Vexu merged 1 commit intomasterfrom
unknown repository
May 9, 2023
Merged

fix [x]u65529 and above overflowing#15537
Vexu merged 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented May 1, 2023

$ cat overflow.zig
test {
    var a: [1]u65535 = undefined;
    _ = a;
}
$ zig-out/bin/zig test overflow.zig
thread 290266 panic: integer overflow
zig/src/type.zig:3604:55: 0xada43d in intAbiAlignment (zig)
            std.math.ceilPowerOfTwoPromote(u16, (bits + 7) / 8),
                                                      ^
zig/src/type.zig:3598:42: 0xadd4ea in intAbiSize (zig)
        const alignment = intAbiAlignment(bits, target);
                                         ^
zig/src/type.zig:3500:61: 0x92be91 in abiSizeAdvanced (zig)
                return AbiSizeAdvanced{ .scalar = intAbiSize(bits, target) };
                                                            ^
zig/src/type.zig:3385:62: 0x928933 in abiSizeAdvanced (zig)
                switch (try payload.elem_type.abiSizeAdvanced(target, strat)) {
                                                             ^
zig/src/type.zig:3268:32: 0x92c012 in abiSize (zig)
        return (abiSizeAdvanced(ty, target, .eager) catch unreachable).scalar;
                               ^

This is only noticed in a debug build of zig and silently does the wrong thing and overflows in release builds.

This happened to [x]u65529 and above because of the + 7 on a u16.

@Vexu Vexu enabled auto-merge (rebase) May 1, 2023 19:18
auto-merge was automatically disabled May 3, 2023 08:35

Head branch was pushed to by a user without write access

@Vexu Vexu enabled auto-merge (rebase) May 3, 2023 08:51
auto-merge was automatically disabled May 6, 2023 07:38

Head branch was pushed to by a user without write access

@ghost
Copy link
Author

ghost commented May 6, 2023

Everything passed except aarch64-windows. I guess I'll try force-pushing.

@ghost
Copy link
Author

ghost commented May 6, 2023

aarch64-windows failed again. Is this some alignment or too little stack memory thing? I don't see any text telling me what's wrong. The test is fairly target-unspecific.
Am I supposed to do if (builtin.cpu.arch == .aarch64) return error.SkipZigTest;?

@Vexu
Copy link
Member

Vexu commented May 8, 2023

aarch64-windows failed again. Is this some alignment or too little stack memory thing? I don't see any text telling me what's wrong. The test is fairly target-unspecific.
Am I supposed to do if (builtin.cpu.arch == .aarch64) return error.SkipZigTest;?

The test run failed the same way when I restarted it so you should either skip it or modify the test.

```
$ cat overflow.zig
test {
    var a: [1]u65535 = undefined;
    _ = a;
}
$ zig-out/bin/zig test overflow.zig
thread 290266 panic: integer overflow
zig/src/type.zig:3604:55: 0xada43d in intAbiAlignment (zig)
            std.math.ceilPowerOfTwoPromote(u16, (bits + 7) / 8),
                                                      ^
zig/src/type.zig:3598:42: 0xadd4ea in intAbiSize (zig)
        const alignment = intAbiAlignment(bits, target);
                                         ^
zig/src/type.zig:3500:61: 0x92be91 in abiSizeAdvanced (zig)
                return AbiSizeAdvanced{ .scalar = intAbiSize(bits, target) };
                                                            ^
zig/src/type.zig:3385:62: 0x928933 in abiSizeAdvanced (zig)
                switch (try payload.elem_type.abiSizeAdvanced(target, strat)) {
                                                             ^
zig/src/type.zig:3268:32: 0x92c012 in abiSize (zig)
        return (abiSizeAdvanced(ty, target, .eager) catch unreachable).scalar;
                               ^
```
This is only noticed in a debug build of zig and silently does the wrong
thing and overflows in release builds.

This happened to `[x]u65529` and above because of the ` + 7` on a `u16`.
@ghost
Copy link
Author

ghost commented May 8, 2023

And now it's not aarch64-windows but aarch64-macos failing? It has previously passed. A single, unrelated LOC changed. I can't help but think we still have some non-determinism in the compiler.
@Vexu do you mind re-running aarch64-macos?

@Vexu Vexu merged commit 297b5d1 into ziglang:master May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants