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

Support global.get in more constant expressions #7996

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

alexcrichton
Copy link
Member

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

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 alexcrichton requested a review from a team as a code owner February 24, 2024 20:26
@alexcrichton alexcrichton requested review from pchickey and removed request for a team February 24, 2024 20:26
@fitzgen fitzgen requested review from fitzgen and removed request for pchickey February 26, 2024 14:30
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@fitzgen fitzgen added this pull request to the merge queue Feb 26, 2024
Merged via the queue into bytecodealliance:main with commit 36fb62c Feb 26, 2024
19 checks passed
@alexcrichton alexcrichton deleted the update-spec-test-suite branch February 26, 2024 15:17
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Feb 28, 2024
This fixes a mistake introduced in bytecodealliance#7996 where a `global.get` used to
initialize a table did not properly drop previous items in the table
causing them to leak as the reference count was not decremented.
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

2 participants