Skip to content

wast.rs: Validate, roundtrip and snapshot "unlinkable" modules #2131

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

Merged
merged 1 commit into from
Apr 9, 2025

Conversation

keithw
Copy link
Contributor

@keithw keithw commented Apr 9, 2025

This PR adds test coverage for the assert_unlinkable cases in the spec testsuite.

These are valid modules, so this PR makes the wast subcommand treat them the same as a module declaration (i.e. validate, roundtrip, and snapshot). There are some tests in the testsuite that use assert_unlinkable to test extreme (valid) modules without causing the test runner to actually instantiate a gigantic module (e.g. https://github.com/WebAssembly/custom-page-sizes/blob/main/test/core/custom-page-sizes/memory_max.wast), so it would be good to have wasm-tools run these test cases through the validator instead of ignoring them.

Treating them like a module seems the most consistent, but the snapshots are a lot of new files in the repo and I'm not sure they're totally necessary. We could also just validate and roundtrip without the snapshot.

This is part of the "malformed-invalid separation + roundtrip invalid" patchset (https://github.com/keithw/wasm-tools/tree/malformed-invalid-separation).

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks!

I'm also not a huge fan of the quantity of snapshot files but that's sort of where we're at for now so this isn't unreasonable in that sense.

@alexcrichton alexcrichton added this pull request to the merge queue Apr 9, 2025
Merged via the queue into bytecodealliance:main with commit b9f7a3d Apr 9, 2025
32 checks passed
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.

2 participants