Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

Implement a binary reader for function references instructions.#1

Merged
CosineP merged 2 commits intoeffect-handlers:func-reffrom
dhil:funcref-instr-syntax
Jul 6, 2022
Merged

Implement a binary reader for function references instructions.#1
CosineP merged 2 commits intoeffect-handlers:func-reffrom
dhil:funcref-instr-syntax

Conversation

@dhil
Copy link
Copy Markdown

@dhil dhil commented Jul 5, 2022

This patch extends the binary reader to enable reading of

  • call_ref
  • return_call_ref
  • ref.as_non_null
  • br_on_null $l
  • br_on_non_null $l

The static semantics are yet to be implemented.

I have updated the patch to also include mutators and binary encoders for the above instructions.

(I have removed the latest commits from upstream/main to keep the PR clean.)

@dhil dhil requested a review from CosineP July 5, 2022 15:01
@CosineP
Copy link
Copy Markdown
Collaborator

CosineP commented Jul 5, 2022

Cool. Do you know what the testing situation is yet? We should figure out which parts of the proposal are tested where / need to be enabled. Do you know if the wast crate encodes these instructions correctly yet? There are (at least?) two binary format printers.

@dhil
Copy link
Copy Markdown
Author

dhil commented Jul 6, 2022

Good question regarding the tests. I am not sure. I will look into it; I reckon the good people on the Bytecode Alliance Zulip will be able to give us a definite answer.

AFAIK the wast crate encodes the above instructions correctly.

@dhil dhil force-pushed the funcref-instr-syntax branch 2 times, most recently from 5a4ce20 to b476a8a Compare July 6, 2022 12:48
This commit extends the binary reader to enable reading of

* `call_ref`
* `return_call_ref`
* `ref.as_non_null`
* `br_on_null $l`
* `br_on_non_null $l`

The commit also adds mutators and encoders for the said instructions.
@dhil dhil force-pushed the funcref-instr-syntax branch from b476a8a to ae0fe56 Compare July 6, 2022 12:49
@dhil
Copy link
Copy Markdown
Author

dhil commented Jul 6, 2022

I've extended the binary encoder in wasm-encode and the textual printer in wasmprinter; I am not sure what other binary encoder you are referring to.

@CosineP CosineP merged commit 9216ee9 into effect-handlers:func-ref Jul 6, 2022
@dhil dhil deleted the funcref-instr-syntax branch July 6, 2022 15:10
dhil added a commit that referenced this pull request Feb 15, 2023
* [WIP] Parse reference types from binary

This identifies reference types' magic numbers and advances the parser.
It parses heap types and assembles them into a type for non-nullable
references. But, it does not actually change the syntax yet so it just
panics. It remains to be implemented for nullable references and the
sugars.

* Add nullable reference types and desugar forms

Untested

* [WIP] Factor RefType out of ValType

This checks for wasmparser, but no tests and no other crates yet

* [func-refs] Fix parser tests, finish ref type parsing

* [func-refs] Support ref types in pretty-printer

* [func-refs] Fix pretty-printer bugs

Missing parenthesis and printing a valtype instead of a heap type for
func.ref

* [func-refs] Add support for validator func.ref

* [func-refs] Add syntax in encoder but not support

* [func-refs] Fix printer giving bad types of indices

* Clean up debugging nonsense

* [func-refs] Implement subtyping

* Implement a binary reader for function references instructions. (#1)

* Change ref.null to take heaptype, not valtype

* Update error message to match tests

* Hack around error message test

* [WIP] [func-refs] Add call_ref static semantics

WIP because untested

* Static semantics for function references (#2)

Further changes to validation need to be made, at least including adding subtyping to more places.

* [func-refs] Add subtyping to elements

* Subtyping relation (#4)

* Refactor the validator for function references (#5)

Co-authored-by: Daniel Hillerström <daniel.hillerstrom@ed.ac.uk>
Co-authored-by: cosine <CosineP@users.noreply.github.com>

* [func-refs] Assume matches V128 V128 = true

It looks to me that the function references spec was forked from
webassembly before vectors were merged, so it has no definitive answer
on V128 subtyping.  It also means we accidentally reverted `select`
to an outdated spec.  I think for now it's reasonable to use common
sense for V128 subtyping, and at some point the proposal is presumably
planned to be rebased?

* [func-refs] Support validate flag, br_table.wast notes

* [func-refs] Bypass inconsistent test message

This is a mismatch from the original spec tests, however in
function-references the same exact test for some reason has a slightly
different message, so the bypass needs another case.

* [func-refs] Require defaultable table types

This is from the specification of defaultability from here:
WebAssembly/function-references#62

That change still doesn't include locals, which are under discussion.
The implementation as of now still requires defaultable locals as in the
test suite.

* [func-refs] Add subtyping to table.copy

* [func-refs] Correct f type structural equivalence

This checks that arguments to a function are themselves structurally
equivalent. Note that this is at least as difficult as a recursive
subtyping relation would be, since we now need to implement
vt1 <= vt2
AND
vt1 == vt2
including for function types, despite the fact that <= is (even stated
to be) preferable in this case

* [func-refs] Perform some cleanup

Move some things into order and fix some doc comments.  This is not
enough cleanup, more needs to be done

* Fix bad brace matching from github commit

* [func-refs] Order operator validation as in spec

* [func-refs] Restore feature check in return_call_ref

* Fix todos in validator/core.rs (#8)

* Add missing brace

* Fix bad merge

* Fix failing test case (#11)

* Implement missing functions in EmptyResources trait in func.rs

* Fix failing test

* [func-refs] Reorder wasmprinter instructions

* [func-refs] Remove incorrect comment; format

* Merge with upstream; simplify the implementation of matches.

* Run cargo fmt

* Fix compile errors with the rest of the workspace

* Leave `unimplemented!()` for `Ref` types other than funcref/externref
* Leave `unreachable!()` for `Bot` cases

* Print old `funcref` and `externref` for those types

Helps improve compatibility with other text parsers and additionally
fixes a few tests which have "golden output" and assert that `funcref`
and `extenref` are printed.

* Fix a doctest example

* Fix indirect_call subtype; small comments

* Fix wasmparser benchmark

* Fixup merge conflicts

* Add type payload to `call_ref` and `return_call_ref`

* Fix a typo

* Fix tests

* Remove ValType::Bot

* Rename `HeapType::Index` to `HeapType::TypedFunc`.

* Enable new working, disable new broken tests

* Pack ValType into 4 bytes

* [func-refs] Implement initialization checking

* Fix printing of element kind

* Update dump

* wip

* wip

* Fix failing `*.wast` tests and align APIs

This commit fixes some decoding of types in the `wasmparser` crate and
then additionally adds support in `wasmparser`, `wasm-encoder`, etc, for
the initialization expression of a table being specified. This involved
aligning the type hierarchy of `wasm-encoder` with that of `wasmparser`
which involved quite a few changes in a number of crates. Overall though
this is mostly syntactic changes without much meat happening here.

* Touch up some docs and style

* Handle some minor TODO comments

* Improve an error message

* Update test exemptions

* Remove redundant branches

* Attempt to fix fuzz/fuzz_targets/validate.rs

* fmt

* Remove `HeapType::Bot` from the public API

Move it as an internal implementation detail of the validator.

---------

Co-authored-by: Luna Phipps-Costin <phipps-costin.l@northeastern.edu>
Co-authored-by: cosine <CosineP@users.noreply.github.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
Co-authored-by: cosine <trash@cosine.online>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants