[EH] Fuzz catch+rethrow on the JS side (#7287)

Add a parameter to call-export, call-ref, where the first bit says "catch and
rethrow any exception". This has no observable effect in binaryen, but causes us
to use different code paths in VMs (for example, a wasm exception may end up
caught in JS, then thrown in JS; passing wasm exnrefs to JS for throwing is not
possible otherwise, so this is the closest we can get).

Author: kripken

scripts/fuzz_shell.js

@@ -170,14 +170,18 @@ function callFunc(func) {
   return func.apply(null, args);
 }
 
-// Calls a given function in a try-catch, swallowing JS exceptions, and return 1
-// if we did in fact swallow an exception. Wasm traps are not swallowed (see
-// details below).
-/* async */ function tryCall(func) {
+// Calls a given function in a try-catch. Return 1 if an exception was thrown.
+// If |rethrow| is set, and an exception is thrown, it is caught and rethrown.
+// Wasm traps are not swallowed (see details below).
+/* async */ function tryCall(func, rethrow) {
   try {
     /* await */ func();
     return 0;
   } catch (e) {
+    // The exception must exist, and not behave oddly when we access a
+    // property on it. (VM bugs could cause errors here.)
+    e.a;
+
     // We only want to catch exceptions, not wasm traps: traps should still
     // halt execution. Handling this requires different code in wasm2js, so
     // check for that first (wasm2js does not define RuntimeError, so use
@@ -208,7 +212,11 @@ function callFunc(func) {
       }
     }
     // Otherwise, this is a normal exception we want to catch (a wasm
-    // exception, or a conversion error on the wasm/JS boundary, etc.).
+    // exception, or a conversion error on the wasm/JS boundary, etc.). Rethrow
+    // if we were asked to.
+    if (rethrow) {
+      throw e;
+    }
     return 1;
   }
 }
@@ -271,7 +279,7 @@ var imports = {
     'throw': (which) => {
       if (!which) {
         // Throw a JS exception.
-        throw 0;
+        throw new Error('js exception');
       } else {
         // Throw a wasm exception.
         throw new WebAssembly.Exception(wasmTag, [which]);
@@ -287,19 +295,39 @@ var imports = {
     },
 
     // Export operations.
-    'call-export': /* async */ (index) => {
-      /* await */ callFunc(exportList[index].value);
+    'call-export': /* async */ (index, flags) => {
+      var rethrow = flags & 1;
+      if (JSPI) {
+        // TODO: Figure out why JSPI fails here.
+        rethrow = 0;
+      }
+      if (!rethrow) {
+        /* await */ callFunc(exportList[index].value);
+      } else {
+        tryCall(/* async */ () => /* await */ callFunc(exportList[index].value),
+                rethrow);
+      }
     },
     'call-export-catch': /* async */ (index) => {
       return tryCall(/* async */ () => /* await */ callFunc(exportList[index].value));
     },
 
     // Funcref operations.
-    'call-ref': /* async */ (ref) => {
+    'call-ref': /* async */ (ref, flags) => {
       // This is a direct function reference, and just like an export, it must
       // be wrapped for JSPI.
       ref = wrapExportForJSPI(ref);
-      /* await */ callFunc(ref);
+      var rethrow = flags & 1;
+      if (JSPI) {
+        // TODO: Figure out why JSPI fails here.
+        rethrow = 0;
+      }
+      if (!rethrow) {
+        /* await */ callFunc(ref);
+      } else {
+        tryCall(/* async */ () => /* await */ callFunc(ref),
+                rethrow);
+      }
     },
     'call-ref-catch': /* async */ (ref) => {
       ref = wrapExportForJSPI(ref);

src/tools/execution-results.h

@@ -132,6 +132,10 @@ struct LoggingExternalInterface : public ShellExternalInterface {
         return {};
       } else if (import->base == "call-export") {
         callExportAsJS(arguments[0].geti32());
+        // The second argument determines if we should catch and rethrow
+        // exceptions. There is no observable difference in those two modes in
+        // the binaryen interpreter, so we don't need to do anything.
+
         // Return nothing. If we wanted to return a value we'd need to have
         // multiple such functions, one for each signature.
         return {};
@@ -143,9 +147,8 @@ struct LoggingExternalInterface : public ShellExternalInterface {
           return {Literal(int32_t(1))};
         }
       } else if (import->base == "call-ref") {
+        // Similar to call-export*, but with a ref.
         callRefAsJS(arguments[0]);
-        // Return nothing. If we wanted to return a value we'd need to have
-        // multiple such functions, one for each signature.
         return {};
       } else if (import->base == "call-ref-catch") {
         try {
@@ -181,11 +184,13 @@ struct LoggingExternalInterface : public ShellExternalInterface {
   }
 
   void throwJSException() {
-    // JS exceptions contain an externref, which wasm can't read (so the actual
-    // value here does not matter, but it does need to match what the 'throw'
-    // import does in fuzz_shell.js, as the fuzzer will do comparisons).
-    Literal externref = Literal::makeI31(0, Unshared).externalize();
-    Literals arguments = {externref};
+    // JS exceptions contain an externref. Use the same type of value as a JS
+    // exception would have, which is a reference to an object, and which will
+    // print out "object" in the logging from JS. A trivial struct is enough for
+    // us to log the same thing here.
+    auto empty = HeapType(Struct{});
+    auto inner = Literal(std::make_shared<GCData>(empty, Literals{}), empty);
+    Literals arguments = {inner.externalize()};
     auto payload = std::make_shared<ExnData>(jsTag, arguments);
     throwException(WasmException{Literal(payload)});
   }

src/tools/fuzzing/fuzzing.cpp

@@ -851,12 +851,16 @@ void TranslateToFuzzReader::addImportCallingSupport() {
 
   if (choice & 1) {
     // Given an export index, call it from JS.
+    // A second parameter has flags. The first bit determines whether we catch
+    // and rethrow all exceptions. (This ends up giving us the same signature
+    // and behavior as when we do not rethrow, so we just add the flags here
+    // rather than another export.)
     callExportImportName = Names::getValidFunctionName(wasm, "call-export");
     auto func = std::make_unique<Function>();
     func->name = callExportImportName;
     func->module = "fuzzing-support";
     func->base = "call-export";
-    func->type = Signature({Type::i32}, Type::none);
+    func->type = Signature({Type::i32, Type::i32}, Type::none);
     wasm.addFunction(std::move(func));
   }
 
@@ -884,7 +888,10 @@ void TranslateToFuzzReader::addImportCallingSupport() {
       func->name = callRefImportName;
       func->module = "fuzzing-support";
       func->base = "call-ref";
-      func->type = Signature({Type(HeapType::func, Nullable)}, Type::none);
+      // As call-export, there is a flags param that allows us to catch+rethrow
+      // all exceptions.
+      func->type =
+        Signature({Type(HeapType::func, Nullable), Type::i32}, Type::none);
       wasm.addFunction(std::move(func));
     }
 
@@ -1135,7 +1142,13 @@ Expression* TranslateToFuzzReader::makeImportCallCode(Type type) {
     if ((catching && (!exportTarget || oneIn(2))) || (!catching && oneIn(4))) {
       // Most of the time make a non-nullable funcref, to avoid errors.
       auto refType = Type(HeapType::func, oneIn(10) ? Nullable : NonNullable);
-      return builder.makeCall(refTarget, {make(refType)}, type);
+      std::vector<Expression*> args = {make(refType)};
+      if (!catching) {
+        // Only the first bit matters here, so we can send anything (this is
+        // future-proof for later bits, and has no downside now).
+        args.push_back(make(Type::i32));
+      }
+      return builder.makeCall(refTarget, args, type);
     }
   }
 
@@ -1163,7 +1176,16 @@ Expression* TranslateToFuzzReader::makeImportCallCode(Type type) {
     index = builder.makeBinary(
       RemUInt32, index, builder.makeConst(int32_t(maxIndex)));
   }
-  return builder.makeCall(exportTarget, {index}, type);
+
+  // The non-catching variants send a flags argument, which says whether to
+  // catch+rethrow.
+  std::vector<Expression*> args = {index};
+  if (!catching) {
+    // Only the first bit matters here, so we can send anything (this is
+    // future-proof for later bits, and has no downside now).
+    args.push_back(make(Type::i32));
+  }
+  return builder.makeCall(exportTarget, args, type);
 }
 
 Expression* TranslateToFuzzReader::makeImportSleep(Type type) {

test/lit/d8/fuzz_shell_exceptions.wast

@@ -31,7 +31,7 @@
 ;; RUN: v8 %S/../../../scripts/fuzz_shell.js -- %t.wasm | filecheck %s
 ;;
 ;; CHECK: [fuzz-exec] calling throwing-js
-;; CHECK: exception thrown: 0
+;; CHECK: exception thrown: Error: js exception
 ;; CHECK: [fuzz-exec] calling throwing-tag
 ;; CHECK: exception thrown: [object WebAssembly.Exception]
 

test/lit/exec/fuzzing-api.wast

@@ -13,10 +13,10 @@
  (import "fuzzing-support" "table-set" (func $table.set (param i32 funcref)))
  (import "fuzzing-support" "table-get" (func $table.get (param i32) (result funcref)))
 
- (import "fuzzing-support" "call-export" (func $call.export (param i32)))
+ (import "fuzzing-support" "call-export" (func $call.export (param i32 i32)))
  (import "fuzzing-support" "call-export-catch" (func $call.export.catch (param i32) (result i32)))
 
- (import "fuzzing-support" "call-ref" (func $call.ref (param funcref)))
+ (import "fuzzing-support" "call-ref" (func $call.ref (param funcref i32)))
  (import "fuzzing-support" "call-ref-catch" (func $call.ref.catch (param funcref) (result i32)))
 
  (import "fuzzing-support" "sleep" (func $sleep (param i32 i32) (result i32)))
@@ -110,10 +110,32 @@
   ;; At index 0 in the exports we have $logging, so we will do those loggings.
   (call $call.export
    (i32.const 0)
+   ;; First bit unset in the flags means a normal call.
+   (i32.const 0)
+  )
+  ;; At index 999 we have nothing, so we'll error.
+  (call $call.export
+   (i32.const 999)
+   (i32.const 0)
+  )
+ )
+
+ ;; CHECK:      [fuzz-exec] calling export.calling.rethrow
+ ;; CHECK-NEXT: [LoggingExternalInterface logging 42]
+ ;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
+ ;; CHECK-NEXT: [exception thrown: imported-js-tag externref]
+ (func $export.calling.rethrow (export "export.calling.rethrow")
+  ;; As above, but the second param is different.
+  (call $call.export
+   (i32.const 0)
+   ;; First bit set in the flags means a catch+rethrow. There is no visible
+   ;; effect here, but there might be in JS VMs.
+   (i32.const 1)
   )
   ;; At index 999 we have nothing, so we'll error.
   (call $call.export
    (i32.const 999)
+   (i32.const 1)
   )
  )
 
@@ -146,10 +168,31 @@
   ;; This will emit some logging.
   (call $call.ref
    (ref.func $logging)
+   ;; Normal call.
+   (i32.const 0)
+  )
+  ;; This will throw.
+  (call $call.ref
+   (ref.null func)
+   (i32.const 0)
+  )
+ )
+
+ ;; CHECK:      [fuzz-exec] calling ref.calling.rethrow
+ ;; CHECK-NEXT: [LoggingExternalInterface logging 42]
+ ;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
+ ;; CHECK-NEXT: [exception thrown: imported-js-tag externref]
+ (func $ref.calling.rethrow (export "ref.calling.rethrow")
+  ;; As with calling an export, when we set the flags to 1 exceptions are
+  ;; caught and rethrown, but there is no noticeable difference here.
+  (call $call.ref
+   (ref.func $logging)
+   (i32.const 1)
   )
   ;; This will throw.
   (call $call.ref
    (ref.null func)
+   (i32.const 1)
   )
  )
 
@@ -195,6 +238,7 @@
   ;; logging from the function, "12".
   (call $call.ref
    (ref.func $legal)
+   (i32.const 1)
   )
  )
 
@@ -335,7 +379,6 @@
 
  ;; CHECK:      [fuzz-exec] calling do-sleep
  ;; CHECK-NEXT: [fuzz-exec] note result: do-sleep => 42
- ;; CHECK-NEXT: warning: no passes specified, not doing any work
  (func $do-sleep (export "do-sleep") (result i32)
   (call $sleep
    ;; A ridiculous amount of ms, but in the interpreter it is ignored anyhow.
@@ -344,6 +387,24 @@
    (i32.const 42)
   )
  )
+
+ ;; CHECK:      [fuzz-exec] calling return-externref-exception
+ ;; CHECK-NEXT: [fuzz-exec] note result: return-externref-exception => object
+ ;; CHECK-NEXT: warning: no passes specified, not doing any work
+ (func $return-externref-exception (export "return-externref-exception") (result externref)
+  ;; Call JS table.set in a way that throws (on out of bounds). The JS exception
+  ;; is caught and returned from the function, so we can see what it looks like
+  ;; to the fuzzer, which should be "object" (an exception object).
+  (block $block (result externref)
+   (try_table (catch $imported-js-tag $block)
+    (call $table.set
+     (i32.const 99990)
+     (ref.null func)
+    )
+   )
+   (unreachable)
+  )
+ )
 )
 ;; CHECK:      [fuzz-exec] calling logging
 ;; CHECK-NEXT: [LoggingExternalInterface logging 42]
@@ -368,6 +429,11 @@
 ;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
 ;; CHECK-NEXT: [exception thrown: imported-js-tag externref]
 
+;; CHECK:      [fuzz-exec] calling export.calling.rethrow
+;; CHECK-NEXT: [LoggingExternalInterface logging 42]
+;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
+;; CHECK-NEXT: [exception thrown: imported-js-tag externref]
+
 ;; CHECK:      [fuzz-exec] calling export.calling.catching
 ;; CHECK-NEXT: [LoggingExternalInterface logging 42]
 ;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
@@ -379,6 +445,11 @@
 ;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
 ;; CHECK-NEXT: [exception thrown: imported-js-tag externref]
 
+;; CHECK:      [fuzz-exec] calling ref.calling.rethrow
+;; CHECK-NEXT: [LoggingExternalInterface logging 42]
+;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
+;; CHECK-NEXT: [exception thrown: imported-js-tag externref]
+
 ;; CHECK:      [fuzz-exec] calling ref.calling.catching
 ;; CHECK-NEXT: [LoggingExternalInterface logging 42]
 ;; CHECK-NEXT: [LoggingExternalInterface logging 3.14159]
@@ -413,10 +484,14 @@
 
 ;; CHECK:      [fuzz-exec] calling do-sleep
 ;; CHECK-NEXT: [fuzz-exec] note result: do-sleep => 42
+
+;; CHECK:      [fuzz-exec] calling return-externref-exception
+;; CHECK-NEXT: [fuzz-exec] note result: return-externref-exception => object
 ;; CHECK-NEXT: [fuzz-exec] comparing catch-js-tag
 ;; CHECK-NEXT: [fuzz-exec] comparing do-sleep
 ;; CHECK-NEXT: [fuzz-exec] comparing export.calling
 ;; CHECK-NEXT: [fuzz-exec] comparing export.calling.catching
+;; CHECK-NEXT: [fuzz-exec] comparing export.calling.rethrow
 ;; CHECK-NEXT: [fuzz-exec] comparing logging
 ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling
 ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.catching
@@ -426,7 +501,9 @@
 ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.illegal-v128
 ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.legal
 ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.legal-result
+;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.rethrow
 ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.trap
+;; CHECK-NEXT: [fuzz-exec] comparing return-externref-exception
 ;; CHECK-NEXT: [fuzz-exec] comparing table.getting
 ;; CHECK-NEXT: [fuzz-exec] comparing table.setting
 ;; CHECK-NEXT: [fuzz-exec] comparing throwing