Validating "joined" types for variant during lifting

[alexcrichton]

Currently the canonical ABI defines a "join" function to allow variants to be represented with a flat list of types to get lifted, currently only exposed as the parameters to functions imported by the wasm module. As-is I believe the current rules for validating these parameters are:

• The discriminant, an i32, must be in-bounds
• Used flat values must have zero'd upper bits if the upper bits are discarded for a particular case
• Unused flat values for a particular case are not validated

In implementing this I'm finding it somewhat nontrivial to perform the second bit of validation here, ensuring that the upper bits of each flat type is zero when accessed during lowering. I could elaborate further about that as well, but before getting into that it felt like there was a bit of a discrepancy between validating that unused upper bits are zero but not validating that unused values entirely are zero. Given that, would it perhaps be reasonable to stop validating that the upper bits are zero? Instead allowing arbitrary bit-patterns there? (and variants just use the bits needed for each case)

[lukewagner]

Great points and question! This is one of those cases where I just aimed for symmetry where, in general, we have dynamic traps for unused high bits in scalars. But you're right that the unused scalars of a flattened variant are wholly ignored. Also, looking at this again, unused (padding) bytes are ignored by load_variant/load_int. So there's just as much of a symmetry argument for dropping bullet 2 (concretely, dropping the trap_if from narrow_i64_to_i32) in addition to these implementation concerns you've found, so I'm in favor.

This actually makes me wonder whether, for full symmetry, we should drop the analogous traps in the other lifting definitions:

• lift_flat_unsigned/lift_flat_signed could ignore (semantically mask out) the high bytes
• narrow_uint_to_bool could either ignore high bits or (less surprisingly) treat all non-zero values as true
• unpack_flags_from_int could drop the final trap_if
• i32_to_char should I think stay trapping as-is

WDYT?

[alexcrichton]

I can't recall the reason to validate signed/unsigned integers and the upper bits off the top of my head right now. In lieu of that I think what you're saying of dropping extra validation checks makes sense to me. I definitely agree leaving char the way it is makes the most sense, and it's just the other types where we're truncating entire bits seems fine to ignore that (and for bool treating any nonzero value as "true")

[lukewagner]

This should be fixed by #35.