invalid enum value not always detected where enum size is not explicit

[jorangreef]

Zig Version

0.9.1

Steps to Reproduce

@sentientwaffle and I found something interesting working on TigerBeetle's crash recovery:

const std = @import("std");

const Thing = enum {
    one,
    two,
    three,
};

test "cat in the hat" {
    const expectEqual = std.testing.expectEqual;

    try expectEqual(Thing.one, std.mem.bytesAsValue(Thing, &[_]u8{0}).*);   // 0b00
    try expectEqual(Thing.two, std.mem.bytesAsValue(Thing, &[_]u8{1}).*);   // 0b01
    try expectEqual(Thing.three, std.mem.bytesAsValue(Thing, &[_]u8{2}).*); // 0b10

    // Crash: `invalid enum value`
    //try expectEqual(Thing.one, std.mem.bytesAsValue(Thing, &[_]u8{3}).*); // 0b11

    // These will only crash with `invalid enum value` if you change Thing to enum(u8):
    try expectEqual(Thing.one, std.mem.bytesAsValue(Thing, &[_]u8{4}).*);   // 0b00
    try expectEqual(Thing.two, std.mem.bytesAsValue(Thing, &[_]u8{5}).*);   // 0b01
    try expectEqual(Thing.three, std.mem.bytesAsValue(Thing, &[_]u8{6}).*); // 0b10

    // Crash: invalid enum value
    //try expectEqual(Thing.one, std.mem.bytesAsValue(Thing, &[_]u8{7}).*); // 0b11
}

In our case, we were accidentally accessing an enum on an untrusted data header, before checking the header's checksum. After casting some bytes into a message header, we were inadvertently checking header.command == .prepare and header.valid_checksum() (instead of the other way around).

Expected Behavior

We expected Zig to catch this at runtime with invalid enum value for all invalid values.

Actual Behavior

Unfortunately, some invalid values were bypassing the runtime check, and worse, colliding with valid values if they shared the same binary suffix.

[jorangreef]

On second thought, this might be a little surprising but it's not necessarily a bug, since the size of the enum here is not explicitly defined—there has to be some fixed size under the hood (in this case 2 bits) and it's natural for the safety check to operate on that range only without checking anything more significant.

[jorangreef]

The test is also going out of band with std.mem.bytesAsValue instead of using @intToEnum.

[ifreund]

This has the same root cause as #11263, namely LLVM's arbitrary bit width integer semantics which I described in that thread.

I don't think this is a compiler bug so much as a design flaw in the language. This behavior is not intuitive and I think it should be improved.