Change an assert_malformed to assert_invalid

[alexcrichton]

Inspired by changes in bytecodealliance/wasm-tools#2134 and intended to reflect how the maximum page size is an artifact of validation, not binary parsing.

[alexcrichton]

There's some discussion about this here as well. One consequence of this change is that large values of the page size wouldn't be easily representable in the text format so printing an invalid module could be "weird"

[rossberg]

There is precedence for this in terms of load/store alignment, which also is represented in log2 in the binary format, while the text format in fact is specified to only allow for a u32 non-logarithmic value. And yet checking the max value is part of validation. Technically, that breaks the intended inter-convertibility between the two formats, which is annoying.

In principle, we could easily allow the text format to support the whole value range, but that would require every parser to effectively use bigints to parse logarithmic constants.

[keithw]

@rossberg Hmm, that hadn't been my understanding. In the spec main branch, a binary align of 32 or greater is assert_malformed in the binary format (tests at https://github.com/WebAssembly/spec/blob/main/test/core/align.wast#L890, and here's the require (I32.lt_u align 32l) in the reference interpreter binary decoder: https://github.com/WebAssembly/spec/blob/main/interpreter/binary/decode.ml#L223).

As far as I knew, any well-formed alignment in the binary format can currently be represented in the text format and vice versa. It would sort of be nice to keep that principle if possible?

[fitzgen]

LGTM but I haven't followed the malformed vs invalid discussions in detail, so I am hesitant to merge this until more folks sign off on this / there seems like there's consensus that this is what we want.

[alexcrichton]

Makes sense, although I'm not keen on championing this so I'll close this out.

[rossberg]

@keithw,

With Wasm 3.0 and multi-memory, we have repurposed the alignment immediate in the binary format as a bitfield, with only 6 bits used for the actual alignment. So any log value larger than 2^6 is now unrepresentable and hence malformed (like in the test you link). Any value between that and the natural alignment will be a validation error, however (see tests starting at https://github.com/WebAssembly/spec/blob/main/test/core/align.wast#L305, which are converted to binary by the harness).

The text format isn't quite consistent with that and still allows any u32 alignment value to go through parsing (and then result in a validation error). Perhaps we should change that.

Relevant spec links:

Binary format: https://wasm-dsl.github.io/spectec/core/binary/instructions.html#binary-memarg
Text format: https://wasm-dsl.github.io/spectec/core/text/instructions.html#text-memarg
Validation: https://wasm-dsl.github.io/spectec/core/valid/instructions.html#t-mathsf-xref-syntax-instructions-syntax-instr-memory-mathsf-load-n-mathsf-xref-syntax-instructions-syntax-sx-mathit-sx-x-xref-syntax-instructions-syntax-memarg-mathit-memarg

[rossberg]

Small correction: the tests you have linked do not actually overflow the binary rep. But they are in fact outdated. With 3.0/multi-memory, they have been changed to invalid:

https://github.com/WebAssembly/spec/blob/wasm-3.0/test/core/align.wast#L890