[wasm] Check that sync and async errors match

This makes the existing error message tests also test the error
produced by asynchronous compilation and instantiation.
It also slightly tweaks the error message to contain the name of the
API function invoked instead of "WebAssembly Instantiation".

R=titzer@chromium.org

Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Bug: chromium:926311
Change-Id: If4ab963cee8267d43b289169d21b31637c471d6d
Reviewed-on: https://chromium-review.googlesource.com/c/1456085
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59442}

src/wasm/wasm-engine.cc

@@ -228,7 +228,7 @@ MaybeHandle<WasmInstanceObject> WasmEngine::SyncInstantiate(
 void WasmEngine::AsyncInstantiate(
     Isolate* isolate, std::unique_ptr<InstantiationResultResolver> resolver,
     Handle<WasmModuleObject> module_object, MaybeHandle<JSReceiver> imports) {
-  ErrorThrower thrower(isolate, "WebAssembly Instantiation");
+  ErrorThrower thrower(isolate, "WebAssembly.instantiate()");
   // Instantiate a TryCatch so that caught exceptions won't progagate out.
   // They will still be set as pending exceptions on the isolate.
   // TODO(clemensh): Avoid TryCatch, use Execution::TryCall internally to invoke

src/wasm/wasm-js.cc

@@ -723,7 +723,7 @@ MaybeLocal<Value> WebAssemblyInstantiateImpl(Isolate* isolate,
 
   i::MaybeHandle<i::Object> instance_object;
   {
-    ScheduledErrorThrower thrower(i_isolate, "WebAssembly Instantiation");
+    ScheduledErrorThrower thrower(i_isolate, "WebAssembly.Instance()");
 
     // TODO(ahaas): These checks on the module should not be necessary here They
     // are just a workaround for https://crbug.com/837417.
@@ -862,7 +862,7 @@ void WebAssemblyInstantiate(const v8::FunctionCallbackInfo<v8::Value>& args) {
   i_isolate->CountUsage(
       v8::Isolate::UseCounterFeature::kWebAssemblyInstantiation);
 
-  ScheduledErrorThrower thrower(i_isolate, "WebAssembly Instantiation");
+  ScheduledErrorThrower thrower(i_isolate, "WebAssembly.instantiate()");
 
   HandleScope scope(isolate);
 

test/mjsunit/wasm/errors.js

@@ -6,128 +6,130 @@
 
 load("test/mjsunit/wasm/wasm-module-builder.js");
 
-function module(bytes) {
-  let buffer = bytes;
-  if (typeof buffer === 'string') {
-    buffer = new ArrayBuffer(bytes.length);
-    let view = new Uint8Array(buffer);
-    for (let i = 0; i < bytes.length; ++i) {
-      view[i] = bytes.charCodeAt(i);
-    }
-  }
-  return new WebAssembly.Module(buffer);
-}
-
-function instance(bytes, imports = {}) {
-  return new WebAssembly.Instance(module(bytes), imports);
-}
-
-// instantiate should succeed but run should fail.
-function instantiateAndFailAtRuntime(bytes, imports = {}) {
-  var instance = new WebAssembly.Instance(module(bytes), imports);
-  instance.exports.run();
-}
-
 function builder() {
   return new WasmModuleBuilder;
 }
 
 function assertCompileError(bytes, msg) {
-  if (typeof msg === 'string') msg = 'WebAssembly.Module(): ' + msg;
-  assertThrows(() => module(bytes), WebAssembly.CompileError, msg);
+  assertThrows(
+      () => new WebAssembly.Module(bytes), WebAssembly.CompileError,
+      'WebAssembly.Module(): ' + msg);
+  assertThrowsAsync(
+      WebAssembly.compile(bytes), WebAssembly.CompileError,
+      'WebAssembly.compile(): ' + msg);
+}
+
+function assertInstantiateError(error, bytes, imports = {}, msg) {
+  assertThrows(
+      () => new WebAssembly.Instance(new WebAssembly.Module(bytes), imports),
+      error, 'WebAssembly.Instance(): ' + msg);
+  assertThrowsAsync(
+      WebAssembly.instantiate(bytes, imports), error,
+      'WebAssembly.instantiate(): ' + msg);
 }
 
 // default imports to {} so we get LinkError by default, thus allowing us to
 // distinguish the TypeError we want to catch
 function assertTypeError(bytes, imports = {}, msg) {
-  assertThrows(() => instance(bytes, imports), TypeError, msg);
+  assertInstantiateError(TypeError, bytes, imports, msg);
 }
 
 function assertLinkError(bytes, imports, msg) {
-  assertThrows(() => instance(bytes, imports), WebAssembly.LinkError, msg);
+  assertInstantiateError(WebAssembly.LinkError, bytes, imports, msg);
 }
 
 function assertConversionError(bytes, imports, msg) {
-  assertThrows(
-      () => instantiateAndFailAtRuntime(bytes, imports), TypeError, msg);
+  let instance =
+      new WebAssembly.Instance(new WebAssembly.Module(bytes), imports);
+  assertThrows(() => instance.exports.run(), TypeError, msg);
 }
 
 (function TestDecodingError() {
   print(arguments.callee.name);
-  assertCompileError("", /is empty/);
-  assertCompileError("X", /expected 4 bytes, fell off end @\+0/);
+  assertCompileError(bytes(), 'BufferSource argument is empty');
+  assertCompileError(bytes('X'), 'expected 4 bytes, fell off end @+0');
   assertCompileError(
-    "\0x00asm", /expected magic word 00 61 73 6d, found 00 78 30 30 @\+0/);
+      bytes('\0x00asm'),
+      'expected magic word 00 61 73 6d, found 00 78 30 30 @+0');
 })();
 
 (function TestValidationError() {
   print(arguments.callee.name);
+  let f_error = msg => 'Compiling wasm function "f" failed: ' + msg;
   assertCompileError(
       builder().addFunction('f', kSig_i_v).end().toBuffer(),
-      'Compiling wasm function "f" failed: ' +
-          'function body must end with "end" opcode @+24');
+      f_error('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 @/);
+      f_error('expected 1 elements on the stack for return, found 0 @+24'));
   assertCompileError(builder().addFunction('f', kSig_v_v).addBody([
     kExprGetLocal, 0
-  ]).end().toBuffer(), /invalid local index: 0 @/);
+  ]).end().toBuffer(), f_error('invalid local index: 0 @+24'));
   assertCompileError(
-      builder().addStart(0).toBuffer(), /function index 0 out of bounds/);
+      builder().addStart(0).toBuffer(),
+      'start function index 0 out of bounds (0 entries) @+10');
 })();
 
+function import_error(index, module, func, msg) {
+  let full_msg = 'Import #' + index + ' module=\"' + module + '\"';
+  if (func !== undefined) full_msg += ' function=\"' + func + '\"';
+  return full_msg + ' error: ' + msg;
+}
+
 (function TestTypeError() {
   print(arguments.callee.name);
-  let b;
-  b = builder();
-  b.addImport("foo", "bar", kSig_v_v);
-  assertTypeError(b.toBuffer(), {}, /module is not an object or function/);
+  let b = builder();
+  b.addImport('foo', 'bar', kSig_v_v);
+  let msg =
+      import_error(0, 'foo', undefined, 'module is not an object or function');
+  assertTypeError(b.toBuffer(), {}, msg);
 
   b = builder();
-  b.addImportedGlobal("foo", "bar", kWasmI32);
-  assertTypeError(b.toBuffer(), {}, /module is not an object or function/);
+  b.addImportedGlobal('foo', 'bar', kWasmI32);
+  assertTypeError(b.toBuffer(), {}, msg);
 
   b = builder();
-  b.addImportedMemory("foo", "bar");
-  assertTypeError(b.toBuffer(), {}, /module is not an object or function/);
+  b.addImportedMemory('foo', 'bar');
+  assertTypeError(b.toBuffer(), {}, msg);
 })();
 
 (function TestLinkingError() {
   print(arguments.callee.name);
   let b;
+  let msg;
 
   b = builder();
-  b.addImport("foo", "bar", kSig_v_v);
-  assertLinkError(
-      b.toBuffer(), {foo: {}}, /function import requires a callable/);
+  msg = import_error(0, 'foo', 'bar', 'function import requires a callable');
+  b.addImport('foo', 'bar', kSig_v_v);
+  assertLinkError(b.toBuffer(), {foo: {}}, msg);
   b = builder();
-  b.addImport("foo", "bar", kSig_v_v);
-  assertLinkError(
-      b.toBuffer(), {foo: {bar: 9}}, /function import requires a callable/);
+  b.addImport('foo', 'bar', kSig_v_v);
+  assertLinkError(b.toBuffer(), {foo: {bar: 9}}, msg);
 
   b = builder();
-  b.addImportedGlobal("foo", "bar", kWasmI32);
-  assertLinkError(b.toBuffer(), {foo: {}}, /global import must be a number/);
+  msg = import_error(
+      0, 'foo', 'bar',
+      'global import must be a number or WebAssembly.Global object');
+  b.addImportedGlobal('foo', 'bar', kWasmI32);
+  assertLinkError(b.toBuffer(), {foo: {}}, msg);
   b = builder();
-  b.addImportedGlobal("foo", "bar", kWasmI32);
-  assertLinkError(
-      b.toBuffer(), {foo: {bar: ""}}, /global import must be a number/);
+  b.addImportedGlobal('foo', 'bar', kWasmI32);
+  assertLinkError(b.toBuffer(), {foo: {bar: ''}}, msg);
   b = builder();
-  b.addImportedGlobal("foo", "bar", kWasmI32);
-  assertLinkError(
-      b.toBuffer(), {foo: {bar: () => 9}}, /global import must be a number/);
+  b.addImportedGlobal('foo', 'bar', kWasmI32);
+  assertLinkError(b.toBuffer(), {foo: {bar: () => 9}}, msg);
 
   b = builder();
-  b.addImportedMemory("foo", "bar");
-  assertLinkError(
-      b.toBuffer(), {foo: {}},
-      /memory import must be a WebAssembly\.Memory object/);
+  msg = import_error(
+      0, 'foo', 'bar', 'memory import must be a WebAssembly.Memory object');
+  b.addImportedMemory('foo', 'bar');
+  assertLinkError(b.toBuffer(), {foo: {}}, msg);
   b = builder();
-  b.addImportedMemory("foo", "bar", 1);
+  b.addImportedMemory('foo', 'bar', 1);
   assertLinkError(
       b.toBuffer(), {foo: {bar: () => new WebAssembly.Memory({initial: 0})}},
-      /memory import must be a WebAssembly\.Memory object/);
+      msg);
 })();
 
 (function TestTrapUnreachable() {

test/mjsunit/wasm/ffi-error.js

@@ -30,12 +30,17 @@ function checkSuccessfulInstantiation(builder, ffi, handler) {
   assertPromiseResult(builder.asyncInstantiate(ffi), handler);
 }
 
-function checkFailingInstantiation(builder, ffi, error, message) {
+function checkFailingInstantiation(
+    builder, ffi, error, message, prepend_context = true) {
   // Test synchronous instantiation.
-  assertThrows(_ => builder.instantiate(ffi), error, message);
+  assertThrows(
+      _ => builder.instantiate(ffi), error,
+      (prepend_context ? 'WebAssembly.Instance(): ' : '') + message);
 
   // Test asynchronous instantiation.
-  assertThrowsAsync(builder.asyncInstantiate(ffi), error, message);
+  assertThrowsAsync(
+      builder.asyncInstantiate(ffi), error,
+      (prepend_context ? 'WebAssembly.instantiate(): ' : '') + message);
 }
 
 (function testValidFFI() {
@@ -48,19 +53,19 @@ function checkFailingInstantiation(builder, ffi, error, message) {
   print(arguments.callee.name);
   checkFailingInstantiation(
       CreateDefaultBuilder(), 17, TypeError,
-      'WebAssembly Instantiation: Argument 1 must be an object');
+      'Argument 1 must be an object');
   checkFailingInstantiation(
       CreateDefaultBuilder(), {}, TypeError,
-      'WebAssembly Instantiation: Import #0 module="mod" error: module is not an object or function');
+      'Import #0 module="mod" error: module is not an object or function');
   checkFailingInstantiation(
       CreateDefaultBuilder(), {mod: {}}, WebAssembly.LinkError,
-      'WebAssembly Instantiation: Import #0 module="mod" function="fun" error: function import requires a callable');
+      'Import #0 module="mod" function="fun" error: function import requires a callable');
   checkFailingInstantiation(
       CreateDefaultBuilder(), {mod: {fun: {}}}, WebAssembly.LinkError,
-      'WebAssembly Instantiation: Import #0 module="mod" function="fun" error: function import requires a callable');
+      'Import #0 module="mod" function="fun" error: function import requires a callable');
   checkFailingInstantiation(
       CreateDefaultBuilder(), {mod: {fun: 0}}, WebAssembly.LinkError,
-      'WebAssembly Instantiation: Import #0 module="mod" function="fun" error: function import requires a callable');
+      'Import #0 module="mod" function="fun" error: function import requires a callable');
 })();
 
 (function testImportWithInvalidSignature() {
@@ -79,7 +84,7 @@ function checkFailingInstantiation(builder, ffi, error, message) {
   let exported = builder.instantiate().exports.exp;
   checkFailingInstantiation(
       CreateDefaultBuilder(), {mod: {fun: exported}}, WebAssembly.LinkError,
-      'WebAssembly Instantiation: Import #0 module="mod" function="fun" error: imported function does not match the expected type');
+      'Import #0 module="mod" function="fun" error: imported function does not match the expected type');
 })();
 
 (function regression870646() {
@@ -91,7 +96,8 @@ function checkFailingInstantiation(builder, ffi, error, message) {
     }
   });
 
-  checkFailingInstantiation(CreateDefaultBuilder(), ffi, Error, 'my_exception');
+  checkFailingInstantiation(
+      CreateDefaultBuilder(), ffi, Error, 'my_exception', false);
 })();
 
 // "fun" matches signature "i_dd"

test/mjsunit/wasm/unicode.js

@@ -59,7 +59,7 @@ checkExports('☺☺mul☺☺', '☺☺mul☺☺', '☺☺add☺☺', '☺☺add
   builder.addImport('three snowmen: ☃☃☃', 'foo', kSig_i_v);
   assertThrows(
       () => builder.instantiate({}), TypeError,
-      /WebAssembly Instantiation: Import #0 module="three snowmen: ☃☃☃" error: /);
+      /WebAssembly.Instance\(\): Import #0 module="three snowmen: ☃☃☃" error: /);
 })();
 
 (function errorMessageUnicodeInImportElemName() {
@@ -67,7 +67,7 @@ checkExports('☺☺mul☺☺', '☺☺mul☺☺', '☺☺add☺☺', '☺☺add
   builder.addImport('mod', 'three snowmen: ☃☃☃', kSig_i_v);
   assertThrows(
       () => builder.instantiate({mod: {}}), WebAssembly.LinkError,
-      'WebAssembly Instantiation: Import #0 module="mod" function="three ' +
+      'WebAssembly.Instance\(\): Import #0 module="mod" function="three ' +
           'snowmen: ☃☃☃" error: function import requires a callable');
 })();
 
@@ -78,7 +78,7 @@ checkExports('☺☺mul☺☺', '☺☺mul☺☺', '☺☺add☺☺', '☺☺add
   builder.addImport(mod_name, func_name, kSig_i_v);
   assertThrows(
       () => builder.instantiate({[mod_name]: {}}), WebAssembly.LinkError,
-      'WebAssembly Instantiation: Import #0 module="' + mod_name +
+      'WebAssembly.Instance(): Import #0 module="' + mod_name +
           '" function="' + func_name +
           '" error: function import requires a callable');
 })();

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

@@ -8,15 +8,30 @@ let byte_view = new Uint8Array(__buffer);
 let f32_view = new Float32Array(__buffer);
 let f64_view = new Float64Array(__buffer);
 
-function bytes() {
-  var buffer = new ArrayBuffer(arguments.length);
-  var view = new Uint8Array(buffer);
-  for (var i = 0; i < arguments.length; i++) {
-    var val = arguments[i];
-    if ((typeof val) == "string") val = val.charCodeAt(0);
+// The bytes function receives one of
+//  - several arguments, each of which is either a number or a string of length
+//    1; if it's a string, the charcode of the contained character is used.
+//  - a single array argument containing the actual arguments
+//  - a single string; the returned buffer will contain the char codes of all
+//    contained characters.
+function bytes(...input) {
+  if (input.length == 1 && typeof input[0] == 'array') input = input[0];
+  if (input.length == 1 && typeof input[0] == 'string') {
+    let len = input[0].length;
+    let view = new Uint8Array(len);
+    for (let i = 0; i < len; i++) view[i] = input[0].charCodeAt(i);
+    return view.buffer;
+  }
+  let view = new Uint8Array(input.length);
+  for (let i = 0; i < input.length; i++) {
+    let val = input[i];
+    if (typeof val == 'string') {
+      assertEquals(1, val.length, 'string inputs must have length 1');
+      val = val.charCodeAt(0);
+    }
     view[i] = val | 0;
   }
-  return buffer;
+  return view.buffer;
 }
 
 // Header declaration constants
@@ -34,23 +49,10 @@ var kHeaderSize = 8;
 var kPageSize = 65536;
 var kSpecMaxPages = 65535;
 
-function bytesWithHeader() {
-  var buffer = new ArrayBuffer(kHeaderSize + arguments.length);
-  var view = new Uint8Array(buffer);
-  view[0] = kWasmH0;
-  view[1] = kWasmH1;
-  view[2] = kWasmH2;
-  view[3] = kWasmH3;
-  view[4] = kWasmV0;
-  view[5] = kWasmV1;
-  view[6] = kWasmV2;
-  view[7] = kWasmV3;
-  for (var i = 0; i < arguments.length; i++) {
-    var val = arguments[i];
-    if ((typeof val) == "string") val = val.charCodeAt(0);
-    view[kHeaderSize + i] = val | 0;
-  }
-  return buffer;
+function bytesWithHeader(...input) {
+  const header =
+      [kWasmH0, kWasmH1, kWasmH2, kWasmH3, kWasmV0, kWasmV1, kWasmV2, kWasmV3];
+  return bytes(header + input);
 }
 
 let kDeclNoLocals = 0;