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.

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

Update WebAssembly testsuite #6705

Closed
wants to merge 2 commits into from

Conversation

abrown
Copy link
Collaborator

@abrown abrown commented Jul 7, 2023

This PR updates the WebAssembly testsuite to use all of the latest tests. It highlights four failures, I think:

  • elem.wast
  • function_references/data.wast
  • function_references/elem.wast
  • function_references/global.wast

Not sure how critical these are to fix but I thought I would point these out in a draft PR.

@abrown
Copy link
Collaborator Author

abrown commented Jul 7, 2023

@alexcrichton
Copy link
Member

I think these failures are related to wasmparser and its validation, so the test suite submodule probably will need to get updated in the wasm-tools repository before it's updated here.

@alexcrichton
Copy link
Member

With WebAssembly/testsuite#68, bytecodealliance/wasm-tools#1126, and publishing wasm-tools this should be good to go.

@abrown
Copy link
Collaborator Author

abrown commented Jul 18, 2023

Ok, rebased on top of #6739.

@alexcrichton
Copy link
Member

Oh can you also try re-updating the submodule its latest? I think that will surface a single error in tests/spec_testsuite/elem.wast:690 which I think is something we need to implement, but otherwise the other failures I think are fixed by updating the test suite

@alexcrichton
Copy link
Member

Ok I think those are legitimate failures where we accidentally don't support global.get in table element segment offset initializers.

@TethysSvensson
Copy link

It seems like global.get has been a constant expression at least since 2017 (WebAssembly/spec#445). However it was not usable inside element segments until the reference types proposal landed in 2021 (WebAssembly/spec#1287). This made funcrefs/externrefs available as globals, and thus global.get became usable in element segments.

Unfortunately the interaction between this proposal and element segments was initially missed and a test was not added for it until earlier this year in WebAssembly/spec#1640. I think most wasm parsers missed this detail. For instance wabt only added support for this today in WebAssembly/wabt#2288.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Feb 24, 2024
This commit updates Wasmtime to support `global.get` in constant
expressions when located in table initializers and element segments.
Pre-reference-types this never came up because there was no valid
`global.get` that would typecheck. After the reference-types proposal
landed however this became possible but Wasmtime did not support it.
This was surfaced in bytecodealliance#6705 when the spec test suite was updated and has
a new test that exercises this functionality.

This commit both updates the spec test suite and additionally adds
support for this new form of element segment and table initialization
expression.

The fact that Wasmtime hasn't supported this until now also means that
we have a gap in our fuzz-testing infrastructure. The `wasm-smith`
generator is being updated in bytecodealliance/wasm-tools#1426 to
generate modules with this particular feature and I've tested that with
that PR fuzzing here eventually generates an error before this PR.

Closes bytecodealliance#6705
@alexcrichton
Copy link
Member

I've "rebased" this and implemented the missing features from Wasmtime in #7996

github-merge-queue bot pushed a commit that referenced this pull request Feb 26, 2024
This commit updates Wasmtime to support `global.get` in constant
expressions when located in table initializers and element segments.
Pre-reference-types this never came up because there was no valid
`global.get` that would typecheck. After the reference-types proposal
landed however this became possible but Wasmtime did not support it.
This was surfaced in #6705 when the spec test suite was updated and has
a new test that exercises this functionality.

This commit both updates the spec test suite and additionally adds
support for this new form of element segment and table initialization
expression.

The fact that Wasmtime hasn't supported this until now also means that
we have a gap in our fuzz-testing infrastructure. The `wasm-smith`
generator is being updated in bytecodealliance/wasm-tools#1426 to
generate modules with this particular feature and I've tested that with
that PR fuzzing here eventually generates an error before this PR.

Closes #6705
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants