Skip to content

Commit

Permalink
deps: V8: backport 49712d8a from upstream
Browse files Browse the repository at this point in the history
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 <mstarzinger@chromium.org>
  Commit-Queue: Andreas Haas <ahaas@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#52755}

PR-URL: #21334
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
ofrobots authored and rvagg committed Aug 16, 2018
1 parent ed0d939 commit 8e0f28b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 50 deletions.
2 changes: 1 addition & 1 deletion deps/v8/include/v8-version.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
Expand Down
81 changes: 32 additions & 49 deletions deps/v8/src/wasm/wasm-js.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,25 +270,7 @@ MaybeLocal<Value> 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<v8::Value>& args) {
DCHECK_GE(args.Length(), 1);
v8::Isolate* isolate = args.GetIsolate();
MicrotasksScope does_not_run_microtasks(isolate,
MicrotasksScope::kDoNotRunMicrotasks);

HandleScope scope(args.GetIsolate());
Local<Value> module = args[0];
Local<Value> ffi = args.Data();
Local<Value> instance;
if (WebAssemblyInstantiateImpl(isolate, module, ffi).ToLocal(&instance)) {
args.GetReturnValue().Set(instance);
}
}

void WebAssemblyInstantiateToPairCallback(
void WebAssemblyInstantiateCallback(
const v8::FunctionCallbackInfo<v8::Value>& args) {
DCHECK_GE(args.Length(), 1);
Isolate* isolate = args.GetIsolate();
Expand Down Expand Up @@ -369,7 +351,7 @@ void WebAssemblyInstantiateStreaming(
DCHECK(!module_promise.IsEmpty());
Local<Value> 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);
}
Expand All @@ -390,20 +372,12 @@ void WebAssemblyInstantiate(const v8::FunctionCallbackInfo<v8::Value>& args) {
Local<Context> context = isolate->GetCurrentContext();

ASSIGN(Promise::Resolver, resolver, Promise::Resolver::New(context));
Local<Promise> 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> promise = resolver->GetPromise();
args.GetReturnValue().Set(promise);

Local<Value> first_arg_value = args[0];
// If args.Length < 2, this will be undefined - see FunctionCallbackInfo.
Local<Value> ffi = args[1];
i::Handle<i::Object> first_arg = Utils::OpenHandle(*first_arg_value);
if (!first_arg->IsJSObject()) {
thrower.TypeError(
Expand All @@ -414,26 +388,35 @@ void WebAssemblyInstantiate(const v8::FunctionCallbackInfo<v8::Value>& 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<Promise>::Cast(async_compile_retval);
instantiator = WebAssemblyInstantiateToPairCallback;
i::Handle<i::WasmModuleObject> module_obj =
i::Handle<i::WasmModuleObject>::cast(first_arg);
// If args.Length < 2, this will be undefined - see FunctionCallbackInfo.
i::MaybeHandle<i::JSReceiver> 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<Value> 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<Promise>::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);
}

Expand Down
21 changes: 21 additions & 0 deletions deps/v8/test/mjsunit/regress/wasm/regress-836141.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit 8e0f28b

Please sign in to comment.