binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Fuzzer: Remove --emit-js-shell logic and reuse fuzz_shell.js instead

Open kripken opened this issue 2 years ago • 0 comments

We had two JS files that could run a wasm file for fuzzing purposes:

  1. --emit-js-shell, which emitted a custom JS file that runs the wasm.
  2. scripts/fuzz_shell.js, which was a generic file that did the same.

Both of those load the wasm and then call the exports in order and print out logging as it goes of their return values (if any), exceptions, etc. Then the fuzzer compares that output to running the same wasm in another VM, etc. The difference is that one was custom for the wasm file, and one was generic. Aside from that they are similar and duplicated a bunch of code.

This PR improves things by removing 1 and using 2 in all places, that is, we now use the generic file everywhere.

I believe we added 1 because we thought a generic file can't do all the things we need, like know the order of exports and the types of return values, but in practice there are ways to do those things: The exports are in fact in the proper order (JS order of iteration is deterministic, thankfully), and for the type we don't want to print type internals anyhow since that would limit fuzzing --closed-world. We do need to be careful with types in JS (see notes in the PR about the type of null) but it's not too bad. As for the types of params, it's fine to pass in null for them all anyhow (null converts to a number or a reference without error).

There are two risks here:

  1. Out of tree users of --emit-js-shell. I'm not aware of any myself.
  2. If new requirements show up later that force us to use more than generic JS. In that case we'd need to write some new code, but it could still be better than the status quo - we could at least build on top of fuzz_shell.js rather than duplicate it.

kripken avatar Feb 15 '24 00:02 kripken