wasmparser: Fix validation of the return_call family of instructions
#1585
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
We need to additionally check that the callee's results are an exact match of the caller's results. We were incorrectly allowing return calls that would push more values on the operand stack than would be returned. That is fine with a `call; return` sequence, where extra values on the stack are allowed to dangle, but not okay with a `return_call`. With a `return_call` it doesn't make sense because the callee might need a return pointer to put all its results into, but the caller can't supply one since its frame is going away, nor can the caller forward a return pointer that it received to the callee, since it might not return enough values to require a return pointer. This commit fixes the validation to match the spec and disallow `return_call`s that would leave dangling values on the operand stack. cc bytecodealliance/wasmtime#8704 Co-Authored-By: Trevor Elliott <[email protected]>
fitzgen
force-pushed
the
fix-return-call-validation
branch
from
6f9b07b to
d7eabc4
Compare
alexcrichton
approved these changes
May 29, 2024
|
Upstreaming these new tests to the spec testsuite here: WebAssembly/function-references#116 |
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.
We need to additionally check that the callee's results are an exact match of the caller's results. We were incorrectly allowing return calls that would push more values on the operand stack than would be returned. That is fine with a
call; returnsequence, where extra values on the stack are allowed to dangle, but not okay with areturn_call. With areturn_callit doesn't make sense because the callee might need a return pointer to put all its results into, but the caller can't supply one since its frame is going away, nor can the caller forward a return pointer that it received to the callee, since it might not return enough values to require a return pointer. This commit fixes the validation to match the spec and disallowreturn_calls that would leave dangling values on the operand stack.cc bytecodealliance/wasmtime#8704