-
Notifications
You must be signed in to change notification settings - Fork 457
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
Add tests for alignment overflow #1751
Conversation
heh. looks like the test is working as intended. ^^ but also. uh, how do you fix that one?
(assert_invalid
(module
(type $0 (func))
(memory $0 1)
(func $0 (type 0) (i32.const 0) (i32.load align=0) (drop))
)
"alignment must not be larger than natural"
) |
To assert malformed text format, you'll need to use |
take a look at the filename again, it's a roundtrip(?) one. we found a bug in the roundtrip(?) function. |
Ah, actually, the bug is in the decoder not considering 2^32 malformed (which then overflows into 0 when converting to text). Can you try changing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. (But please never force-push to a branch under review, that breaks GH PR mechanics.)
oh, really? sorry, we're so used to force-pushing for other projects we contribute to, but we'll keep it in mind. |
Well, it's a common mistake, but you shouldn't do that for any GH project. As just one example of what breaks, here is what happens to commit links; reviewers lose relative diffs and diff links, comments get orphaned or vanish, etc. |
ah, we see... interestingly(? or frustratingly, as it may be) github also provides a "compare" link on the web interface that does work: https://github.com/WebAssembly/spec/compare/b2e95b5d1231ad49d0b381a1af39483e5697c79c..9a09f09f6c6191706ae4a39b6c0aee905f3029ae but yeah, we see what you mean. |
let's see what CI says about these...
closes #1749