Fuzzer: Add call-ref, call-ref-catch imports (#7137)

Similar to call-export*, these imports call a wasm function from outside the
module. The difference is that we send a function reference for them to call
(rather than an export index).

This gives more coverage, first by sending a ref from wasm to JS, and also
since we will now try to call anything that is sent. Exports, in comparison,
are filtered by the fuzzer to things that JS can handle, so this may lead to
more traps, but maybe also some new situations. This also leads to adding
more logic to execution-results.h to model JS trapping properly.

fuzz_shell.js is refactored to allow sharing code between call-export* and
call-ref*.

3 files changed, 143 insertions, 40 deletions

diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h
index 25d0c0772..bffc4e8f2 100644
--- a/src/tools/execution-results.h
+++ b/src/tools/execution-results.h
@@ -105,13 +105,25 @@ public:
         tableStore(exportedTable, index, arguments[1]);
         return {};
       } else if (import->base == "call-export") {
-        callExport(arguments[0].geti32());
+        callExportAsJS(arguments[0].geti32());
         // 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-export-catch") {
         try {
-          callExport(arguments[0].geti32());
+          callExportAsJS(arguments[0].geti32());
+          return {Literal(int32_t(0))};
+        } catch (const WasmException& e) {
+          return {Literal(int32_t(1))};
+        }
+      } else if (import->base == "call-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 {
+          callRefAsJS(arguments[0]);
           return {Literal(int32_t(0))};
         } catch (const WasmException& e) {
           return {Literal(int32_t(1))};
@@ -145,7 +157,7 @@ public:
     throwException(WasmException{Literal(payload)});
   }
 
-  Literals callExport(Index index) {
+  Literals callExportAsJS(Index index) {
     if (index >= wasm.exports.size()) {
       // No export.
       throwEmptyException();
@@ -155,20 +167,47 @@ public:
       // No callable export.
       throwEmptyException();
     }
-    auto* func = wasm.getFunction(exp->value);
+    return callFunctionAsJS(exp->value);
+  }
+
+  Literals callRefAsJS(Literal ref) {
+    if (!ref.isFunction()) {
+      // Not a callable ref.
+      throwEmptyException();
+    }
+    return callFunctionAsJS(ref.getFunc());
+  }
 
-    // TODO JS traps on some types on the boundary, which we should behave the
-    // same on. For now, this is not needed because the fuzzer will prune all
-    // non-JS-compatible exports anyhow.
+  // Call a function in a "JS-ey" manner, adding arguments as needed, and
+  // throwing if necessary, the same way JS does.
+  Literals callFunctionAsJS(Name name) {
+    auto* func = wasm.getFunction(name);
 
-    // Send default values as arguments, or trap if we need anything else.
+    // Send default values as arguments, or error if we need anything else.
     Literals arguments;
     for (const auto& param : func->getParams()) {
+      // An i64 param can work from JS, but fuzz_shell provides 0, which errors
+      // on attempts to convert it to BigInt. v128 cannot work at all.
+      if (param == Type::i64 || param == Type::v128) {
+        throwEmptyException();
+      }
       if (!param.isDefaultable()) {
         throwEmptyException();
       }
       arguments.push_back(Literal::makeZero(param));
     }
+
+    // Error on illegal results. Note that this happens, as per JS semantics,
+    // *before* the call.
+    for (const auto& result : func->getResults()) {
+      // An i64 result is fine: a BigInt will be provided. But v128 still
+      // errors.
+      if (result == Type::v128) {
+        throwEmptyException();
+      }
+    }
+
+    // Call the function.
     return instance->callFunction(func->name, arguments);
   }
 
diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h
index a3261ccbe..78219045c 100644
--- a/src/tools/fuzzing.h
+++ b/src/tools/fuzzing.h
@@ -115,6 +115,8 @@ private:
   Name tableSetImportName;
   Name callExportImportName;
   Name callExportCatchImportName;
+  Name callRefImportName;
+  Name callRefCatchImportName;
 
   std::unordered_map<Type, std::vector<Name>> globalsByType;
   std::unordered_map<Type, std::vector<Name>> mutableGlobalsByType;
@@ -244,7 +246,9 @@ private:
   Expression* makeImportThrowing(Type type);
   Expression* makeImportTableGet();
   Expression* makeImportTableSet(Type type);
-  Expression* makeImportCallExport(Type type);
+  // Call either an export or a ref. We do this from a single function to better
+  // control the frequency of each.
+  Expression* makeImportCallCode(Type type);
   Expression* makeMemoryHashLogging();
 
   // Function creation
diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp
index ab200a126..a7f5e0d01 100644
--- a/src/tools/fuzzing/fuzzing.cpp
+++ b/src/tools/fuzzing/fuzzing.cpp
@@ -771,22 +771,33 @@ void TranslateToFuzzReader::addImportLoggingSupport() {
 }
 
 void TranslateToFuzzReader::addImportCallingSupport() {
+  if (wasm.features.hasReferenceTypes() && closedWorld) {
+    // In closed world mode we must *remove* the call-ref* imports, if they
+    // exist in the initial content. These are not valid to call in closed-world
+    // mode as they call function references. (Another solution here would be to
+    // make closed-world issue validation errors on these imports, but that
+    // would require changes to the general-purpose validator.)
+    for (auto& func : wasm.functions) {
+      if (func->imported() && func->module == "fuzzing-support" &&
+          func->base.startsWith("call-ref")) {
+        // Make it non-imported, and with a simple body.
+        func->module = func->base = Name();
+        auto results = func->getResults();
+        func->body =
+          results.isConcrete() ? makeConst(results) : makeNop(Type::none);
+      }
+    }
+  }
+
   // Only add these some of the time, as they inhibit some fuzzing (things like
   // wasm-ctor-eval and wasm-merge are sensitive to the wasm being able to call
-  // its own exports, and to care about the indexes of the exports):
-  //
-  //  0 - none
-  //  1 - call-export
-  //  2 - call-export-catch
-  //  3 - call-export & call-export-catch
-  //  4 - none
-  //  5 - none
-  //
-  auto choice = upTo(6);
-  if (choice >= 4) {
+  // its own exports, and to care about the indexes of the exports).
+  if (oneIn(2)) {
     return;
   }
 
+  auto choice = upTo(16);
+
   if (choice & 1) {
     // Given an export index, call it from JS.
     callExportImportName = Names::getValidFunctionName(wasm, "call-export");
@@ -811,6 +822,34 @@ void TranslateToFuzzReader::addImportCallingSupport() {
     func->type = Signature(Type::i32, Type::i32);
     wasm.addFunction(std::move(func));
   }
+
+  // If the wasm will be used for closed-world testing, we cannot use the
+  // call-ref variants, as mentioned before.
+  if (wasm.features.hasReferenceTypes() && !closedWorld) {
+    if (choice & 4) {
+      // Given an funcref, call it from JS.
+      callRefImportName = Names::getValidFunctionName(wasm, "call-ref");
+      auto func = std::make_unique<Function>();
+      func->name = callRefImportName;
+      func->module = "fuzzing-support";
+      func->base = "call-ref";
+      func->type = Signature({Type(HeapType::func, Nullable)}, Type::none);
+      wasm.addFunction(std::move(func));
+    }
+
+    if (choice & 8) {
+      // Given an funcref, call it from JS and catch all exceptions (similar
+      // to callExportCatch), return 1 if we caught).
+      callRefCatchImportName =
+        Names::getValidFunctionName(wasm, "call-ref-catch");
+      auto func = std::make_unique<Function>();
+      func->name = callRefCatchImportName;
+      func->module = "fuzzing-support";
+      func->base = "call-ref-catch";
+      func->type = Signature(Type(HeapType::func, Nullable), Type::i32);
+      wasm.addFunction(std::move(func));
+    }
+  }
 }
 
 void TranslateToFuzzReader::addImportThrowingSupport() {
@@ -998,27 +1037,48 @@ Expression* TranslateToFuzzReader::makeImportTableSet(Type type) {
     Type::none);
 }
 
-Expression* TranslateToFuzzReader::makeImportCallExport(Type type) {
-  // The none-returning variant just does the call. The i32-returning one
-  // catches any errors and returns 1 when it saw an error. Based on the
-  // variant, pick which to call, and the maximum index to call.
-  Name target;
+Expression* TranslateToFuzzReader::makeImportCallCode(Type type) {
+  // Call code: either an export or a ref. Each has a catching and non-catching
+  // variant. The catching variants return i32, the others none.
+  assert(type == Type::none || type == Type::i32);
+  auto catching = type == Type::i32;
+  auto exportTarget =
+    catching ? callExportCatchImportName : callExportImportName;
+  auto refTarget = catching ? callRefCatchImportName : callRefImportName;
+
+  // We want to call a ref less often, as refs are more likely to error (a
+  // function reference can have arbitrary params and results, including things
+  // that error on the JS boundary; an export is already filtered for such
+  // things in some cases - when we legalize the boundary - and even if not, we
+  // emit lots of void(void) functions - all the invoke_foo functions - that are
+  // safe to call).
+  if (refTarget) {
+    // This matters a lot more in the variants that do *not* catch (in the
+    // catching ones, we just get a result of 1, but when not caught it halts
+    // execution).
+    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);
+    }
+  }
+
+  if (!exportTarget) {
+    // We decided not to emit a call-ref here, due to fear of erroring, and
+    // there is no call-export, so just emit something trivial.
+    return makeTrivial(type);
+  }
+
+  // Pick the maximum export index to call.
   Index maxIndex = wasm.exports.size();
-  if (type == Type::none) {
-    target = callExportImportName;
-  } else if (type == Type::i32) {
-    target = callExportCatchImportName;
-    // This never traps, so we can be less careful, but we do still want to
-    // avoid trapping a lot as executing code is more interesting. (Note that
+  if (type == Type::i32) {
+    // This swallows errors, so we can be less careful, but we do still want to
+    // avoid swallowing a lot as executing code is more interesting. (Note that
     // even though we double here, the risk is not that great: we are still
     // adding functions as we go, so the first half of functions/exports can
     // double here and still end up in bounds by the time we've added them all.)
     maxIndex = (maxIndex + 1) * 2;
-  } else {
-    WASM_UNREACHABLE("bad import.call");
   }
-  // We must have set up the target function.
-  assert(target);
 
   // Most of the time, call a valid export index in the range we picked, but
   // sometimes allow anything at all.
@@ -1027,7 +1087,7 @@ Expression* TranslateToFuzzReader::makeImportCallExport(Type type) {
     index = builder.makeBinary(
       RemUInt32, index, builder.makeConst(int32_t(maxIndex)));
   }
-  return builder.makeCall(target, {index}, type);
+  return builder.makeCall(exportTarget, {index}, type);
 }
 
 Expression* TranslateToFuzzReader::makeMemoryHashLogging() {
@@ -1705,8 +1765,8 @@ Expression* TranslateToFuzzReader::_makeConcrete(Type type) {
     options.add(FeatureSet::Atomics, &Self::makeAtomic);
   }
   if (type == Type::i32) {
-    if (callExportCatchImportName) {
-      options.add(FeatureSet::MVP, &Self::makeImportCallExport);
+    if (callExportCatchImportName || callRefCatchImportName) {
+      options.add(FeatureSet::MVP, &Self::makeImportCallCode);
     }
     options.add(FeatureSet::ReferenceTypes, &Self::makeRefIsNull);
     options.add(FeatureSet::ReferenceTypes | FeatureSet::GC,
@@ -1787,8 +1847,8 @@ Expression* TranslateToFuzzReader::_makenone() {
   if (tableSetImportName) {
     options.add(FeatureSet::ReferenceTypes, &Self::makeImportTableSet);
   }
-  if (callExportImportName) {
-    options.add(FeatureSet::MVP, &Self::makeImportCallExport);
+  if (callExportImportName || callRefImportName) {
+    options.add(FeatureSet::MVP, &Self::makeImportCallCode);
   }
   return (this->*pick(options))(Type::none);
 }