How to check if deserialized exhaustive enum value is legal?

Help
1

A puzzle from TigerBeetle that I don’t quite know how to solve.

We have a setup roughly like this:

const Command = enum(u8) {
  reserved = 0,
  ...
  request_blocks = 19,
  // Note: no `_` here.
}

const MessageHeader = extern struct {
    checksum: u128,
    command: Command,
    // A bunch of other stuff
}

When we receive a message over the network, we verify the checksum, then cast raw bytes to MessageHeader, and verify semantic validity of each field. This works for almost all fields, which are integers or enums with _ (so any bitpattern goes).

The only problem is this command: Command exhaustive enum, as, if we cast it first and then validate, we might create a value with illegal bitpatern, and that would be IB (illegal behavior).

One way I can handle this is by making the enum non-exhaustive, but that means that all the code would have to deal with non-exhaustiveness, which feels wrong — it should be enough to check this once at the edge of the system.

Another approach I can think about is to comptime-compute the offset of the field, and poke the memory directly, while it is still []const u8, to verify that the byte is OK. This feels somewhat indirect to me though, and error prone.

Yet another approach is to write a type-level function which would take my MessageHeader type and return a different type with exhaustive enums replaced with their reprs (MessageHeaderRaw). I can then cast bytes to that MessageHeaderRaw (where every bitpattern is valid), check that enums are all-right, and then cast Raw into non-Raw. Conceptually, it feels like the right way to do it, but it needs a fair amount of comptime machinery (a whole new function to clone the struct basically).

Is there some simpler, more direct way to do what I want here?

1 Like
2

I guess there’s a rather precise question about Illegal Behavior hiding here. If I re-interpret memory as a value of a type, and that value turns out to be invalid, do I hit IB immediately when I crate such a value, or only when I use it?

Like, if I have an *[1] u8 and men.bytesAsValue that to *bool, is it an immediate game over if the value actually stored there is 92, or can I cast that bool back to byte and check that a byte is valid?

If UB is postponed, I think it would be correct for me to do the following:

  • reinterpret bytes as MessageHeader
  • asBytes header.command
  • verify the range of byte
3

Can you explain your thoughts a bit more on why you think the non-exhaustive approach is not favourable to potentially casting invalid bit patterns to other types? I’m interested in what you think the trade-off is to inspire the need for that technique?

4

There’s a bunch of exhaustive matches in the code which I’d rather keep as exhaustive, and things like EnumSet break for non-exhaustive enums.

More philosophically, _ crates ambiguity as to whose job it is to deal with extra variants. If you make enum exhaustive, that becomes clear — the code which re interprets bytes as enum must handle that.

5

Probably there won’t be a final answer to your IB question until we’re going to have a spec, but generally speaking Zig doesn’t have a problem with malformed data existing, just when it gets accessed by operators that expect it to be correct (eg @enumFromInt, @tagName), that said I think the option of checking the byte array directly deserves more consideration since it completely bypasses any question about the language memory model.

const command_bytes = data[@offsetOf(MessageHeader, "command")..][0..@sizeOf(Command)];
const command_id: u32 = @bitCast(command_bytes);
// etc

With just a sprinkle of metaprogramming you can make this entire thing parametric and reusable.

1 Like
6

How about a switch that lists all the enum values, possibly with ranges? Or if your enum has no holes, how about checking if the value is less than or equal to the last enum tag?

There is std.meta.intToEnum, which looks to me like the implementation could perhaps be enhanced but it seems like it matches the API you want.

2 Likes
7

We ended up deciding that crashing would actually be the correct, safe behavior in the face of unknown values for this particular enum, and just added a comment:

2 Likes