Fix encoding of heap type definitions #6856
Merged
Conversation
This file contains hidden or 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
kripken
reviewed
Aug 20, 2024
| @@ -1,963 +0,0 @@ | |||
| ;; Unsigned LEB128 can have non-minimal length | |||
Member
There was a problem hiding this comment.
Is this test invalid? I'd expect a fix to allow more tests to run, so it's not obvious to me why a test was removed.
Member
Author
There was a problem hiding this comment.
It's removed because we can now run the upstream version at test/spec/testsuite/binary-leb128.wast.
kripken
reviewed
Aug 20, 2024
src/wasm/wasm-binary.cpp
Outdated
| } | ||
|
|
||
| if (rawAlignment > 8) { | ||
| if (rawAlignment >= 8) { |
kripken
approved these changes
Aug 20, 2024
tlively
force-pushed
the
testsuite-fix-1
branch
from
c472eff to
d4cda74
Compare
tlively
force-pushed
the
testsuite-fix-1
branch
from
d4cda74 to
88d0c99
Compare
The leading bytes that indicate what kind of heap type is being defined are bytes, but we were previously treating them as SLEB128-encoded values. Since we emit the smallest LEB encodings possible, we were writing the correct bytes in output files, but we were also improperly accepting binaries that used more than one byte to encode these values. This was caught by an upstream spec test.
tlively
force-pushed
the
testsuite-fix-1
branch
from
88d0c99 to
69d4a07
Compare
Closed
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.
The leading bytes that indicate what kind of heap type is being defined
are bytes, but we were previously treating them as SLEB128-encoded
values. Since we emit the smallest LEB encodings possible, we were
writing the correct bytes in output files, but we were also improperly
accepting binaries that used more than one byte to encode these values.
This was caught by an upstream spec test.