[wasm] Revise compile error messages

This CL revises some of our error messages, and removes unneeded parts
(like "AsyncCompilation: " or "(null): "). It also extends existing
tests to check for the precise error message more thoroughly to detect
changes or nondeterminism earlier.

R=titzer@chromium.org, ahaas@chromium.org

Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Bug: chromium:926311
Change-Id: I1ccfb307d4a61291f4582330152a53fbadd0848f
Reviewed-on: https://chromium-review.googlesource.com/c/1445897
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59296}

src/wasm/module-compiler.cc

@@ -1063,10 +1063,9 @@ void AsyncCompileJob::FinishCompile() {
   FinishModule();
 }
 
-void AsyncCompileJob::AsyncCompileFailed(const char* context,
-                                         const WasmError& error) {
-  ErrorThrower thrower(isolate_, "AsyncCompile");
-  thrower.CompileFailed(context, error);
+void AsyncCompileJob::AsyncCompileFailed(const WasmError& error) {
+  ErrorThrower thrower(isolate_, "WebAssembly.compile()");
+  thrower.CompileFailed(error);
   // {job} keeps the {this} pointer alive.
   std::shared_ptr<AsyncCompileJob> job =
       isolate_->wasm_engine()->RemoveCompileJob(this);
@@ -1117,7 +1116,7 @@ class AsyncCompileJob::CompilationStateCallback {
               job->isolate_->set_context(*job->native_context_);
               WasmError error = Impl(job->native_module_->compilation_state())
                                     ->GetCompileError();
-              return job->AsyncCompileFailed("Async compilation failed", error);
+              return job->AsyncCompileFailed(error);
             }));
 
         break;
@@ -1308,7 +1307,7 @@ class AsyncCompileJob::DecodeFail : public CompileStep {
   void RunInForeground(AsyncCompileJob* job) override {
     TRACE_COMPILE("(1b) Decoding failed.\n");
     // {job_} is deleted in AsyncCompileFailed, therefore the {return}.
-    return job->AsyncCompileFailed("Wasm decoding failed", error_);
+    return job->AsyncCompileFailed(error_);
   }
 };
 

src/wasm/module-compiler.h

@@ -107,7 +107,7 @@ class AsyncCompileJob {
 
   void FinishCompile();
 
-  void AsyncCompileFailed(const char* context, const WasmError&);
+  void AsyncCompileFailed(const WasmError&);
 
   void AsyncCompileSucceeded(Handle<WasmModuleObject> result);
 

src/wasm/wasm-engine.cc

@@ -160,7 +160,7 @@ MaybeHandle<WasmModuleObject> WasmEngine::SyncCompile(
       DecodeWasmModule(enabled, bytes.start(), bytes.end(), false, kWasmOrigin,
                        isolate->counters(), allocator());
   if (result.failed()) {
-    thrower->CompileFailed("Wasm decoding failed", result.error());
+    thrower->CompileFailed(result.error());
     return {};
   }
 

src/wasm/wasm-result.h

@@ -122,12 +122,6 @@ class V8_EXPORT_PRIVATE ErrorThrower {
   PRINTF_FORMAT(2, 3) void LinkError(const char* fmt, ...);
   PRINTF_FORMAT(2, 3) void RuntimeError(const char* fmt, ...);
 
-  void CompileFailed(const char* context, const WasmError& error) {
-    DCHECK(error.has_error());
-    CompileError("%s: %s @+%u", context, error.message().c_str(),
-                 error.offset());
-  }
-
   void CompileFailed(const WasmError& error) {
     DCHECK(error.has_error());
     CompileError("%s @+%u", error.message().c_str(), error.offset());

test/mjsunit/mjsunit.js

@@ -584,13 +584,15 @@ var prettyPrinted;
   }
 
   assertPromiseResult = function(promise, success, fail) {
+    if (success !== undefined) assertEquals('function', typeof success);
+    if (fail !== undefined) assertEquals('function', typeof fail);
     const stack = (new Error()).stack;
 
     var test_promise = promise.then(
         result => {
           try {
             if (--promiseTestCount == 0) testRunner.notifyDone();
-            if (success) success(result);
+            if (success !== undefined) success(result);
           } catch (e) {
             // Use setTimeout to throw the error again to get out of the promise
             // chain.
@@ -602,7 +604,7 @@ var prettyPrinted;
         result => {
           try {
             if (--promiseTestCount == 0) testRunner.notifyDone();
-            if (!fail) throw result;
+            if (fail === undefined) throw result;
             fail(result);
           } catch (e) {
             // Use setTimeout to throw the error again to get out of the promise

test/mjsunit/wasm/async-compile.js

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --expose-wasm --allow-natives-syntax
-
 load("test/mjsunit/wasm/wasm-module-builder.js");
 
 async function assertCompiles(buffer) {
@@ -11,13 +9,15 @@ async function assertCompiles(buffer) {
   assertInstanceof(module, WebAssembly.Module);
 }
 
-async function assertCompileError(buffer) {
-  try {
-    await WebAssembly.compile(buffer);
-    assertUnreachable();
-  } catch (e) {
+function assertCompileError(buffer, msg) {
+  assertEquals('string', typeof msg);
+  msg = 'WebAssembly.compile(): ' + msg;
+  function checkException(e) {
     if (!(e instanceof WebAssembly.CompileError)) throw e;
+    assertEquals(msg, e.message, 'Error message');
   }
+  return assertPromiseResult(
+      WebAssembly.compile(buffer), assertUnreachable, checkException);
 }
 
 assertPromiseResult(async function basicCompile() {
@@ -49,7 +49,7 @@ assertPromiseResult(async function basicCompile() {
 
   // Three compilations of the bad module should fail.
   for (var i = 0; i < kNumCompiles; i++) {
-    await assertCompileError(bad_buffer);
+    await assertCompileError(bad_buffer, 'BufferSource argument is empty');
   }
 }());
 
@@ -68,7 +68,10 @@ assertPromiseResult(async function badFunctionInTheMiddle() {
     builder.addFunction('b' + i, sig).addBody([kExprI32Const, 42]);
   }
   let buffer = builder.toBuffer();
-  await assertCompileError(buffer);
+  await assertCompileError(
+      buffer,
+      'Compiling wasm function \"bad\" failed: ' +
+          'expected 1 elements on the stack for fallthru to @1, found 0 @+94');
 }());
 
 assertPromiseResult(async function importWithoutCode() {

test/mjsunit/wasm/errors.js

@@ -4,8 +4,6 @@
 
 // Flags: --expose-wasm --allow-natives-syntax
 
-'use strict';
-
 load("test/mjsunit/wasm/wasm-module-builder.js");
 
 function module(bytes) {
@@ -36,6 +34,7 @@ function builder() {
 }
 
 function assertCompileError(bytes, msg) {
+  if (typeof msg === 'string') msg = 'WebAssembly.Module(): ' + msg;
   assertThrows(() => module(bytes), WebAssembly.CompileError, msg);
 }
 
@@ -49,18 +48,13 @@ function assertLinkError(bytes, imports, msg) {
   assertThrows(() => instance(bytes, imports), WebAssembly.LinkError, msg);
 }
 
-function assertRuntimeError(bytes, imports, msg) {
-  assertThrows(
-      () => instantiateAndFailAtRuntime(bytes, imports),
-      WebAssembly.RuntimeError, msg);
-}
-
 function assertConversionError(bytes, imports, msg) {
   assertThrows(
       () => instantiateAndFailAtRuntime(bytes, imports), TypeError, msg);
 }
 
 (function TestDecodingError() {
+  print(arguments.callee.name);
   assertCompileError("", /is empty/);
   assertCompileError("X", /expected 4 bytes, fell off end @\+0/);
   assertCompileError(
@@ -68,14 +62,16 @@ function assertConversionError(bytes, imports, msg) {
 })();
 
 (function TestValidationError() {
+  print(arguments.callee.name);
   assertCompileError(
-      builder().addFunction("f", kSig_i_v).end().toBuffer(),
-      /function body must end with "end" opcode @/);
+      builder().addFunction('f', kSig_i_v).end().toBuffer(),
+      'Compiling wasm function "f" failed: ' +
+          'function body must end with "end" opcode @+24');
   assertCompileError(
       builder().addFunction('f', kSig_i_v).addBody([kExprReturn])
           .end().toBuffer(),
       /expected 1 elements on the stack for return, found 0 @/);
-  assertCompileError(builder().addFunction("f", kSig_v_v).addBody([
+  assertCompileError(builder().addFunction('f', kSig_v_v).addBody([
     kExprGetLocal, 0
   ]).end().toBuffer(), /invalid local index: 0 @/);
   assertCompileError(
@@ -83,6 +79,7 @@ function assertConversionError(bytes, imports, msg) {
 })();
 
 (function TestTypeError() {
+  print(arguments.callee.name);
   let b;
   b = builder();
   b.addImport("foo", "bar", kSig_v_v);
@@ -98,6 +95,7 @@ function assertConversionError(bytes, imports, msg) {
 })();
 
 (function TestLinkingError() {
+  print(arguments.callee.name);
   let b;
 
   b = builder();
@@ -131,34 +129,33 @@ function assertConversionError(bytes, imports, msg) {
   assertLinkError(
       b.toBuffer(), {foo: {bar: () => new WebAssembly.Memory({initial: 0})}},
       /memory import must be a WebAssembly\.Memory object/);
+})();
 
-  b = builder();
-  b.addFunction("startup", kSig_v_v).addBody([
-    kExprUnreachable,
-  ]).end().addStart(0);
-  assertRuntimeError(b.toBuffer(), {}, "unreachable");
+(function TestTrapUnreachable() {
+  print(arguments.callee.name);
+  let instance = builder().addFunction('run', kSig_v_v)
+    .addBody([kExprUnreachable]).exportFunc().end().instantiate();
+  assertTraps(kTrapUnreachable, instance.exports.run);
+})();
+
+(function TestTrapDivByZero() {
+  print(arguments.callee.name);
+  let instance = builder().addFunction('run', kSig_v_v).addBody(
+     [kExprI32Const, 1, kExprI32Const, 0, kExprI32DivS, kExprDrop])
+    .exportFunc().end().instantiate();
+  assertTraps(kTrapDivByZero, instance.exports.run);
 })();
 
-(function TestTrapError() {
-  assertRuntimeError(builder().addFunction("run", kSig_v_v).addBody([
-    kExprUnreachable
-  ]).exportFunc().end().toBuffer(), {}, "unreachable");
-
-  assertRuntimeError(builder().addFunction("run", kSig_v_v).addBody([
-    kExprI32Const, 1,
-    kExprI32Const, 0,
-    kExprI32DivS,
-    kExprDrop
-  ]).exportFunc().end().toBuffer(), {}, "divide by zero");
-
-  assertRuntimeError(builder().
-      addFunction("run", kSig_v_v).addBody([]).exportFunc().end().
-      addFunction("start", kSig_v_v).addBody([kExprUnreachable]).end().
-      addStart(1).toBuffer(),
-    {}, "unreachable");
+(function TestUnreachableInStart() {
+  print(arguments.callee.name);
+
+  let b = builder().addFunction("start", kSig_v_v).addBody(
+     [kExprUnreachable]).end().addStart(0);
+  assertTraps(kTrapUnreachable, () => b.instantiate());
 })();
 
 (function TestConversionError() {
+  print(arguments.callee.name);
   let b = builder();
   b.addImport('foo', 'bar', kSig_v_l);
   let buffer = b.addFunction('run', kSig_v_v)
@@ -180,6 +177,7 @@ function assertConversionError(bytes, imports, msg) {
 
 
 (function InternalDebugTrace() {
+  print(arguments.callee.name);
   var builder = new WasmModuleBuilder();
   var sig = builder.addType(kSig_i_dd);
   builder.addImport("mod", "func", sig);

test/mjsunit/wasm/exceptions-export.js

@@ -43,7 +43,7 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
   builder.addExportOfKind("ex_oob", kExternalException, except + 1);
   assertThrows(
       () => builder.instantiate(), WebAssembly.CompileError,
-      /Wasm decoding failed: exception index 1 out of bounds/);
+      'WebAssembly.Module(): exception index 1 out of bounds (1 entry) @+30');
 })();
 
 (function TestExportSameNameTwice() {
@@ -54,7 +54,8 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
   builder.addExportOfKind("ex", kExternalException, except);
   assertThrows(
       () => builder.instantiate(), WebAssembly.CompileError,
-      /Duplicate export name 'ex' for exception 0 and exception 0/);
+      'WebAssembly.Module(): Duplicate export name \'ex\' ' +
+          'for exception 0 and exception 0 @+28');
 })();
 
 (function TestExportModuleGetExports() {

test/mjsunit/wasm/start-function.js

@@ -45,7 +45,7 @@ assertThrows(() => {instantiate(kSig_i_v, [kExprI32Const, 0]);});
 
   assertThrows(
       () => builder.instantiate(), WebAssembly.CompileError,
-      'WebAssembly.Module(): Wasm decoding failed: ' +
+      'WebAssembly.Module(): ' +
           'start function index 1 out of bounds (1 entry) @+20');
 })();
 
@@ -62,8 +62,7 @@ assertThrows(() => {instantiate(kSig_i_v, [kExprI32Const, 0]);});
 
   assertThrows(
       () => builder.instantiate(), WebAssembly.CompileError,
-      'WebAssembly.Module(): Wasm decoding failed: ' +
-          'unexpected section <Start> @+27');
+      'WebAssembly.Module(): unexpected section <Start> @+27');
 })();
 
 

test/mjsunit/wasm/wasm-module-builder.js

@@ -484,19 +484,7 @@ let kTrapMsgs = [
 ];
 
 function assertTraps(trap, code) {
-  try {
-    if (typeof code === 'function') {
-      code();
-    } else {
-      eval(code);
-    }
-  } catch (e) {
-    assertEquals('object', typeof e);
-    assertEquals(kTrapMsgs[trap], e.message);
-    // Success.
-    return;
-  }
-  throw new MjsUnitAssertionError('Did not trap, expected: ' + kTrapMsgs[trap]);
+  assertThrows(code, WebAssembly.RuntimeError, kTrapMsgs[trap]);
 }
 
 class Binary extends Array {