[wasm] JS-API: enable WebAssembly.instantiate tests; fix LinkError

We weren't throwing LinkError where appropriate

progress

BUG=v8:5835

Review-Url: https://codereview.chromium.org/2629523007
Cr-Commit-Position: refs/heads/master@{#42342}

src/asmjs/asm-js.cc

@@ -254,8 +254,8 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(i::Isolate* isolate,
                                               Handle<JSReceiver> foreign) {
   base::ElapsedTimer instantiate_timer;
   instantiate_timer.Start();
-  i::Handle<i::JSObject> module(
-      i::JSObject::cast(wasm_data->get(kWasmDataCompiledModule)));
+  i::Handle<i::WasmModuleObject> module(
+      i::WasmModuleObject::cast(wasm_data->get(kWasmDataCompiledModule)));
   i::Handle<i::FixedArray> foreign_globals(
       i::FixedArray::cast(wasm_data->get(kWasmDataForeignGlobals)));
 

src/wasm/wasm-js.cc

@@ -188,7 +188,7 @@ void WebAssemblyModule(const v8::FunctionCallbackInfo<v8::Value>& args) {
 }
 
 MaybeLocal<Value> InstantiateModuleImpl(
-    i::Isolate* i_isolate, i::Handle<i::JSObject> i_module_obj,
+    i::Isolate* i_isolate, i::Handle<i::WasmModuleObject> i_module_obj,
     const v8::FunctionCallbackInfo<v8::Value>& args, ErrorThrower* thrower) {
   // It so happens that in both the WebAssembly.instantiate, as well as
   // WebAssembly.Instance ctor, the positions of the ffi object and memory
@@ -199,6 +199,10 @@ MaybeLocal<Value> InstantiateModuleImpl(
 
   MaybeLocal<Value> nothing;
   i::Handle<i::JSReceiver> ffi = i::Handle<i::JSObject>::null();
+  // This is a first - level validation of the argument. If present, we only
+  // check its type. {Instantiate} will further check that if the module
+  // has imports, the argument must be present, as well as piecemeal
+  // import satisfaction.
   if (args.Length() > kFfiOffset && !args[kFfiOffset]->IsUndefined()) {
     if (!args[kFfiOffset]->IsObject()) {
       thrower->TypeError("Argument %d must be an object", kFfiOffset);
@@ -208,6 +212,7 @@ MaybeLocal<Value> InstantiateModuleImpl(
     ffi = i::Handle<i::JSReceiver>::cast(v8::Utils::OpenHandle(*obj));
   }
 
+  // The memory argument is a legacy, not spec - compliant artifact.
   i::Handle<i::JSArrayBuffer> memory = i::Handle<i::JSArrayBuffer>::null();
   if (args.Length() > kMemOffset && !args[kMemOffset]->IsUndefined()) {
     if (!args[kMemOffset]->IsObject()) {
@@ -317,8 +322,8 @@ void WebAssemblyInstance(const v8::FunctionCallbackInfo<v8::Value>& args) {
   }
 
   Local<Object> module_obj = Local<Object>::Cast(args[0]);
-  i::Handle<i::JSObject> i_module_obj =
-      i::Handle<i::JSObject>::cast(v8::Utils::OpenHandle(*module_obj));
+  i::Handle<i::WasmModuleObject> i_module_obj =
+      i::Handle<i::WasmModuleObject>::cast(v8::Utils::OpenHandle(*module_obj));
 
   MaybeLocal<Value> instance =
       InstantiateModuleImpl(i_isolate, i_module_obj, args, &thrower);
@@ -339,7 +344,7 @@ void WebAssemblyInstantiate(const v8::FunctionCallbackInfo<v8::Value>& args) {
     thrower.TypeError("Argument 0 must be a buffer source");
     return;
   }
-  i::MaybeHandle<i::JSObject> module_obj =
+  i::MaybeHandle<i::WasmModuleObject> module_obj =
       CreateModuleObject(isolate, args[0], &thrower);
 
   Local<Context> context = isolate->GetCurrentContext();

src/wasm/wasm-module.cc

@@ -1121,9 +1121,10 @@ void wasm::UpdateDispatchTables(Isolate* isolate,
 class WasmInstanceBuilder {
  public:
   WasmInstanceBuilder(Isolate* isolate, ErrorThrower* thrower,
-                      Handle<JSObject> module_object, Handle<JSReceiver> ffi,
-                      Handle<JSArrayBuffer> memory)
+                      Handle<WasmModuleObject> module_object,
+                      Handle<JSReceiver> ffi, Handle<JSArrayBuffer> memory)
       : isolate_(isolate),
+        module_(module_object->compiled_module()->module()),
         thrower_(thrower),
         module_object_(module_object),
         ffi_(ffi),
@@ -1132,6 +1133,15 @@ class WasmInstanceBuilder {
   // Build an instance, in all of its glory.
   MaybeHandle<WasmInstanceObject> Build() {
     MaybeHandle<WasmInstanceObject> nothing;
+
+    // Check that an imports argument was provided, if the module requires it.
+    // No point in continuing otherwise.
+    if (!module_->import_table.empty() && ffi_.is_null()) {
+      thrower_->TypeError(
+          "Imports argument must be present and must be an object");
+      return nothing;
+    }
+
     HistogramTimerScope wasm_instantiate_module_time_scope(
         isolate_->counters()->wasm_instantiate_module_time());
     Factory* factory = isolate_->factory();
@@ -1154,8 +1164,7 @@ class WasmInstanceBuilder {
       Handle<WasmCompiledModule> original;
       {
         DisallowHeapAllocation no_gc;
-        original = handle(
-            WasmCompiledModule::cast(module_object_->GetInternalField(0)));
+        original = handle(module_object_->compiled_module());
         if (original->has_weak_owning_instance()) {
           owner = handle(WasmInstanceObject::cast(
               original->weak_owning_instance()->value()));
@@ -1206,7 +1215,6 @@ class WasmInstanceBuilder {
       compiled_module_->set_code_table(code_table);
       compiled_module_->set_native_context(isolate_->native_context());
     }
-    module_ = compiled_module_->module();
 
     //--------------------------------------------------------------------------
     // Allocate the instance object.
@@ -1442,7 +1450,7 @@ class WasmInstanceBuilder {
 
     DCHECK(!isolate_->has_pending_exception());
     TRACE("Finishing instance %d\n", compiled_module_->instance_id());
-    TRACE_CHAIN(WasmCompiledModule::cast(module_object_->GetInternalField(0)));
+    TRACE_CHAIN(module_object_->compiled_module());
     return instance;
   }
 
@@ -1456,9 +1464,9 @@ class WasmInstanceBuilder {
   };
 
   Isolate* isolate_;
-  WasmModule* module_;
+  WasmModule* const module_;
   ErrorThrower* thrower_;
-  Handle<JSObject> module_object_;
+  Handle<WasmModuleObject> module_object_;
   Handle<JSReceiver> ffi_;
   Handle<JSArrayBuffer> memory_;
   Handle<JSArrayBuffer> globals_;
@@ -1475,9 +1483,9 @@ class WasmInstanceBuilder {
         import_name->length(), import_name->ToCString().get(), error);
   }
 
-  MaybeHandle<Object> ReportTypeError(const char* error, uint32_t index,
+  MaybeHandle<Object> ReportLinkError(const char* error, uint32_t index,
                                       Handle<String> module_name) {
-    thrower_->TypeError("Import #%d module=\"%.*s\" error: %s", index,
+    thrower_->LinkError("Import #%d module=\"%.*s\" error: %s", index,
                         module_name->length(), module_name->ToCString().get(),
                         error);
     return MaybeHandle<Object>();
@@ -1486,22 +1494,22 @@ class WasmInstanceBuilder {
   // Look up an import value in the {ffi_} object.
   MaybeHandle<Object> LookupImport(uint32_t index, Handle<String> module_name,
                                    Handle<String> import_name) {
-    if (ffi_.is_null()) {
-      return ReportTypeError("FFI is not an object", index, module_name);
-    }
+    // We pre-validated in the js-api layer that the ffi object is present, and
+    // a JSObject, if the module has imports.
+    DCHECK(!ffi_.is_null());
 
     // Look up the module first.
     MaybeHandle<Object> result =
         Object::GetPropertyOrElement(ffi_, module_name);
     if (result.is_null()) {
-      return ReportTypeError("module not found", index, module_name);
+      return ReportLinkError("module not found", index, module_name);
     }
 
     Handle<Object> module = result.ToHandleChecked();
 
     // Look up the value in the module.
     if (!module->IsJSReceiver()) {
-      return ReportTypeError("module is not an object or function", index,
+      return ReportLinkError("module is not an object or function", index,
                              module_name);
     }
 
@@ -2101,8 +2109,9 @@ class WasmInstanceBuilder {
 // Instantiates a WASM module, creating a WebAssembly.Instance from a
 // WebAssembly.Module.
 MaybeHandle<WasmInstanceObject> WasmModule::Instantiate(
-    Isolate* isolate, ErrorThrower* thrower, Handle<JSObject> wasm_module,
-    Handle<JSReceiver> ffi, Handle<JSArrayBuffer> memory) {
+    Isolate* isolate, ErrorThrower* thrower,
+    Handle<WasmModuleObject> wasm_module, Handle<JSReceiver> ffi,
+    Handle<JSArrayBuffer> memory) {
   WasmInstanceBuilder builder(isolate, thrower, wasm_module, ffi, memory);
   return builder.Build();
 }

src/wasm/wasm-module.h

@@ -216,8 +216,9 @@ struct V8_EXPORT_PRIVATE WasmModule {
 
   // Creates a new instantiation of the module in the given isolate.
   static MaybeHandle<WasmInstanceObject> Instantiate(
-      Isolate* isolate, ErrorThrower* thrower, Handle<JSObject> wasm_module,
-      Handle<JSReceiver> ffi, Handle<JSArrayBuffer> memory);
+      Isolate* isolate, ErrorThrower* thrower,
+      Handle<WasmModuleObject> wasm_module, Handle<JSReceiver> ffi,
+      Handle<JSArrayBuffer> memory);
 
   MaybeHandle<WasmCompiledModule> CompileFunctions(
       Isolate* isolate, Handle<Managed<WasmModule>> module_wrapper,

test/cctest/wasm/test-run-wasm-module.cc

@@ -240,8 +240,8 @@ class WasmSerializationTest {
     ErrorThrower thrower(current_isolate(), "");
     v8::Local<v8::WasmCompiledModule> deserialized_module;
     CHECK(Deserialize().ToLocal(&deserialized_module));
-    Handle<JSObject> module_object =
-        Handle<JSObject>::cast(v8::Utils::OpenHandle(*deserialized_module));
+    Handle<WasmModuleObject> module_object = Handle<WasmModuleObject>::cast(
+        v8::Utils::OpenHandle(*deserialized_module));
     {
       DisallowHeapAllocation assume_no_gc;
       Handle<WasmCompiledModule> compiled_part(

test/mjsunit/wasm/errors.js

@@ -25,6 +25,23 @@ function instance(bytes, imports = {}) {
   return new WebAssembly.Instance(module(bytes), imports);
 }
 
+// instantiate should succeed but run should fail.
+function instantiateAndFailAtRuntime(bytes, imports = {}) {
+  var instance = undefined;
+  try {
+    instance = new WebAssembly.Instance(module(bytes), imports);
+  } catch(e) {
+    // If we fail at startup.
+    if (e instanceof WebAssembly.RuntimeError) {
+      throw e;
+    }
+    // Swallow other instantiation errors because we expect instantiation
+    // to succeed but runtime to fail.
+    return;
+  }
+  instance.exports.run();
+}
+
 function builder() {
   return new WasmModuleBuilder;
 }
@@ -33,21 +50,23 @@ function assertCompileError(bytes) {
   assertThrows(() => module(bytes), WebAssembly.CompileError);
 }
 
+// default imports to {} so we get LinkError by default, thus allowing us to
+// distinguish the TypeError we want to catch
 function assertTypeError(bytes, imports = {}) {
   assertThrows(() => instance(bytes, imports), TypeError);
 }
 
-function assertLinkError(bytes, imports = {}) {
+function assertLinkError(bytes, imports) {
   assertThrows(() => instance(bytes, imports), WebAssembly.LinkError);
 }
 
-function assertRuntimeError(bytes, imports = {}) {
-  assertThrows(() => instance(bytes, imports).exports.run(),
+function assertRuntimeError(bytes, imports) {
+  assertThrows(() => instantiateAndFailAtRuntime(bytes, imports),
     WebAssembly.RuntimeError);
 }
 
-function assertConversionError(bytes, imports = {}) {
-  assertThrows(() => instance(bytes, imports).exports.run(), TypeError);
+function assertConversionError(bytes, imports) {
+  assertThrows(() => instantiateAndFailAtRuntime(bytes, imports), TypeError);
 }
 
 (function TestDecodingError() {
@@ -72,7 +91,7 @@ function assertConversionError(bytes, imports = {}) {
 
   b = builder();
   b.addImport("foo", "bar", kSig_v_v);
-  assertTypeError(b.toBuffer(), {});
+  assertLinkError(b.toBuffer(), {});
   b = builder();
   b.addImport("foo", "bar", kSig_v_v);
   assertLinkError(b.toBuffer(), {foo: {}});
@@ -82,7 +101,7 @@ function assertConversionError(bytes, imports = {}) {
 
   b = builder();
   b.addImportedGlobal("foo", "bar", kWasmI32);
-  assertTypeError(b.toBuffer(), {});
+  assertLinkError(b.toBuffer(), {});
   b = builder();
   b.addImportedGlobal("foo", "bar", kWasmI32);
   assertLinkError(b.toBuffer(), {foo: {}});
@@ -95,7 +114,7 @@ function assertConversionError(bytes, imports = {}) {
 
   b = builder();
   b.addImportedMemory("foo", "bar");
-  assertTypeError(b.toBuffer(), {});
+  assertLinkError(b.toBuffer(), {});
   b = builder();
   b.addImportedMemory("foo", "bar");
   assertLinkError(b.toBuffer(), {foo: {}});
@@ -105,7 +124,7 @@ function assertConversionError(bytes, imports = {}) {
       {foo: {bar: () => new WebAssembly.Memory({initial: 0})}});
 
   b = builder();
-  b.addFunction("f", kSig_v_v).addBody([
+  b.addFunction("startup", kSig_v_v).addBody([
     kExprUnreachable,
   ]).end().addStart(0);
   assertRuntimeError(b.toBuffer());
@@ -135,8 +154,9 @@ function assertConversionError(bytes, imports = {}) {
   b.addImport("foo", "bar", kSig_v_l);
   assertConversionError(b.addFunction("run", kSig_v_v).addBody([
     kExprI64Const, 0, kExprCallFunction, 0
-  ]).exportFunc().end().toBuffer());
+  ]).exportFunc().end().toBuffer(), {foo:{bar: (l)=>{}}});
 
+  b = builder()
   assertConversionError(builder().addFunction("run", kSig_l_v).addBody([
     kExprI64Const, 0
   ]).exportFunc().end().toBuffer());

test/mjsunit/wasm/js-api.js

@@ -41,6 +41,12 @@ let importingModuleBinary = (() => {
   return new Int8Array(builder.toBuffer());
 })();
 
+let memoryImportingModuleBinary = (() => {
+  var builder = new WasmModuleBuilder();
+  builder.addImportedMemory("", "my_memory");
+  return new Int8Array(builder.toBuffer());
+})();
+
 let moduleBinaryImporting2Memories = (() => {
   var builder = new WasmModuleBuilder();
   builder.addImportedMemory("", "memory1");
@@ -256,10 +262,16 @@ let Instance = WebAssembly.Instance;
 assertEq(Instance, instanceDesc.value);
 assertEq(Instance.length, 1);
 assertEq(Instance.name, "Instance");
+
 assertErrorMessage(() => Instance(), TypeError, /constructor without new is forbidden/);
 assertErrorMessage(() => new Instance(1), TypeError, "first argument must be a WebAssembly.Module");
 assertErrorMessage(() => new Instance({}), TypeError, "first argument must be a WebAssembly.Module");
 assertErrorMessage(() => new Instance(emptyModule, null), TypeError, "second argument must be an object");
+assertErrorMessage(() => new Instance(importingModule, null), TypeError, "");
+assertErrorMessage(() => new Instance(importingModule, undefined), TypeError, "");
+assertErrorMessage(() => new Instance(importingModule, {"":{g:()=>{}}}), LinkError, "");
+assertErrorMessage(() => new Instance(importingModule, {t:{f:()=>{}}}), LinkError, "");
+
 assertEq(new Instance(emptyModule) instanceof Instance, true);
 assertEq(new Instance(emptyModule, {}) instanceof Instance, true);
 
@@ -563,55 +575,63 @@ function assertCompileSuccess(bytes) {
 assertCompileSuccess(emptyModuleBinary);
 assertCompileSuccess(emptyModuleBinary.buffer);
 
-if (false) {  // TODO: implement WebAssembly.instantiate
-  // 'WebAssembly.instantiate' data property
-  let instantiateDesc = Object.getOwnPropertyDescriptor(WebAssembly, 'instantiate');
-  assertEq(typeof instantiateDesc.value, "function");
-  assertEq(instantiateDesc.writable, true);
-  assertEq(instantiateDesc.enumerable, false);
-  assertEq(instantiateDesc.configurable, true);
-
-  // 'WebAssembly.instantiate' function
-  let instantiate = WebAssembly.instantiate;
-  assertEq(instantiate, instantiateDesc.value);
-  assertEq(instantiate.length, 2);
-  assertEq(instantiate.name, "instantiate");
-  function assertInstantiateError(args, err, msg) {
-    var error = null;
-    try {
-      instantiate(...args).catch(e => error = e);
-    } catch(e) {
-      error = e;
-    }
-    drainJobQueue();
-    assertEq(error instanceof err, true);
-    assertEq(Boolean(error.stack.match("jsapi.js")), true);
-    assertEq(Boolean(error.message.match(msg)), true);
-  }
-  assertInstantiateError([], TypeError, /requires more than 0 arguments/);
-  assertInstantiateError([undefined], TypeError, /first argument must be a WebAssembly.Module, ArrayBuffer or typed array object/);
-  assertInstantiateError([1], TypeError, /first argument must be a WebAssembly.Module, ArrayBuffer or typed array object/);
-  assertInstantiateError([{}], TypeError, /first argument must be a WebAssembly.Module, ArrayBuffer or typed array object/);
-  assertInstantiateError([new Uint8Array()], CompileError, /failed to match magic number/);
-  assertInstantiateError([new ArrayBuffer()], CompileError, /failed to match magic number/);
-  assertInstantiateError([importingModule], TypeError, /second argument must be an object/);
-  assertInstantiateError([importingModule, null], TypeError, /second argument must be an object/);
-  assertInstantiateError([importingModuleBinary, null], TypeError, /second argument must be an object/);
-  function assertInstantiateSuccess(module, imports) {
-    var result = null;
-    instantiate(module, imports).then(r => result = r);
-    drainJobQueue();
-    if (module instanceof Module) {
-      assertEq(result instanceof Instance, true);
-    } else {
-      assertEq(result.module instanceof Module, true);
-      assertEq(result.instance instanceof Instance, true);
-    }
+// 'WebAssembly.instantiate' data property
+let instantiateDesc = Object.getOwnPropertyDescriptor(WebAssembly, 'instantiate');
+assertEq(typeof instantiateDesc.value, "function");
+assertEq(instantiateDesc.writable, true);
+assertEq(instantiateDesc.enumerable, false);
+assertEq(instantiateDesc.configurable, true);
+
+// 'WebAssembly.instantiate' function
+let instantiate = WebAssembly.instantiate;
+assertEq(instantiate, instantiateDesc.value);
+assertEq(instantiate.length, 1);
+assertEq(instantiate.name, "instantiate");
+function assertInstantiateError(args, err, msg) {
+  var error = null;
+  try {
+    instantiate(...args).catch(e => error = e);
+  } catch(e) {
+    error = e;
   }
-  assertInstantiateSuccess(emptyModule);
-  assertInstantiateSuccess(emptyModuleBinary);
-  assertInstantiateSuccess(emptyModuleBinary.buffer);
-  assertInstantiateSuccess(importingModule, {"":{f:()=>{}}});
-  assertInstantiateSuccess(importingModuleBinary, {"":{f:()=>{}}});
-  assertInstantiateSuccess(importingModuleBinary.buffer, {"":{f:()=>{}}});
+  drainJobQueue();
+  assertEq(error instanceof err, true);
+  assertEq(Boolean(error.stack.match("js-api.js")), true);
+  //TOassertEq(Boolean(error.message.match(msg)), true);
+}
+var scratch_memory = new WebAssembly.Memory(new ArrayBuffer(10));
+assertInstantiateError([], TypeError, /requires more than 0 arguments/);
+assertInstantiateError([undefined], TypeError, /first argument must be a BufferSource/);
+assertInstantiateError([1], TypeError, /first argument must be a BufferSource/);
+assertInstantiateError([{}], TypeError, /first argument must be a BufferSource/);
+assertInstantiateError([new Uint8Array()], CompileError, /failed to match magic number/);
+assertInstantiateError([new ArrayBuffer()], CompileError, /failed to match magic number/);
+assertInstantiateError([new Uint8Array("hi!")], CompileError, /failed to match magic number/);
+assertInstantiateError([new ArrayBuffer("hi!")], CompileError, /failed to match magic number/);
+assertInstantiateError([importingModule], TypeError, /second argument must be an object/);
+assertInstantiateError([importingModule, null], TypeError, /second argument must be an object/);
+assertInstantiateError([importingModuleBinary, null], TypeError, /second argument must be an object/);
+assertInstantiateError([emptyModule, null], TypeError, /first argument must be a BufferSource/);
+assertInstantiateError([importingModule, {"":{f:()=>{}}}], TypeError, /first argument must be a BufferSource/);
+assertInstantiateError([importingModuleBinary, null], TypeError, /TODO: error messages?/);
+assertInstantiateError([importingModuleBinary, undefined], TypeError, /TODO: error messages?/);
+assertInstantiateError([importingModuleBinary, {}], LinkError, /TODO: error messages?/);
+assertInstantiateError([importingModuleBinary, {"":{g:()=>{}}}], LinkError, /TODO: error messages?/);
+assertInstantiateError([importingModuleBinary, {t:{f:()=>{}}}], LinkError, /TODO: error messages?/);
+assertInstantiateError([memoryImportingModuleBinary, null], TypeError, /TODO: error messages?/);
+assertInstantiateError([memoryImportingModuleBinary, undefined], TypeError, /TODO: error messages?/);
+assertInstantiateError([memoryImportingModuleBinary, {}], LinkError, /TODO: error messages?/);
+assertInstantiateError([memoryImportingModuleBinary, {"mod": {"my_memory": scratch_memory}}], LinkError, /TODO: error messages?/);
+assertInstantiateError([memoryImportingModuleBinary, {"": {"memory": scratch_memory}}], LinkError, /TODO: error messages?/);
+
+function assertInstantiateSuccess(module_bytes, imports) {
+  var result = null;
+  instantiate(module_bytes, imports).then(r => result = r).catch(e => print(e));
+  drainJobQueue();
+  assertEq(result instanceof Instance, true);
 }
+assertInstantiateSuccess(emptyModuleBinary);
+assertInstantiateSuccess(emptyModuleBinary.buffer);
+assertInstantiateSuccess(importingModuleBinary, {"":{f:()=>{}}});
+assertInstantiateSuccess(importingModuleBinary.buffer, {"":{f:()=>{}}});
+assertInstantiateSuccess(memoryImportingModuleBinary, {"": {"my_memory": scratch_memory}});