Support Wasm Release 2.0 except SIMD

[doehyunbaek]

I have confirmed the test passing for all the tests in the valid-wasm-binaries.txt.

[doehyunbaek]

As this is a quite low-priority task, closing this.

[danleh]

Opening again, to boost visibility of this fork (which is great!). Eventually, I want to merge this, once I have a bit more time on my hands.

[doehyunbaek]

I just pushed what I use in Wasm-R3 to this branch. This might be a bit more messier but it's more battle-tested in some sense.

If you need a bit more cleaner version of the current fork I am willing to spend a day's evening on this PR, so if you want that to happen just let me know.

[danleh]

Thanks for updating the PR/branch. Two questions/requests before reviewing it further:

1. Could you run cargo fmt on it (preferably with the current version rustfmt 1.7.0-stable, i.e., run rustup update before), so that the diff is minimized.
2. Am I understanding correctly that this also includes, e.g., the JS bigint integration for Wasm i64 passing (instead of long.js/splitting into two i32s?); and support for compiling Wasabi itself to the browser with wasm-pack? Are there some other high-level changes besides the adding of extension support that I might have missed?

Specifically regarding 2: I might cherry pick some individual files/commits, so that I can piece-wise review this, otherwise it's a bit much (hard to understand, test and land).

[doehyunbaek]

Ah yes at its current state it's a mess. I totally agree. I just pushed what I have to make the most recently maintained fork visible.

If you intend to review the changes I will do the work to trim this down to only extending support of wasm release 2.0

[danleh]

Since the Wasm 2.0 extensions is probably the largest chunk, let's do that last and land the other things first:

1. Could you just reformat (nothing else) to minimize the diff (don't make yourself too much work with it...)
2. I'll cherry pick the changes for i64 <-> BigInt conversion
3. I'll cherry pick the wasabi_js changes and examples (although that might be in a half-way broken state at first)
4. Lastly, see what differences remain and review the Wasm 2.0 changes :)

[doehyunbaek]

Ah glad to hear! I am also delighted to see my work gets upstreamed :).

Yeah I also don't wanna do too much work but I kind of want to support SIMD so yeah.. I am a little bit conflicted with these two goals. Maybe I am a little bit too ambitious with this haha.

I'll do my best to deliver the best PR possible while minimizing the time spent. Will update!

[doehyunbaek]

I thought a bit, and to me it seems that it's better to go with SIMD affter we merge other changes first. So will proceed as you suggested.

[danleh]

Agreed. I also wanted to fix a couple small other things here and there, but the more our two branches diverge, the harder the merge will be, so I'll try to do this here first. I have another 1-2 hours this weekend and hopefully a bit more next weekend, so unless it's critical to have SIMD on your end, let's try to land steps 1-3 in the next two weeks 🤞.

[doehyunbaek]

The diff is huge.. I'm kinda worried but I have faith in you. I'm in the middle of the huge refactor of Wasm-R3 also.

One question: would you be interested if I provide a github actions suite using wasm-r3's to test Wasabi? i.e. When updating Wasabi, check if Wasm-r3's unit tests break.

I need them anyway, and I think I am going to use Wasabi at Wasm-R3 for quite long term, so...

[doehyunbaek]

Also, what do you think about switching wasm-validate to wasm-tools validate? For this WasmBench-dataset testcase, it takes a fraction of time and memory.

wasabi/test-inputs/valid-wasm-binaries.txt

Line 366 in d537a83

WasmBench-dataset/filtered/05e8fe0df5b9aa916ce3ccbb4a2ccc3bfc21992ba2164f5a597f8f6e87def7e3.wasm

~/wasabi/test-outputs/add-hooks/WasmBench-dataset/filtered time wasm-validate 05e8fe0df5b9aa916ce3ccbb4a2ccc3bfc21992ba2164f5a597f8f6e87def7e3.wasm

real    0m11.256s
user    0m7.566s
sys     0m3.691s
~/wasabi/test-outputs/add-hooks/WasmBench-dataset/filtered time wasm-tools validate 05e8fe0df5b9aa916ce3ccbb4a2ccc3bfc21992ba2164f5a597f8f6e87def7e3.wasm

real    0m0.327s
user    0m2.137s
sys     0m0.214s

(wasm-validate takes 19.4GB of memory while wasm-tools validate ends so fast so I really can't tell from eye at htop).

Also, wabt is generally falling back in maintenance, I also observed that for some valid wasm files, wasm-validate fails to validate. To reproduce:

~/wasabi/test-inputs/wasm-spec-tests git diff
        if ! wasm-validate "$file"; then
                echo "invalid binary $file, deleting..."
-               rm "$file"
+               # rm "$file"
        else
~/wasabi/test-inputs/wasm-spec-tests bash build.sh 
0000062: error: expected ref.null or ref.func in passive element segment
0000063: error: expected END opcode after element expression
invalid binary build/elem.wasm, deleting...
~/wasabi/test-inputs/wasm-spec-tests wasm-validate build/elem.wasm 
0000062: error: expected ref.null or ref.func in passive element segment
0000063: error: expected END opcode after element expression
~/wasabi/test-inputs/wasm-spec-tests wasm-tools validate build/elem.wasm 
~/wasabi/test-inputs/wasm-spec-tests 

The only downside might be that, contrary to 1.0.27 version of wasm-validate, which does not error on too many locals, wasm-tools validate errors on it; though I am not sure this is really a downside. If wasabi produces a Wasm binary that exceeds the local count of Wasm-tools, I believe it is better to label it as an error.

If you agree about my point, I can change all uses of wasm-validate to wasm-tools validate, at this PR or at another PR.

[doehyunbaek]

I think this PR lacks the required changes to the runtime.js to support new Wasabi instrumentation, though I am not sure what would be the appropriate one yet.

Will try to figure out and update, if you think it's more appropriate to turn this PR into a draft, then do so.

[doehyunbaek]

I think this PR lacks the required changes to the runtime.js to support new Wasabi instrumentation, though I am not sure what would be the appropriate one yet.

I pushed what I have here: 6ba2665 but it is not thoroughly tested yet. If I have more confidence I will update. Sorry for the force push.

I'll cherry pick the changes for i64 <-> BigInt conversion
I'll cherry pick the wasabi_js changes and examples (although that might be in a half-way broken state at first)

I needed them anyway so I did the work and pushed to here: https://github.com/doehyunbaek/wasabi/tree/master

I confirmed cargo test passing, pldi-tutorial task1 working correctly, and npm run test of wasabi_js passing.

One oddity to point out is this line:

doehyunbaek@83f0102#diff-b9dd56b27786e1119956ce0436abcab055141601a9d4ce33f66451b8ffb1bbd2R175

I had to revert the monkey patch, because without it running eval with the JS file monkey patched on top of the monkey patch. I am not sure yet if there is more elegant solution to this.

I will progress on top of this commit and if there's anything to say let me know.