Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
takikawa opened this issue Apr 1, 2022 · 1 comment · May be fixed by #1889
Open

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

takikawa opened this issue Apr 1, 2022 · 1 comment · May be fixed by #1889

Comments

@takikawa
Copy link
Contributor

takikawa commented Apr 1, 2022 •
edited
Loading

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 #1828) to a representative type in the canonicalized type store.

These are all needed eventually for GC support too.
@takikawa takikawa changed the title Typed funcref support should support subtyping and "concrete type" equality Type funcref crash due to ref type getting read as "any" type instead of a ref type Apr 4, 2022
@takikawa
Copy link
Contributor Author

takikawa commented Apr 4, 2022

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 takikawa linked a pull request Apr 4, 2022 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix parsing for typed function reference types takikawa/wabt
1 participant
@takikawa