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

…6310)

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

* --emit-js-shell, which emitted a custom JS file that runs the wasm.
* 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).

scripts/fuzz_opt.py

@@ -665,8 +665,11 @@ def run_d8_js(js, args=[], liftoff=True):
     return run_vm(cmd)
 
 
+FUZZ_SHELL_JS = in_binaryen('scripts', 'fuzz_shell.js')
+
+
 def run_d8_wasm(wasm, liftoff=True):
-    return run_d8_js(in_binaryen('scripts', 'fuzz_shell.js'), [wasm], liftoff=liftoff)
+    return run_d8_js(FUZZ_SHELL_JS, [wasm], liftoff=liftoff)
 
 
 def all_disallowed(features):
@@ -746,8 +749,7 @@ class D8:
             name = 'd8'
 
             def run(self, wasm, extra_d8_flags=[]):
-                run([in_bin('wasm-opt'), wasm, '--emit-js-wrapper=' + wasm + '.js'] + FEATURE_OPTS)
-                return run_vm([shared.V8, wasm + '.js'] + shared.V8_OPTS + extra_d8_flags + ['--', wasm])
+                return run_vm([shared.V8, FUZZ_SHELL_JS] + shared.V8_OPTS + extra_d8_flags + ['--', wasm])
 
             def can_run(self, wasm):
                 # INITIAL_CONTENT is disallowed because some initial spec testcases
@@ -1036,7 +1038,8 @@ def fix_number(x):
                 compare_between_vms(before, interpreter, 'Wasm2JS (vs interpreter)')
 
     def run(self, wasm):
-        wrapper = run([in_bin('wasm-opt'), wasm, '--emit-js-wrapper=/dev/stdout'] + FEATURE_OPTS)
+        with open(FUZZ_SHELL_JS) as f:
+            wrapper = f.read()
         cmd = [in_bin('wasm2js'), wasm, '--emscripten']
         # avoid optimizations if we have nans, as we don't handle them with
         # full precision and optimizations can change things

scripts/fuzz_shell.js

@@ -38,15 +38,34 @@ var detrand = (function() {
   };
 })();
 
-// Fuzz integration.
-function logValue(x, y) {
+// Print out a value in a way that works well for fuzzing.
+function printed(x, y) {
   if (typeof y !== 'undefined') {
-    console.log('[LoggingExternalInterface logging ' + x + ' ' + y + ']');
+    // A pair of i32s which are a legalized i64.
+    return x + ' ' + y;
+  } else if (x === null) {
+    // JS has just one null. Print that out rather than typeof null which is
+    // 'object', below.
+    return 'null';
+  } else if (typeof x !== 'number' && typeof x !== 'string') {
+    // Something that is not a number or string, like a reference. We can't
+    // print a reference because it could look different after opts - imagine
+    // that a function gets renamed internally (that is, the problem is that
+    // JS printing will emit some info about the reference and not a stable
+    // external representation of it). In those cases just print the type,
+    // which will be 'object' or 'function'.
+    return typeof x;
   } else {
-    console.log('[LoggingExternalInterface logging ' + x + ']');
+    // A number. Print the whole thing.
+    return '' + x;
   }
 }
 
+// Fuzzer integration.
+function logValue(x, y) {
+  console.log('[LoggingExternalInterface logging ' + printed(x, y) + ']');
+}
+
 // Set up the imports.
 var imports = {
   'fuzzing-support': {
@@ -94,21 +113,25 @@ function refreshView() {
 }
 
 // Run the wasm.
-var sortedExports = [];
 for (var e in exports) {
-  sortedExports.push(e);
-}
-sortedExports.sort();
-sortedExports.forEach(function(e) {
-  if (typeof exports[e] !== 'function') return;
+  if (typeof exports[e] !== 'function') {
+    continue;
+  }
+  // Send the function a null for each parameter. Null can be converted without
+  // error to both a number and a reference.
+  var func = exports[e];
+  var args = [];
+  for (var i = 0; i < func.length; i++) {
+    args.push(null);
+  }
   try {
     console.log('[fuzz-exec] calling ' + e);
-    var result = exports[e]();
+    var result = func.apply(null, args);
     if (typeof result !== 'undefined') {
-      console.log('[fuzz-exec] note result: $' + e + ' => ' + result);
+      console.log('[fuzz-exec] note result: ' + e + ' => ' + printed(result));
     }
   } catch (e) {
     console.log('exception!');// + [e, e.stack]);
   }
-});
+}
 

src/tools/execution-results.h

@@ -119,8 +119,18 @@ struct ExecutionResults {
             if (resultType.isRef() && !resultType.isString()) {
               // Don't print reference values, as funcref(N) contains an index
               // for example, which is not guaranteed to remain identical after
-              // optimizations.
-              std::cout << resultType << '\n';
+              // optimizations. Do not print the type in detail (as even that
+              // may change due to closed-world optimizations); just print a
+              // simple type like JS does, 'object' or 'function', but also
+              // print null for a null (so a null function does not get
+              // printed as object, as in JS we have typeof null == 'object').
+              if (values->size() == 1 && (*values)[0].isNull()) {
+                std::cout << "null\n";
+              } else if (resultType.isFunction()) {
+                std::cout << "function\n";
+              } else {
+                std::cout << "object\n";
+              }
             } else {
               // Non-references can be printed in full. So can strings, since we
               // always know how to print them and there is just one string

src/tools/wasm-opt.cpp

@@ -23,7 +23,6 @@
 
 #include "execution-results.h"
 #include "fuzzing.h"
-#include "js-wrapper.h"
 #include "optimization-options.h"
 #include "pass.h"
 #include "shell-interface.h"
@@ -86,7 +85,6 @@ int main(int argc, const char* argv[]) {
   bool fuzzPasses = false;
   bool fuzzMemory = true;
   bool fuzzOOB = true;
-  std::string emitJSWrapper;
   std::string emitSpecWrapper;
   std::string emitWasm2CWrapper;
   std::string inputSourceMapFilename;
@@ -180,15 +178,6 @@ int main(int argc, const char* argv[]) {
          WasmOptOption,
          Options::Arguments::Zero,
          [&](Options* o, const std::string& arguments) { fuzzOOB = false; })
-    .add("--emit-js-wrapper",
-         "-ejw",
-         "Emit a JavaScript wrapper file that can run the wasm with some test "
-         "values, useful for fuzzing",
-         WasmOptOption,
-         Options::Arguments::One,
-         [&](Options* o, const std::string& arguments) {
-           emitJSWrapper = arguments;
-         })
     .add("--emit-spec-wrapper",
          "-esw",
          "Emit a wasm spec interpreter wrapper file that can run the wasm with "
@@ -329,24 +318,11 @@ int main(int argc, const char* argv[]) {
     }
   }
 
-  if (emitJSWrapper.size() > 0) {
-    // As the code will run in JS, we must legalize it.
-    PassRunner runner(&wasm);
-    runner.add("legalize-js-interface");
-    runner.run();
-  }
-
   ExecutionResults results;
   if (fuzzExecBefore) {
     results.get(wasm);
   }
 
-  if (emitJSWrapper.size() > 0) {
-    std::ofstream outfile;
-    outfile.open(wasm::Path::to_path(emitJSWrapper), std::ofstream::out);
-    outfile << generateJSWrapper(wasm);
-    outfile.close();
-  }
   if (emitSpecWrapper.size() > 0) {
     std::ofstream outfile;
     outfile.open(wasm::Path::to_path(emitSpecWrapper), std::ofstream::out);

test/lit/exec/no-compare-refs.wast

@@ -12,11 +12,11 @@
   ;; The type of the reference this function returns will change as a result of
   ;; signature pruning. The fuzzer should not complain about this.
   ;; CHECK:      [fuzz-exec] calling return-ref
-  ;; CHECK-NEXT: [fuzz-exec] note result: return-ref => funcref
+  ;; CHECK-NEXT: [fuzz-exec] note result: return-ref => function
   (func $return-ref (export "return-ref") (result funcref)
     (ref.func $no-use-param)
   )
 )
 ;; CHECK:      [fuzz-exec] calling return-ref
-;; CHECK-NEXT: [fuzz-exec] note result: return-ref => funcref
+;; CHECK-NEXT: [fuzz-exec] note result: return-ref => function
 ;; CHECK-NEXT: [fuzz-exec] comparing return-ref

test/lit/help/wasm-opt.test

@@ -53,10 +53,6 @@
 ;; CHECK-NEXT:                                                 loads/stores/indirect calls when
 ;; CHECK-NEXT:                                                 fuzzing
 ;; CHECK-NEXT:
-;; CHECK-NEXT:   --emit-js-wrapper,-ejw                        Emit a JavaScript wrapper file
-;; CHECK-NEXT:                                                 that can run the wasm with some
-;; CHECK-NEXT:                                                 test values, useful for fuzzing
-;; CHECK-NEXT:
 ;; CHECK-NEXT:   --emit-spec-wrapper,-esw                      Emit a wasm spec interpreter
 ;; CHECK-NEXT:                                                 wrapper file that can run the
 ;; CHECK-NEXT:                                                 wasm with some test values,

test/lit/unicode-filenames.wast

@@ -1,6 +1,6 @@
 ;; RUN: wasm-as %s -o %t-❤.wasm --source-map %t-🗺️.map
 ;; RUN: cat %t-🗺️.map | filecheck %s --check-prefix SOURCEMAP
-;; RUN: wasm-opt %t-❤.wasm -o %t-🤬.wasm --emit-js-wrapper %t-❤.js --input-source-map %t-🗺️.map --output-source-map %t-🗺️.out.map
+;; RUN: wasm-opt %t-❤.wasm -o %t-🤬.wasm --emit-spec-wrapper %t-❤.js --input-source-map %t-🗺️.map --output-source-map %t-🗺️.out.map
 ;; RUN: cat %t-🗺️.out.map | filecheck %s --check-prefix SOURCEMAP
 ;; RUN: wasm-dis %t-🤬.wasm | filecheck %s --check-prefix MODULE