[Bug] Unable compile optimized wasm file at old node.js version

[starua]

From downstream wasm-opt-rs issure

Version: wasm-opt version 112 (version_112)
Command args: -O4 -g
Node.js Version: v8.9.3

Got this error when calling WebAssembly.Module:

CompileError: WebAssembly.Module(): Compiling wasm function #318:loop failed: Invalid opcode @+122854
    at Object.<anonymous> (<Glue File>:695:20)
    at Module._compile (module.js:635:30)
    at Object.Module._extensions..js (module.js:646:10)
    at Module.load (module.js:554:32)
    at tryModuleLoad (module.js:497:12)
    at Function.Module._load (module.js:489:3)
    at Module.require (module.js:579:17)
    at require (internal/module.js:11:18)
    at repl:1:9
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)

Note that upgrade node.js or disable optimize can solve this error.

File Pack: pack.zip

Origin wasm file: binary.wasm
Optimized wasm file: binary.wasm-opt.wasm
wasm-opt binary archive: wasm-opt

PS. neither pass --mvp-features flag nor downgrade to version 110 worked.

[kripken]

Looks like this is due to the sign-extend proposal operations. LLVM and Binaryen have recently flipped them on by default.

But running with --mvp-features should work, and when I test that locally I do see it emit what I'd expect, and it runs ok in node 8.9.3 (well, it gets past that error and runs into not having some wasm-bindgen import, as I'm running with generic JS loading code).

Is there a chance --mvp-features is not being passed to binaryen properly in your invocation? Do you see that flag affect the binary output?

[starua]

Is there a chance --mvp-features is not being passed to binaryen properly in your invocation? Do you see that flag affect the binary output?

Yes i'm sure the flag is passed, here's my command:

~/Downloads/binaryen-version_112/bin/wasm-opt in.wasm -o opt.wasm -O4 -g                        
~/Downloads/binaryen-version_112/bin/wasm-opt in.wasm -o mvp.wasm --mvp-features -O4 -g  

PS.~/Downloads/binaryen-version_112 is unzip from https://github.com/WebAssembly/binaryen/releases/download/version_112/binaryen-version_112-x86_64-linux.tar.gz
But i can't see it affect the out put, opt.wasm and mvp.wasm is totally same.
opt.wasm.gz
I try replace -O4 -g to -O, and those two file still have no difference.

[kripken]

Very strange, it works for me:

$ wasm-opt binary.wasm -o opt.wasm -O4 -g
$ wasm-opt binary.wasm -o mvp.wasm --mvp-features -O4 -g
$ ls -al opt.wasm mvp.wasm
-rw-r--r-- 1 azakai primarygroup 198629 Apr 13 06:58 mvp.wasm
-rw-r--r-- 1 azakai primarygroup 198458 Apr 13 06:57 opt.wasm

I see the same with main and version_112 on my system (both building from source, and the release binaries), so I'm not sure what could be going wrong on your machine.

To debug this, can you build and test main from source? If that still shows the issue, adding a few debug prints will quickly help narrow this down.

[starua]

I tried build from source, and got version wasm-opt version 112 (version_112-130-g34d5e7856), and the mvp file stay the same.
I'm not good at cpp and cmake, so i don't know where should i add/enable debug prints, can you tell how to do that?

PS.I tried opt with version 110, the file is different with those above, but still fail when compiling, is there a chance that we got with wrong problem?
opt-v110.wasm.gz

[kripken]

Adding std::cout << "hello from changed code\n"; in main() in wasm-opt.cpp would be enough to see that you are running that build of wasm-opt.

After that, I'd add debug prints before and after the feature checks for sign-ext in OptimizeInstructions, like here:

binaryen/src/passes/OptimizeInstructions.cpp

Line 498 in edf9087

if (getModule()->features.hasSignExt()) {

If you see that condition being true despite passing --mvp-features then inspecting getModule()->features is what I'd try next.

[starua]

Command: /path/bin/wasm-opt in.wasm -o opt.wasm -O4 -g --mvp-features 1> stdout.txt 2> stderr.txt
pack.zip

[kripken]

Ok, that shows the sign-ext feature is enabled. So the next question is why --mvp-features doesn't disable that.

You can add some logging here to make sure that that flag is received:

binaryen/src/tools/tool-options.h

Line 47 in bd1822d

enabledFeatures.setMVP();

Logging the values around here could verify nothing goes wrong after:

binaryen/src/tools/tool-options.h

Line 185 in bd1822d

}

If that isn't enough, the next thing I'd log is whether the features section is a factor:

binaryen/src/wasm/wasm-binary.cpp

Line 3608 in bd1822d

feature = FeatureSet::SignExt;

(but I don't remember offhand how that works with --mvp-features; @tlively would know best. but, again, hopefully the earlier loggings I mentioned clear things up already).

[starua]

pack.zip

[kripken]

That output suggests there is a features section in your file, which enables that option. The features section can enable and disable features like commandline flags can.

I don't see the features section in binary.wasm in pack.zip from your first comment, so I think we are seeing different results because we are operating on different wasm files. Here is what I get locally when I apply your diff and then run on binary.wasm:

hello from changed code
setMVP running
Changed check, value:0

Anyhow, aside from the confusion in files, the way to fix this for you is to either

1. Tell the toolchain emitting that wasm file to not enable the sign-extension feature.
2. Tell the toolchain emitting that wasm file to not emit the features section.
3. Tell Binaryen to lower away the feature entirely from the wasm file. The --signext-lowering pass will do that (it both removes signext oerations, and disables the flag) but you will need a very recent Binaryen for that, as the pass was only completed about a week ago.

[starua]

Thanks, --signext-lowering fix my problem.