search

WebAssembly / spec

WebAssembly specification, reference interpreter, and test suite.
https://webassembly.github.io/spec/
Other
3.09k stars 438 forks source link

Add tests for alignment overflow #1751

Closed SoniEx2 closed 1 month ago

SoniEx2 commented 1 month ago

let's see what CI says about these...

closes #1749

SoniEx2 commented 1 month ago

heh. looks like the test is working as intended. ^^

but also. uh, how do you fix that one?

../test/core/_output/align.wast.bin.wast.wast:1269.47-1269.54: syntax error: alignment must be a power of two
(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"
)
rossberg commented 1 month ago

To assert malformed text format, you'll need to use module quote, which defers parsing.

SoniEx2 commented 1 month ago

take a look at the filename again, it's a roundtrip(?) one. we found a bug in the roundtrip(?) function.

rossberg commented 1 month ago

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 le_u in interpreter/binary/decode.ml:223 to lt_u?

SoniEx2 commented 1 month ago

oh, really? sorry, we're so used to force-pushing for other projects we contribute to, but we'll keep it in mind.

rossberg commented 1 month ago

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.

SoniEx2 commented 1 month ago

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.