[wasm] Do an additional IsWasmModuleObject check during instantiation

When WebAssembly.instantiate or WebAssembly.instantiateStreaming is
called in JavaScript, internally we transfrom it into
WebAssembly.compile(buffer).then(WebAssembly.instantiate). However,
modifying the prototype of WebAssembly.Module can change the result of
WebAssembly.compile(buffer). With this CL we make sure that even if the
result of WebAssembly.compile is modified, there is still no type
confusion. In the long term we have to do a refactoring and remove
this internal transformation.

R=mstarzinger@chromium.org

Bug: chromium:837417
Change-Id: I376068b8b8b01b991ec450162da6a62ae7030c62
Reviewed-on: https://chromium-review.googlesource.com/1032392
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52859}

src/wasm/wasm-js.cc

@@ -330,16 +330,22 @@ MaybeLocal<Value> WebAssemblyInstantiateImpl(Isolate* isolate,
   i::MaybeHandle<i::Object> instance_object;
   {
     ScheduledErrorThrower thrower(i_isolate, "WebAssembly Instantiation");
+
+    // TODO(ahaas): These checks on the module should not be necessary here They
+    // are just a workaround for https://crbug.com/837417.
+    i::Handle<i::Object> module_obj = Utils::OpenHandle(*module);
+    if (!module_obj->IsWasmModuleObject()) {
+      thrower.TypeError("Argument 0 must be a WebAssembly.Module object");
+      return {};
+    }
+
     i::MaybeHandle<i::JSReceiver> maybe_imports =
         GetValueAsImports(ffi, &thrower);
     if (thrower.error()) return {};
 
-    i::Handle<i::WasmModuleObject> module_obj =
-        i::Handle<i::WasmModuleObject>::cast(
-            Utils::OpenHandle(Object::Cast(*module)));
     instance_object = i_isolate->wasm_engine()->SyncInstantiate(
-        i_isolate, &thrower, module_obj, maybe_imports,
-        i::MaybeHandle<i::JSArrayBuffer>());
+        i_isolate, &thrower, i::Handle<i::WasmModuleObject>::cast(module_obj),
+        maybe_imports, i::MaybeHandle<i::JSArrayBuffer>());
   }
 
   DCHECK_EQ(instance_object.is_null(), i_isolate->has_scheduled_exception());

test/mjsunit/regress/wasm/regress-836141.js

@@ -11,7 +11,6 @@ builder.addFunction("test", kSig_i_v).addBody([
   kExprI32Const, 12,         // i32.const 0
 ]);
 
-let bla = 0;
 let module = new WebAssembly.Module(builder.toBuffer());
 module.then = () => {
   // Use setTimeout to get out of the promise chain.

test/mjsunit/regress/wasm/regress-837417.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+load('test/mjsunit/wasm/wasm-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+builder.addMemory(16, 32);
+builder.addFunction("test", kSig_i_v).addBody([
+  kExprI32Const, 12,         // i32.const 0
+]);
+
+WebAssembly.Module.prototype.then = resolve => resolve(
+    String.fromCharCode(null, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41));
+
+// WebAssembly.instantiate should not actually throw a TypeError in this case.
+// However, this is a workaround for
+assertPromiseResult(
+    WebAssembly.instantiate(builder.toBuffer()), assertUnreachable,
+    exception => {
+      assertInstanceof(exception, TypeError);
+    });