Add --deterministic flag to wasm2js, for fuzzing (#2757)

In wasm2js we ignore things that trap in wasm that we can't
really handle, like a load from memory out of bounds would
trap in wasm, but in JS we don't want to emit a bounds check
on each load. So wasm2js focuses on programs that don't
trap.

However, this is annoying in the fuzzer as it turns out that
our behavior for places where wasm would trap was not
deterministic. That is, wasm would trap, wasm2js would not
trap and do behavior X, and wasm2js with optimizations
would also not trap but do behavior Y != X. This produced
false positives in the fuzzer (and might be annoying in
manual debugging too).

As a workaround, this adds a --deterministic flag to wasm2js,
which tries to be deterministic about what it does for cases
where wasm would trap. This handles the case of an int
division by 0 which traps in wasm but without this flag could
have different behavior in wasm2js with or without opts
(see details in the patch).

2 files changed, 31 insertions, 4 deletions

diff --git a/src/tools/wasm2js.cpp b/src/tools/wasm2js.cpp
index e7b62c16a..7f3433bb7 100644
--- a/src/tools/wasm2js.cpp
+++ b/src/tools/wasm2js.cpp
@@ -141,7 +141,7 @@ static void replaceInPlaceIfPossible(Ref target, Ref value) {
   }
 }
 
-static void optimizeJS(Ref ast) {
+static void optimizeJS(Ref ast, Wasm2JSBuilder::Flags flags) {
   // Helpers
 
   auto isBinary = [](Ref node, IString op) {
@@ -262,8 +262,21 @@ static void optimizeJS(Ref ast) {
       node[1]->setString(L_NOT);
       node[3]->setNull();
     } else if (isOrZero(node) || isTrshiftZero(node)) {
-      // Just being different from 0 is enough, casts don't matter.
-      return node[2];
+      // Just being different from 0 is enough, casts don't matter. However,
+      // in deterministic mode we care about corner cases that would trap in
+      // wasm, like an integer divide by zero:
+      //
+      //  if ((1 / 0) | 0)  =>  condition is Infinity | 0 = 0 which is falsey
+      //
+      // while
+      //
+      //  if (1 / 0)  =>  condition is Infinity which is truthy
+      //
+      // Thankfully this is not common, and does not occur on % (1 % 0 is a NaN
+      // which has the right truthiness).
+      if (!(flags.deterministic && isBinary(node[2], DIV))) {
+        return node[2];
+      }
     }
     return node;
   };
@@ -511,7 +524,7 @@ static void emitWasm(Module& wasm,
   Wasm2JSBuilder wasm2js(flags, options);
   auto js = wasm2js.processWasm(&wasm, name);
   if (options.optimizeLevel >= 2) {
-    optimizeJS(js);
+    optimizeJS(js, flags);
   }
   Wasm2JSGlue glue(wasm, output, flags, name);
   glue.emitPre();
@@ -849,6 +862,18 @@ int main(int argc, const char* argv[]) {
       Options::Arguments::Zero,
       [&](Options* o, const std::string& argument) { flags.emscripten = true; })
     .add(
+      "--deterministic",
+      "",
+      "Replace WebAssembly trapping behavior deterministically "
+      "(the default is to not care about what would trap in wasm, like a load "
+      "out of bounds or integer divide by zero; with this flag, we try to be "
+      "deterministic at least in what happens, which might or might not be "
+      "to trap like wasm, but at least should not vary)",
+      Options::Arguments::Zero,
+      [&](Options* o, const std::string& argument) {
+        flags.deterministic = true;
+      })
+    .add(
       "--symbols-file",
       "",
       "Emit a symbols file that maps function indexes to their original names",
diff --git a/src/wasm2js.h b/src/wasm2js.h
index 4db734203..1d8003323 100644
--- a/src/wasm2js.h
+++ b/src/wasm2js.h
@@ -120,10 +120,12 @@ class Wasm2JSBuilder {
 
 public:
   struct Flags {
+    // see wasm2js.cpp for details
     bool debug = false;
     bool pedantic = false;
     bool allowAsserts = false;
     bool emscripten = false;
+    bool deterministic = false;
     std::string symbolsFile;
   };