Skip to content
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

Invalid execution of optimized smallpt binary #218

Closed
vshymanskyy opened this issue Mar 24, 2021 · 12 comments
Closed

Invalid execution of optimized smallpt binary #218

vshymanskyy opened this issue Mar 24, 2021 · 12 comments
Labels
bug Something isn't working

Comments

@vshymanskyy
Copy link
Member

vshymanskyy commented Mar 24, 2021

./wasm3 smallpt-ex.wasm 4 32 | sha1sum
Produces (correctly) ea05d85998b2f453b588ef76a1256215bf9b851c

However if smallpt-ex.wasm is build with -O3 or -Os flags, it produces wrong output.
With -O2 or -Oz, it works as expected. They all execute fine in wasmer.

Pre-built binaries: smallpt.zip

@vshymanskyy vshymanskyy added the bug Something isn't working label Mar 24, 2021
@vshymanskyy
Copy link
Member Author

Looks related to this PR: #175

@vshymanskyy
Copy link
Member Author

vshymanskyy commented Mar 30, 2021

After reverting #175 things are better, but -Os build still fails:

$ ./wasm3 smallpt-ex-Os.wasm 4 32 > out-Os.ppm
...
$ sha1sum *.ppm
ea05d85998b2f453b588ef76a1256215bf9b851c  out-O2.ppm
ea05d85998b2f453b588ef76a1256215bf9b851c  out-O3.ppm
b25e6e933fef1ac856e32775706c2ec1dab22ec5  out-Os.ppm
ea05d85998b2f453b588ef76a1256215bf9b851c  out-Os-wasmer.ppm
ea05d85998b2f453b588ef76a1256215bf9b851c  out-Oz.ppm

@vshymanskyy vshymanskyy changed the title Invalid execution of some modules Invalid execution of optimized smallpt binary Apr 6, 2021
@vshymanskyy
Copy link
Member Author

Also in this example wasm3 fails with [trap] invalid conversion to integer, but it doesn't happen on browser engines.
music-bug.zip

The zip file contains execution traces of wasm3 and nodejs for comparison.

Steps:

➜ ./wasm3 --repl
wasm3> :load music.wasm
wasm3> :set-global SAMPLERATE 1024
wasm3> :get-global SAMPLERATE
1024.000000:f32
wasm3> playEventsAndFillSampleBuffer
==== wasm backtrace:
  0: 0x0039ca - .unnamed!<unnamed>
  1: 0x003a7e - .unnamed!<unnamed>
  2: 0x0028f4 - .unnamed!<unnamed>
  3: 0x002e01 - .unnamed!fillSampleBuffer
  4: 0x003693 - .unnamed!playEventsAndFillSampleBuffer
Error: [trap] invalid conversion to integer ()

soundandform added a commit that referenced this issue Apr 9, 2021
@soundandform
Copy link
Member

Made some progress figuring this out. With some tweaking, i can achieve a successful hash on all but I still haven't tracked down the cause of failure.

@vshymanskyy
Copy link
Member Author

@soundandform I can confirm it also fixes the music bug.
@petersalomonsen We can now play your midisynth example 🥳

@vshymanskyy
Copy link
Member Author

@soundandform but it also breaks some other examples from pywasm3 😢

@soundandform
Copy link
Member

@soundandform but it also breaks some other examples from pywasm3 😢

@vshymanskyy taking a quick look at pywasm code, this is probably due to the change of call frame, preparing for multi-return. empty slots (= to return count) need to be padded before args.

@vshymanskyy
Copy link
Member Author

vshymanskyy commented Apr 9, 2021 via email

soundandform added a commit that referenced this issue Apr 9, 2021
@petersalomonsen
Copy link
Contributor

Awesome that you now can play the midisynth 😀

@vshymanskyy
Copy link
Member Author

vshymanskyy commented Apr 11, 2021

@soundandform please check pygame-maze.py example. This one now fails with [trap] invalid conversion to integer. it's not related to arguments layout, as it only runs a single void(void) function.
Also, pywasm3 uses only the public Wasm3 API (those put_arg_on_stack and get_arg_from_stack functions are aptly named).

@vshymanskyy
Copy link
Member Author

@soundandform nevermind, figured it out. The issue was on pywasm3 side, in the generic RawFunction handler.

@vshymanskyy
Copy link
Member Author

I think we can close this, all of the found issues were covered by tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

3 participants