Skip over unsupported instructions instead of panicking #3029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes rustwasm#2969 This changes `wasm-bindgen-wasm-interpreter` to ignore a function rather than panicking if it contains an unsupported instruction. This works around some runtime glue that gets added to our descriptor functions on the WASI target by more-or-less ignoring them. I also put some work into making sure a helpful error message is given if the actual describe function contains unsupported instructions, by keeping track of the instructions that caused us to skip and including them in the error message if the descriptor is invalid.
|
I think that the better error messages here are definitely good to have, but I would prefer to not skip over unknown functions. We can't really say ahead of time whether they'll be crucial to the execution and final result or not, so I wouldn't be comfortable automatically skipping them. |
|
I will close this for now as it mostly addresses a WASI issue, apart from what alexcrichton said. See #3421. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2969
This changes
wasm-bindgen-wasm-interpreterto ignore a function rather than panicking if it contains an unsupported instruction. This works around some runtime glue that gets added to our descriptor functions on the WASI target by more-or-less ignoring them.I also put some work into making sure a helpful error message is given if the actual describe function contains unsupported instructions, by keeping track of the instructions that caused us to skip and including them in the error message if the descriptor is invalid.