wabt icon indicating copy to clipboard operation
wabt copied to clipboard

Type funcref crash due to ref type getting read as "any" type instead of a ref type

Open takikawa opened this issue 3 years ago • 1 comments

A test like the following currently crashes in type-checking:

;;; TOOL: wat2wasm
;;; ARGS: --enable-function-references
;;; ERROR: 1
(module
  (type $f32-f32-1 (func (param f32) (result f32)))
  (type $f32-f32-2 (func (param f32) (result f32)))

  (func $foo (param $f (ref $f32-f32-1)) (result f32)
    (call_ref (f32.const 42.0) (local.get $f))
  )

  (func $bar (type $f32-f32-2)
    (f32.const 1.0)
  )

  (func (export "main") (result f32)
    ;; $f32-f32-1 and $f32-f32-2 should be equal
    (call $foo (ref.func $bar))
  )

  (elem declare funcref (ref.func $bar))
)

(;; STDERR ;;;
;;; STDERR ;;)

Error:

- test/typecheck/funcref-equality.txt (roundtrip)                                   
  b'Signal raised running "wat2wasm": SIGABRT\nwat2wasm: ../../../src/type.h:132: wabt::Index wabt::Type::GetReferenceIndex() const: Assertion `enum_ == Enum::Reference\' failed.\n'

I think the immediate reason is that type checking should account for subtyping, and make sure to compare heap types instead of assuming all reference types have an index (i.e., are a "concrete reference type" as described in the GC spec).

More generally, I think the type-checker needs to also be extended to look at the type section (or a more abstract "type store") to look up type indices when checking equality of concrete types. Alternatively, concrete ref types should be stored pre-canonicalized as a pointer (this would depend on https://github.com/WebAssembly/wabt/pull/1828) to a representative type in the canonicalized type store.

These are all needed eventually for GC support too.

takikawa avatar Apr 01 '22 17:04 takikawa

It turned out that the crash in this issue wasn't due to an issue with type-checking, it seems to actually be a parsing bug so I've renamed the issue.

I'll file a separate issue for the actual subtyping things we should add.

takikawa avatar Apr 04 '22 23:04 takikawa