brson / miri

An experimental compiler from Rust to WebAssembly (inactive - do not use)
Apache License 2.0
209 stars 15 forks source link

Add a test compile/run pass exercizing optimized generation #47

Open lqd opened 7 years ago

lqd commented 7 years ago

I think some of our tests generate invalid wasm modules, even though they still pass.

Some cases were known to fail in the browsers' stricter validation but are now also tripping binaryen, like non-void returns for the unit type (surely the reason for this in @eholk's V8 PR https://github.com/brson/mir2wasm/pull/42/files#diff-ed1805005a6effb6c15eeed860cdd597R313).

But they are hard to see because binaryen validation is not as strong with more complex code. Asking it to optimize the module first, often makes buggy trans visible when validating the simpler module. (More likely than binaryen optimizing incorrectly per se)

We should probably add test passes that launch mir2wasm with the -O flag to see those cases.

lqd commented 7 years ago

Also IIRC the run-pass can more easily catch traps, rather than problems of correctness per se (I don't remember how rustc does it, maybe just checking the ouput of running the compiled test)

Here's a summary of the situation, compiling both without and with binaryen optimizations, and interpreting the resulting modules (with the caveats about "no errors" that of course binaryen can optimize out most of such trivial programs, or, you never know, some of its optimizations could be problematic -- like I said earlier, probably not :smiley: )

1) tests/compile-pass:

2) tests/compile-fail:

kripken commented 7 years ago

Btw, anything that validates and then fails to validate when binaryen optimizes is definitely a binaryen bug. I've been doing a lot of fuzzing recently so things should be getting stable, but bugs are still very possible ;) Please file any you find.

lqd commented 7 years ago

@kripken alright :) I've tried to add the C trace and wast files to https://github.com/WebAssembly/binaryen/issues/738

lqd commented 7 years ago

Upgrading to binaryen master in #48 fixes the couple inconsistent cases not compiling with -O (trivial.rs and nocore-hello-world.rs). I'll recheck the other examples and tests and update the summary.