wasm-parser: should typed select parse with result arity ≠ 1?

[nagisa]

In the webassembly specification select with any number of result types is allowed by the binary representation, and is rejected in validation rather than the binary encoding.

wasmparser::VisitOperator instead models this kind of instruction as visit_typed_select with a single result argument, and therefore is forced to reject this encoding earlier, while decoding the binary representation:

wasm-tools/crates/wasmparser/src/binary_reader.rs

Lines 785 to 794 in b6233ed

	0x1c => {
	let results = self.read_var_u32()?;
	if results != 1 {
	return Err(BinaryReaderError::new(
	"invalid result arity",
	self.position,
	));
	}
	visitor.visit_typed_select(self.read()?)
	}

In particular WebAssembly/spec@ef01de5 has added tests that do successfully parse (and, I think, validate? Not sure.) which triggers this condition in wasmparser.

Should wasmparser pass this test? Should we allow decoding (and encoding) typed selects with result arity ≠ 1?

[alexcrichton]

From the commit referenced it looks like the binary representation of all of the select instructions is still just either untyped or typed as one result? Is there a particular statement you think should be parsed but isn't?

I suspect there may be an issue parsing (result) (result i32) and the other way around, but that's more of a text issue than a binary issue.

[nagisa]

Nope, parsing the text actually works okay (or at least wast does an okay job at it).

The way I have this set up is that I take wast::Module or whatever call encode on it and then throw the bytes at wasmparser. wasmparser is what fails to handle these 2 specific functions.

  (func (result i32) unreachable select (result i32) (result))
  (func (result i32) unreachable select (result i32) (result) select)

I haven’t looked into this but I believe in either case the first of the selects ends up being encoded as 0x1C 0x00 (that is, a typed_select with 0 result types) and then the if condition in the parsing code fails.

I should actually look into what makes the interpreter stomach these functions fine. I’m still not sure whether this passes the interpreter’s validator (and if so – whether than has anything to do with the unreachable) or if it just so happens that the interpreter does not validate this code at all for some reason. I’ll get back to this some time soon.

It might also be the case that encode works weird, but then there are test cases with results specified in either order.

[alexcrichton]

You can inspect a bit with wasm-tools dump like so:

$ wasm-tools dump <<< "(func (result i32) unreachable select (result i32) (result))"
  0x0 | 00 61 73 6d | version 1 (Module)
      | 01 00 00 00
  0x8 | 01 05       | type section
  0xa | 01          | 1 count
  0xb | 60 00 01 7f | [type 0] Func(FuncType { params: [], returns: [I32] })
  0xf | 03 02       | func section
 0x11 | 01          | 1 count
 0x12 | 00          | [func 0] type 0
 0x13 | 0a 07       | code section
 0x15 | 01          | 1 count
============== func 0 ====================
 0x16 | 05          | size of function
 0x17 | 00          | 0 local blocks
 0x18 | 00          | unreachable
 0x19 | 1c 00       | ??
 0x1b | 0b          | end

so you're right it's popping out as 0x1c 0x00 which might be a bug in the AST-to-binary translation since that should probably be 1 with the i32 result type.