From 8e0f28b8f0f865f0d6075847a0edbe20e83e5980 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Mon, 18 Jun 2018 11:29:24 -0700 Subject: [PATCH] deps: V8: backport 49712d8a from upstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [wasm] Call AsyncInstantiate directly when instantiating a module object WebAssembly.instantiate is polymorphic, it can either take a module object as parameter, or a buffer source which should be compiled first. To share code between the two implementations, the module object was first passed to a promise (i.e. which is the result of compilation). However, passing the module object to a promise has a side effect if the module object has a then function. To avoid this side effect I remove this code sharing and call AsyncInstantiate directly in case the parameter is a module object. R=mstarzinger@chromium.org Bug: chromium:836141 Change-Id: I67b76d0d7761c5aeb2cf1deda45b6842e494eed4 Reviewed-on: https://chromium-review.googlesource.com/1025774 Reviewed-by: Michael Starzinger Commit-Queue: Andreas Haas Cr-Commit-Position: refs/heads/master@{#52755} PR-URL: https://github.com/nodejs/node/pull/21334 Reviewed-By: Michaël Zasso --- deps/v8/include/v8-version.h | 2 +- deps/v8/src/wasm/wasm-js.cc | 81 ++++++++----------- .../mjsunit/regress/wasm/regress-836141.js | 21 +++++ 3 files changed, 54 insertions(+), 50 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/wasm/regress-836141.js diff --git a/deps/v8/include/v8-version.h b/deps/v8/include/v8-version.h index f1f29d02de3fb5..ddecda5abe18c5 100644 --- a/deps/v8/include/v8-version.h +++ b/deps/v8/include/v8-version.h @@ -11,7 +11,7 @@ #define V8_MAJOR_VERSION 6 #define V8_MINOR_VERSION 2 #define V8_BUILD_NUMBER 414 -#define V8_PATCH_LEVEL 60 +#define V8_PATCH_LEVEL 61 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) diff --git a/deps/v8/src/wasm/wasm-js.cc b/deps/v8/src/wasm/wasm-js.cc index ce2e3f1341a9ed..54034ef4ef166d 100644 --- a/deps/v8/src/wasm/wasm-js.cc +++ b/deps/v8/src/wasm/wasm-js.cc @@ -270,25 +270,7 @@ MaybeLocal WebAssemblyInstantiateImpl(Isolate* isolate, return Utils::ToLocal(instance_object.ToHandleChecked()); } -// Entered as internal implementation detail of sync and async instantiate. -// args[0] *must* be a WebAssembly.Module. -void WebAssemblyInstantiateImplCallback( - const v8::FunctionCallbackInfo& args) { - DCHECK_GE(args.Length(), 1); - v8::Isolate* isolate = args.GetIsolate(); - MicrotasksScope does_not_run_microtasks(isolate, - MicrotasksScope::kDoNotRunMicrotasks); - - HandleScope scope(args.GetIsolate()); - Local module = args[0]; - Local ffi = args.Data(); - Local instance; - if (WebAssemblyInstantiateImpl(isolate, module, ffi).ToLocal(&instance)) { - args.GetReturnValue().Set(instance); - } -} - -void WebAssemblyInstantiateToPairCallback( +void WebAssemblyInstantiateCallback( const v8::FunctionCallbackInfo& args) { DCHECK_GE(args.Length(), 1); Isolate* isolate = args.GetIsolate(); @@ -369,7 +351,7 @@ void WebAssemblyInstantiateStreaming( DCHECK(!module_promise.IsEmpty()); Local data = args[1]; ASSIGN(Function, instantiate_impl, - Function::New(context, WebAssemblyInstantiateToPairCallback, data)); + Function::New(context, WebAssemblyInstantiateCallback, data)); ASSIGN(Promise, result, module_promise->Then(context, instantiate_impl)); args.GetReturnValue().Set(result); } @@ -390,20 +372,12 @@ void WebAssemblyInstantiate(const v8::FunctionCallbackInfo& args) { Local context = isolate->GetCurrentContext(); ASSIGN(Promise::Resolver, resolver, Promise::Resolver::New(context)); - Local module_promise = resolver->GetPromise(); - args.GetReturnValue().Set(module_promise); - - if (args.Length() < 1) { - thrower.TypeError( - "Argument 0 must be provided and must be either a buffer source or a " - "WebAssembly.Module object"); - auto maybe = resolver->Reject(context, Utils::ToLocal(thrower.Reify())); - CHECK_IMPLIES(!maybe.FromMaybe(false), - i_isolate->has_scheduled_exception()); - return; - } + Local promise = resolver->GetPromise(); + args.GetReturnValue().Set(promise); Local first_arg_value = args[0]; + // If args.Length < 2, this will be undefined - see FunctionCallbackInfo. + Local ffi = args[1]; i::Handle first_arg = Utils::OpenHandle(*first_arg_value); if (!first_arg->IsJSObject()) { thrower.TypeError( @@ -414,26 +388,35 @@ void WebAssemblyInstantiate(const v8::FunctionCallbackInfo& args) { return; } - FunctionCallback instantiator = nullptr; if (first_arg->IsWasmModuleObject()) { - module_promise = resolver->GetPromise(); - if (!resolver->Resolve(context, first_arg_value).IsJust()) return; - instantiator = WebAssemblyInstantiateImplCallback; - } else { - ASSIGN(Function, async_compile, Function::New(context, WebAssemblyCompile)); - ASSIGN(Value, async_compile_retval, - async_compile->Call(context, args.Holder(), 1, &first_arg_value)); - module_promise = Local::Cast(async_compile_retval); - instantiator = WebAssemblyInstantiateToPairCallback; + i::Handle module_obj = + i::Handle::cast(first_arg); + // If args.Length < 2, this will be undefined - see FunctionCallbackInfo. + i::MaybeHandle maybe_imports = + GetValueAsImports(ffi, &thrower); + + if (thrower.error()) { + auto maybe = resolver->Reject(context, Utils::ToLocal(thrower.Reify())); + CHECK_IMPLIES(!maybe.FromMaybe(false), + i_isolate->has_scheduled_exception()); + return; + } + + i::wasm::AsyncInstantiate( + i_isolate, Utils::OpenHandle(*promise), module_obj, maybe_imports); + return; } - DCHECK(!module_promise.IsEmpty()); - DCHECK_NOT_NULL(instantiator); - // If args.Length < 2, this will be undefined - see FunctionCallbackInfo. - // We'll check for that in WebAssemblyInstantiateImpl. - Local data = args[1]; + + // We did not get a WasmModuleObject as input, we first have to compile the + // input. + ASSIGN(Function, async_compile, Function::New(context, WebAssemblyCompile)); + ASSIGN(Value, async_compile_retval, + async_compile->Call(context, args.Holder(), 1, &first_arg_value)); + promise = Local::Cast(async_compile_retval); + DCHECK(!promise.IsEmpty()); ASSIGN(Function, instantiate_impl, - Function::New(context, instantiator, data)); - ASSIGN(Promise, result, module_promise->Then(context, instantiate_impl)); + Function::New(context, WebAssemblyInstantiateCallback, ffi)); + ASSIGN(Promise, result, promise->Then(context, instantiate_impl)); args.GetReturnValue().Set(result); } diff --git a/deps/v8/test/mjsunit/regress/wasm/regress-836141.js b/deps/v8/test/mjsunit/regress/wasm/regress-836141.js new file mode 100644 index 00000000000000..ff6ef85a42644a --- /dev/null +++ b/deps/v8/test/mjsunit/regress/wasm/regress-836141.js @@ -0,0 +1,21 @@ +// 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 +]); + +let bla = 0; +let module = new WebAssembly.Module(builder.toBuffer()); +module.then = () => { + // Use setTimeout to get out of the promise chain. + setTimeout(assertUnreachable); +}; + +WebAssembly.instantiate(module);