Compare binaryen fuzz-exec to JS VMs (#1856)

The main fuzz_opt.py script compares JS VMs, and separately runs binaryen's fuzz-exec that compares the binaryen interpreter to itself (before and after opts). This PR lets us directly compare binaryen's interpreter output to JS VMs. This found a bunch of minor things we can do better on both sides, giving more fuzz coverage.

To enable this, a bunch of tiny fixes were needed:

*    Add --fuzz-exec-before which is like --fuzz-exec but just runs the code before opts are run, instead of before and after.
*    Normalize double printing (so JS and C++ print comparable things). This includes negative zero in JS, which we never printed properly til now.
*    Various improvements to how we print fuzz-exec logging - remove unuseful things, and normalize the others across JS and C++.
*    Properly legalize the wasm when --emit-js-wrapper (i.e., we will run the code from JS), and use that in the JS wrapper code.

check.py

@@ -354,7 +354,7 @@ def check_expected(actual, expected):
           expected = open(expected).read()
 
           # fix it up, our pretty (i32.const 83) must become compared to a homely 83 : i32
-          def fix(x):
+          def fix_expected(x):
             x = x.strip()
             if not x:
               return x
@@ -363,7 +363,13 @@ def fix(x):
               v = v[:-1]  # remove trailing '.'
             return '(' + t + '.const ' + v + ')'
 
-          expected = '\n'.join(map(fix, expected.split('\n')))
+          def fix_actual(x):
+            if '[trap ' in x:
+              return ''
+            return x
+
+          expected = '\n'.join(map(fix_expected, expected.split('\n')))
+          actual = '\n'.join(map(fix_actual, actual.split('\n')))
           print '       (using expected output)'
           actual = actual.strip()
           expected = expected.strip()

scripts/fuzz_opt.py

@@ -17,6 +17,7 @@
 import difflib
 import subprocess
 import random
+import re
 import shutil
 import time
 
@@ -64,85 +65,88 @@ def randomize_pass_debug():
 IGNORE = '[binaryen-fuzzer-ignore]'
 
 
-def test_one(infile, opts):
-  def compare(x, y, comment):
-    if x != y and x != IGNORE and y != IGNORE:
-      message = ''.join([a.rstrip() + '\n' for a in difflib.unified_diff(x.split('\n'), y.split('\n'), fromfile='expected', tofile='actual')])
-      raise Exception(str(comment) + ": Expected to have '%s' == '%s', diff:\n\n%s" % (
-        x, y,
-        message
-      ))
-
-  def run_vms(prefix):
-    def fix_output(out):
-      # exceptions may differ when optimizing, but an exception should occur. so ignore their types
-      # also js engines print them out slightly differently
-      return '\n'.join(map(lambda x: '   *exception*' if 'exception' in x else x, out.split('\n')))
-
-      # normalize different vm output
-      # also the binaryen optimizer can reorder traps (but not remove them), so
-      # it really just matters if you trap, not how you trap
-      return out.replace('unreachable executed', 'unreachable') \
-                .replace('integer result unrepresentable', 'integer overflow') \
-                .replace('invalid conversion to integer', 'integer overflow') \
-                .replace('memory access out of bounds', 'index out of bounds') \
-                .replace('integer divide by zero', 'divide by zero') \
-                .replace('integer remainder by zero', 'remainder by zero') \
-                .replace('remainder by zero', 'divide by zero') \
-                .replace('divide result unrepresentable', 'integer overflow') \
-                .replace('divide by zero', 'integer overflow') \
-                .replace('index out of bounds', 'integer overflow') \
-                .replace('out of bounds memory access', 'integer overflow')
-
-    def fix_spec_output(out):
-      out = fix_output(out)
-      # spec shows a pointer when it traps, remove that
-      out = '\n'.join(map(lambda x: x if 'runtime trap' not in x else x[x.find('runtime trap'):], out.split('\n')))
-      # https://github.com/WebAssembly/spec/issues/543 , float consts are messed up
-      out = '\n'.join(map(lambda x: x if 'f32' not in x and 'f64' not in x else '', out.split('\n')))
-      return out
-
-    def run_vm(cmd):
-      # ignore some vm assertions, if bugs have already been filed
-      known_issues = [
-        'local count too large',  # ignore this; can be caused by flatten, ssa, etc. passes
-        'liftoff-assembler.cc, line 239\n',  # https://bugs.chromium.org/p/v8/issues/detail?id=8631
-        'liftoff-register.h, line 86\n',  # https://bugs.chromium.org/p/v8/issues/detail?id=8632
-      ]
-      try:
-        return run(cmd)
-      except:
-        output = run_unchecked(cmd)
-        for issue in known_issues:
-          if issue in output:
-            return IGNORE
-        raise
-
-    results = []
-    # append to this list to add results from VMs
-    results += [fix_output(run_vm([os.path.expanduser('d8'), prefix + 'js', '--', prefix + 'wasm']))]
-    results += [fix_output(run_vm([os.path.expanduser('d8-debug'), '--wasm-tier-up', prefix + 'js', '--', prefix + 'wasm']))]
-    results += [fix_output(run_vm([os.path.expanduser('d8-debug'), '--no-wasm-tier-up', prefix + 'js', '--', prefix + 'wasm']))]
-    # spec has no mechanism to not halt on a trap. so we just check until the first trap, basically
-    # run(['../spec/interpreter/wasm', prefix + 'wasm'])
-    # results += [fix_spec_output(run_unchecked(['../spec/interpreter/wasm', prefix + 'wasm', '-e', open(prefix + 'wat').read()]))]
-
-    if len(results) == 0:
-      results = [0]
-
-    first = results[0]
-    for i in range(len(results)):
-      compare(first, results[i], 'comparing between vms at ' + str(i))
-
-    return results
+def compare(x, y, comment):
+  if x != y and x != IGNORE and y != IGNORE:
+    message = ''.join([a.rstrip() + '\n' for a in difflib.unified_diff(x.split('\n'), y.split('\n'), fromfile='expected', tofile='actual')])
+    raise Exception(str(comment) + ": Expected to have '%s' == '%s', diff:\n\n%s" % (
+      x, y,
+      message
+    ))
+
+
+def run_vms(prefix):
+  def fix_output(out):
+    # large doubles may print slightly different on different VMs
+    def fix_double(x):
+      x = x.group(1)
+      if 'nan' in x or 'NaN' in x:
+        x = 'nan'
+      else:
+        x = x.replace('Infinity', 'inf')
+        x = str(float(x))
+      return 'f64.const ' + x
+    out = re.sub(r'f64\.const (-?[nanN:abcdefxIity\d+-.]+)', fix_double, out)
+
+    # mark traps from wasm-opt as exceptions, even though they didn't run in a vm
+    out = out.replace('[trap ', 'exception: [trap ')
+
+    # exceptions may differ when optimizing, but an exception should occur. so ignore their types
+    # also js engines print them out slightly differently
+    return '\n'.join(map(lambda x: '   *exception*' if 'exception' in x else x, out.split('\n')))
+
+  def fix_spec_output(out):
+    out = fix_output(out)
+    # spec shows a pointer when it traps, remove that
+    out = '\n'.join(map(lambda x: x if 'runtime trap' not in x else x[x.find('runtime trap'):], out.split('\n')))
+    # https://github.com/WebAssembly/spec/issues/543 , float consts are messed up
+    out = '\n'.join(map(lambda x: x if 'f32' not in x and 'f64' not in x else '', out.split('\n')))
+    return out
+
+  def run_vm(cmd):
+    # ignore some vm assertions, if bugs have already been filed
+    known_issues = [
+      'local count too large',  # ignore this; can be caused by flatten, ssa, etc. passes
+      'liftoff-assembler.cc, line 239\n',  # https://bugs.chromium.org/p/v8/issues/detail?id=8631
+      'liftoff-assembler.cc, line 245\n',  # https://bugs.chromium.org/p/v8/issues/detail?id=8631
+      'liftoff-register.h, line 86\n',  # https://bugs.chromium.org/p/v8/issues/detail?id=8632
+    ]
+    try:
+      return run(cmd)
+    except:
+      output = run_unchecked(cmd)
+      for issue in known_issues:
+        if issue in output:
+          return IGNORE
+      raise
+
+  results = []
+  # append to this list to add results from VMs
+  results += [fix_output(run_vm([in_bin('wasm-opt'), prefix + 'wasm', '--fuzz-exec-before']))]
+  results += [fix_output(run_vm([os.path.expanduser('d8'),       '--experimental-wasm-sat_f2i_conversions',                      prefix + 'js', '--', prefix + 'wasm']))]
+  results += [fix_output(run_vm([os.path.expanduser('d8-debug'), '--experimental-wasm-sat_f2i_conversions', '--wasm-tier-up',    prefix + 'js', '--', prefix + 'wasm']))]
+  results += [fix_output(run_vm([os.path.expanduser('d8-debug'), '--experimental-wasm-sat_f2i_conversions', '--no-wasm-tier-up', prefix + 'js', '--', prefix + 'wasm']))]
+  # spec has no mechanism to not halt on a trap. so we just check until the first trap, basically
+  # run(['../spec/interpreter/wasm', prefix + 'wasm'])
+  # results += [fix_spec_output(run_unchecked(['../spec/interpreter/wasm', prefix + 'wasm', '-e', open(prefix + 'wat').read()]))]
+
+  if len(results) == 0:
+    results = [0]
+
+  first = results[0]
+  for i in range(len(results)):
+    compare(first, results[i], 'comparing between vms at ' + str(i))
+
+  return results
+
 
+def test_one(infile, opts):
   randomize_pass_debug()
 
   bytes = 0
 
   # fuzz vms
   # gather VM outputs on input file
-  run([in_bin('wasm-opt'), infile, '-ttf', '--emit-js-wrapper=a.js', '--emit-spec-wrapper=a.wat', '-o', 'a.wasm', '--mvp-features'])
+  run([in_bin('wasm-opt'), infile, '-ttf', '--emit-js-wrapper=a.js', '--emit-spec-wrapper=a.wat', '-o', 'a.wasm', '--mvp-features', '--enable-nontrapping-float-to-int'])
   wasm_size = os.stat('a.wasm').st_size
   bytes += wasm_size
   print('pre js size :', os.stat('a.js').st_size, ' wasm size:', wasm_size)

src/shell-interface.h

@@ -200,7 +200,7 @@ struct ShellExternalInterface : ModuleInstance::ExternalInterface {
   }
 
   void trap(const char* why) override {
-    std::cerr << "[trap " << why << "]\n";
+    std::cout << "[trap " << why << "]\n";
     throw TrapException();
   }
 };

src/tools/execution-results.h

@@ -26,20 +26,22 @@ namespace wasm {
 
 typedef std::vector<Literal> Loggings;
 
-// Logs every single import call parameter.
+// Logs every relevant import call parameter.
 struct LoggingExternalInterface : public ShellExternalInterface {
   Loggings& loggings;
 
   LoggingExternalInterface(Loggings& loggings) : loggings(loggings) {}
 
   Literal callImport(Function* import, LiteralList& arguments) override {
-    std::cout << "[LoggingExternalInterface logging";
-    loggings.push_back(Literal()); // buffer with a None between calls
-    for (auto argument : arguments) {
-      std::cout << ' ' << argument;
-      loggings.push_back(argument);
+    if (import->module == "fuzzing-support") {
+      std::cout << "[LoggingExternalInterface logging";
+      loggings.push_back(Literal()); // buffer with a None between calls
+      for (auto argument : arguments) {
+        std::cout << ' ' << argument;
+        loggings.push_back(argument);
+      }
+      std::cout << "]\n";
     }
-    std::cout << "]\n";
     return Literal();
   }
 };
@@ -60,21 +62,23 @@ struct ExecutionResults {
       // execute all exported methods (that are therefore preserved through opts)
       for (auto& exp : wasm.exports) {
         if (exp->kind != ExternalKind::Function) continue;
+        std::cout << "[fuzz-exec] calling " << exp->name << "\n";
         auto* func = wasm.getFunction(exp->value);
         if (func->result != none) {
           // this has a result
           results[exp->name] = run(func, wasm, instance);
-          std::cout << "[fuzz-exec] note result: " << exp->name << " => " << results[exp->name] << '\n';
+          // ignore the result if we hit an unreachable and returned no value
+          if (isConcreteType(results[exp->name].type)) {
+            std::cout << "[fuzz-exec] note result: " << exp->name << " => " << results[exp->name] << '\n';
+          }
         } else {
           // no result, run it anyhow (it might modify memory etc.)
           run(func, wasm, instance);
-          std::cout << "[fuzz-exec] no result for void func: " << exp->name << '\n';
         }
       }
     } catch (const TrapException&) {
       // may throw in instance creation (init of offsets)
     }
-    std::cout << "[fuzz-exec] " << results.size() << " results noted\n";
   }
 
   // get current results and check them against previous ones
@@ -85,7 +89,6 @@ struct ExecutionResults {
       std::cout << "[fuzz-exec] optimization passes changed execution results";
       abort();
     }
-    std::cout << "[fuzz-exec] " << results.size() << " results match\n";
   }
 
   bool operator==(ExecutionResults& other) {
@@ -127,8 +130,8 @@ struct ExecutionResults {
     try {
       LiteralList arguments;
       // init hang support, if present
-      if (wasm.getFunctionOrNull("hangLimitInitializer")) {
-        instance.callFunction("hangLimitInitializer", arguments);
+      if (auto* ex = wasm.getExportOrNull("hangLimitInitializer")) {
+        instance.callFunction(ex->value, arguments);
       }
       // call the method
       for (Type param : func->params) {

src/tools/js-wrapper.h

@@ -22,10 +22,6 @@
 namespace wasm {
 
 static std::string generateJSWrapper(Module& wasm) {
-  PassRunner runner(&wasm);
-  runner.add("legalize-js-interface");
-  runner.run();
-
   std::string ret;
   ret += "if (typeof console === 'undefined') {\n"
          "  console = { log: print };\n"
@@ -49,12 +45,26 @@ static std::string generateJSWrapper(Module& wasm) {
          "    binary = read(args[0], 'binary');\n"
          "  }\n"
          "}\n"
+         "function literal(x, type) {\n"
+         "  var ret = type + '.const ';\n"
+         "  switch (type) {\n"
+         "    case 'i32': ret += (x | 0); break;\n"
+         "    case 'f32':\n"
+         "    case 'f64': {\n"
+         "      if (x == 0 && (1 / x) < 0) ret += '-';\n"
+         "      ret += x;\n"
+         "      break;\n"
+         "    }\n"
+         "    default: throw 'what?';\n"
+         "  }\n"
+         "  return ret;\n"
+         "}\n"
          "var instance = new WebAssembly.Instance(new WebAssembly.Module(binary), {\n"
          "  'fuzzing-support': {\n"
-         "    'log-i32': function(x) { console.log('i32: ' + x) },\n"
-         "    'log-i64': function(x, y) { console.log('i64: ' + x + ', ' + y) },\n"
-         "    'log-f32': function(x) { console.log('f32: ' + x) },\n"
-         "    'log-f64': function(x) { console.log('f64: ' + x) }\n"
+         "    'log-i32': function(x)    { console.log('[LoggingExternalInterface logging ' + literal(x, 'i32') + ']') },\n"
+         "    'log-i64': function(x, y) { console.log('[LoggingExternalInterface logging ' + literal(x, 'i32') + ' ' + literal(y, 'i32') + ']') },\n" // legalization: two i32s
+         "    'log-f32': function(x)    { console.log('[LoggingExternalInterface logging ' + literal(x, 'f64') + ']') },\n" // legalization: an f64
+         "    'log-f64': function(x)    { console.log('[LoggingExternalInterface logging ' + literal(x, 'f64') + ']') },\n"
          "  },\n"
          "  'env': {\n"
          "    'setTempRet0': function(x) { tempRet0 = x },\n"
@@ -64,40 +74,38 @@ static std::string generateJSWrapper(Module& wasm) {
   for (auto& exp : wasm.exports) {
     auto* func = wasm.getFunctionOrNull(exp->value);
     if (!func) continue; // something exported other than a function
-    auto bad = false; // check for things we can't support
-    for (Type param : func->params) {
-      if (param == i64) bad = true;
-    }
-    if (func->result == i64) bad = true;
-    if (bad) continue;
     ret += "if (instance.exports.hangLimitInitializer) instance.exports.hangLimitInitializer();\n";
     ret += "try {\n";
-    ret += std::string("  console.log('calling: ") + exp->name.str + "');\n";
+    ret += std::string("  console.log('[fuzz-exec] calling $") + exp->name.str + "');\n";
     if (func->result != none) {
-      ret += "  console.log('   result: ' + ";
+      ret += std::string("  console.log('[fuzz-exec] note result: $") + exp->name.str + " => ' + literal(";
+    } else {
+      ret += "  ";
     }
     ret += std::string("instance.exports.") + exp->name.str + "(";
     bool first = true;
     for (Type param : func->params) {
-      WASM_UNUSED(param);
       // zeros in arguments TODO more?
       if (first) {
         first = false;
       } else {
         ret += ", ";
       }
       ret += "0";
+      if (param == i64) {
+        ret += ", 0";
+      }
     }
     ret += ")";
     if (func->result != none) {
-      ret += ")"; // for console.log
+      ret += ", '" + std::string(printType(func->result)) + "'))";
+      // TODO: getTempRet
     }
     ret += ";\n";
     ret += "} catch (e) {\n";
-    ret += "  console.log('   exception: ' + e);\n";
+    ret += "  console.log('exception: ' + e);\n";
     ret += "}\n";
   }
-  ret += "console.log('done.')\n";
   return ret;
 }