[wasm][fuzzer] Fix detection of traps

Instead of returning 0xDEADBEEF, return a struct with proper
information. Otherwise a function returning 0xDEADBEEF would be
misidentified as trapping in the interpreter.

R=ahaas@chromium.org

Bug: chromium:906997
Change-Id: I92fc3a9972d76d2f8a5b313bf6be6eb027cfc1e9
Reviewed-on: https://chromium-review.googlesource.com/c/1344111Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57658}

test/common/wasm/wasm-module-runner.cc

@@ -160,10 +160,9 @@ int32_t CompileAndRunAsmWasmModule(Isolate* isolate, const byte* module_start,
   return RunWasmModuleForTesting(isolate, instance.ToHandleChecked(), 0,
                                  nullptr);
 }
-int32_t InterpretWasmModule(Isolate* isolate,
-                            Handle<WasmInstanceObject> instance,
-                            ErrorThrower* thrower, int32_t function_index,
-                            WasmValue* args, bool* possible_nondeterminism) {
+WasmInterpretationResult InterpretWasmModule(
+    Isolate* isolate, Handle<WasmInstanceObject> instance,
+    int32_t function_index, WasmValue* args) {
   // Don't execute more than 16k steps.
   constexpr int kMaxNumSteps = 16 * 1024;
 
@@ -186,17 +185,19 @@ int32_t InterpretWasmModule(Isolate* isolate,
   bool stack_overflow = isolate->has_pending_exception();
   isolate->clear_pending_exception();
 
-  *possible_nondeterminism = thread->PossibleNondeterminism();
-  if (stack_overflow) return 0xDEADBEEF;
+  if (stack_overflow) return WasmInterpretationResult::Stopped();
 
-  if (thread->state() == WasmInterpreter::TRAPPED) return 0xDEADBEEF;
+  if (thread->state() == WasmInterpreter::TRAPPED) {
+    return WasmInterpretationResult::Trapped(thread->PossibleNondeterminism());
+  }
 
-  if (interpreter_result == WasmInterpreter::FINISHED)
-    return thread->GetReturnValue().to<int32_t>();
+  if (interpreter_result == WasmInterpreter::FINISHED) {
+    return WasmInterpretationResult::Finished(
+        thread->GetReturnValue().to<int32_t>(),
+        thread->PossibleNondeterminism());
+  }
 
-  thrower->RangeError(
-      "Interpreter did not finish execution within its step bound");
-  return -1;
+  return WasmInterpretationResult::Stopped();
 }
 
 MaybeHandle<WasmExportedFunction> GetExportedFunction(

test/common/wasm/wasm-module-runner.h

@@ -61,13 +61,48 @@ int32_t CompileAndRunWasmModule(Isolate* isolate, const byte* module_start,
 MaybeHandle<WasmInstanceObject> CompileAndInstantiateForTesting(
     Isolate* isolate, ErrorThrower* thrower, const ModuleWireBytes& bytes);
 
+class WasmInterpretationResult {
+ public:
+  static WasmInterpretationResult Stopped() { return {kStopped, 0, false}; }
+  static WasmInterpretationResult Trapped(bool possible_nondeterminism) {
+    return {kTrapped, 0, possible_nondeterminism};
+  }
+  static WasmInterpretationResult Finished(int32_t result,
+                                           bool possible_nondeterminism) {
+    return {kFinished, result, possible_nondeterminism};
+  }
+
+  bool stopped() const { return status_ == kStopped; }
+  bool trapped() const { return status_ == kTrapped; }
+  bool finished() const { return status_ == kFinished; }
+
+  int32_t result() const {
+    DCHECK_EQ(status_, kFinished);
+    return result_;
+  }
+
+  bool possible_nondeterminism() const { return possible_nondeterminism_; }
+
+ private:
+  enum Status { kFinished, kTrapped, kStopped };
+
+  const Status status_;
+  const int32_t result_;
+  const bool possible_nondeterminism_;
+
+  WasmInterpretationResult(Status status, int32_t result,
+                           bool possible_nondeterminism)
+      : status_(status),
+        result_(result),
+        possible_nondeterminism_(possible_nondeterminism) {}
+};
+
 // Interprets the given module, starting at the function specified by
 // {function_index}. The return type of the function has to be int32. The module
 // should not have any imports or exports
-int32_t InterpretWasmModule(Isolate* isolate,
-                            Handle<WasmInstanceObject> instance,
-                            ErrorThrower* thrower, int32_t function_index,
-                            WasmValue* args, bool* possible_nondeterminism);
+WasmInterpretationResult InterpretWasmModule(
+    Isolate* isolate, Handle<WasmInstanceObject> instance,
+    int32_t function_index, WasmValue* args);
 
 // Runs the module instance with arguments.
 int32_t RunWasmModuleForTesting(Isolate* isolate,

test/fuzzer/wasm-fuzzer-common.cc

@@ -316,29 +316,22 @@ int WasmExecutionFuzzer::FuzzWasmModule(Vector<const uint8_t> data,
 
   if (!compiles) return 0;
 
-  int32_t result_interpreter;
-  bool possible_nondeterminism = false;
-  {
-    MaybeHandle<WasmInstanceObject> interpreter_instance =
-        i_isolate->wasm_engine()->SyncInstantiate(
-            i_isolate, &interpreter_thrower, compiled_module.ToHandleChecked(),
-            MaybeHandle<JSReceiver>(), MaybeHandle<JSArrayBuffer>());
+  MaybeHandle<WasmInstanceObject> interpreter_instance =
+      i_isolate->wasm_engine()->SyncInstantiate(
+          i_isolate, &interpreter_thrower, compiled_module.ToHandleChecked(),
+          MaybeHandle<JSReceiver>(), MaybeHandle<JSArrayBuffer>());
 
-    // Ignore instantiation failure.
-    if (interpreter_thrower.error()) {
-      return 0;
-    }
+  // Ignore instantiation failure.
+  if (interpreter_thrower.error()) return 0;
 
-    result_interpreter = testing::InterpretWasmModule(
-        i_isolate, interpreter_instance.ToHandleChecked(), &interpreter_thrower,
-        0, interpreter_args.get(), &possible_nondeterminism);
-  }
+  testing::WasmInterpretationResult interpreter_result =
+      testing::InterpretWasmModule(i_isolate,
+                                   interpreter_instance.ToHandleChecked(), 0,
+                                   interpreter_args.get());
 
   // Do not execute the generated code if the interpreter did not finished after
   // a bounded number of steps.
-  if (interpreter_thrower.error()) {
-    return 0;
-  }
+  if (interpreter_result.stopped()) return 0;
 
   // The WebAssembly spec allows the sign bit of NaN to be non-deterministic.
   // This sign bit can make the difference between an infinite loop and
@@ -346,12 +339,7 @@ int WasmExecutionFuzzer::FuzzWasmModule(Vector<const uint8_t> data,
   // the generated code will not go into an infinite loop and cause a timeout in
   // Clusterfuzz. Therefore we do not execute the generated code if the result
   // may be non-deterministic.
-  if (possible_nondeterminism) {
-    return 0;
-  }
-
-  bool expect_exception =
-      result_interpreter == static_cast<int32_t>(0xDEADBEEF);
+  if (interpreter_result.possible_nondeterminism()) return 0;
 
   int32_t result_compiled;
   {
@@ -367,13 +355,16 @@ int WasmExecutionFuzzer::FuzzWasmModule(Vector<const uint8_t> data,
         "main", num_args, compiler_args.get());
   }
 
-  if (expect_exception != i_isolate->has_pending_exception()) {
+  if (interpreter_result.trapped() != i_isolate->has_pending_exception()) {
     const char* exception_text[] = {"no exception", "exception"};
-    FATAL("interpreter: %s; compiled: %s", exception_text[expect_exception],
+    FATAL("interpreter: %s; compiled: %s",
+          exception_text[interpreter_result.trapped()],
           exception_text[i_isolate->has_pending_exception()]);
   }
 
-  if (!expect_exception) CHECK_EQ(result_interpreter, result_compiled);
+  if (!interpreter_result.trapped()) {
+    CHECK_EQ(interpreter_result.result(), result_compiled);
+  }
 
   // Cleanup any pending exception.
   i_isolate->clear_pending_exception();