bytecodealliance / wasm-tools

CLI and Rust libraries for low-level manipulation of WebAssembly modules
Apache License 2.0
1.28k stars 230 forks source link

validation: difference in GC subtyping #1717

Closed abrown closed 3 weeks ago

abrown commented 4 weeks ago

Looking at a Binaryen test, I discovered a difference between it and how wasm-tools validates the subtyping of a table elem. The test in question has shared in it, but this is a distraction. The difference between wasm-tools and Binaryen boils down to:

(module
  (type $funcs (array (mut (ref null func))))
  (elem (ref null any))
  (func (array.init_elem $funcs 0 (ref.null none) (i32.const 0) (i32.const 0) (i32.const 0)))
)

This fails to validate with:

invalid array.init_elem instruction: element segment 0 type mismatch: expected funcref, found anyref

What appears to be happening is that Binaryen accepts a (passive?) element segment containing any but wasm-tools does not. The reason for this is that the validator code in question, here, expects the element type to be a subtype of the array item we're about to initialize (ignore that the array is actually null in the above example). But Binaryen has some other kind of logic — which is right?

alexcrichton commented 4 weeks ago

According to this at least the validation of array.init_elem requires that the element segment type is a subtype of the array element type, and anyref is not a subtype of funcref, so I think that this is a correct error.

When I roundtrip this through binaryen wasm-opt foo.wasm -o bar.wasm it replaces array.init_elem with an unreachable instruction. It looks like that's happening before the result is fully validated, meaning that this is mistakenly considered valid.

A small tweak to your test case:

(module
  (type $funcs (array (mut (ref null func))))
  (elem (ref null any))
  (func (param (ref $funcs))
        (array.init_elem $funcs 0 (local.get 0) (i32.const 0) (i32.const 0) (i32.const 0)))
)

where the array is local.get 0 instead of ref.null none which always traps, then binaryen yields:

[wasm-validator error in function 0] array.init_elem segment type must match destination type, on
(array.init_elem $funcs $0
 (local.get $0)
 (i32.const 0)
 (i32.const 0)
 (i32.const 0)
)
Fatal: error validating input

so I think that this is an issue with binaryen?

fitzgen commented 3 weeks ago

cc @tlively

tlively commented 3 weeks ago

Thanks for the cc. The immediate fix is in https://github.com/WebAssembly/binaryen/pull/6852, but we still have related bugs I'll take a look at.

alexcrichton commented 3 weeks ago

I think this has been resolved now, so I'm going to close