[wasm] Reimplement WebAssembly.instantiate without desugaring

At the moment, WebAssembly.instantiate(bytes) is implemented by
desugaring it to WebAssembly.compile(bytes).then(WebAssembly.instantiate).
The problem is that the {then} in this snippet is observable. With this
CL I introduce a CompilationResultResolver which allows to do the
desugaring internally and thereby make the {then} unobservable.
Unfortunately the result of WebAssembly.instantiate(bytes) is different
than the result of WebAssembly.instantiate(module). Therefore I also
introduced an InstantiationResultResolver for symmetry with
WebAssembly.compile.

R=mstarzinger@chromium.org
Bug: chromium:837417

Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I2d98e03d65f2ada19041d5a9e2df5da91b24ccca
Reviewed-on: https://chromium-review.googlesource.com/1059783
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53347}

src/api.cc

@@ -7516,6 +7516,38 @@ MaybeLocal<WasmCompiledModule> WasmCompiledModule::Compile(Isolate* isolate,
       Utils::ToLocal(maybe_compiled.ToHandleChecked()));
 }
 
+// Resolves the result of streaming compilation.
+// TODO(ahaas): Refactor the streaming compilation API so that this class can
+// move to wasm-js.cc.
+class AsyncCompilationResolver : public i::wasm::CompilationResultResolver {
+ public:
+  AsyncCompilationResolver(Isolate* isolate, Handle<Promise> promise)
+      : promise_(
+            reinterpret_cast<i::Isolate*>(isolate)->global_handles()->Create(
+                *Utils::OpenHandle(*promise))) {}
+
+  ~AsyncCompilationResolver() {
+    i::GlobalHandles::Destroy(i::Handle<i::Object>::cast(promise_).location());
+  }
+
+  void OnCompilationSucceeded(i::Handle<i::WasmModuleObject> result) override {
+    i::MaybeHandle<i::Object> promise_result =
+        i::JSPromise::Resolve(promise_, result);
+    CHECK_EQ(promise_result.is_null(),
+             promise_->GetIsolate()->has_pending_exception());
+  }
+
+  void OnCompilationFailed(i::Handle<i::Object> error_reason) override {
+    i::MaybeHandle<i::Object> promise_result =
+        i::JSPromise::Reject(promise_, error_reason);
+    CHECK_EQ(promise_result.is_null(),
+             promise_->GetIsolate()->has_pending_exception());
+  }
+
+ private:
+  i::Handle<i::JSPromise> promise_;
+};
+
 WasmModuleObjectBuilderStreaming::WasmModuleObjectBuilderStreaming(
     Isolate* isolate)
     : isolate_(isolate) {
@@ -7524,10 +7556,10 @@ WasmModuleObjectBuilderStreaming::WasmModuleObjectBuilderStreaming(
   Local<Promise::Resolver> resolver = maybe_resolver.ToLocalChecked();
   promise_.Reset(isolate, resolver->GetPromise());
 
-  i::Handle<i::JSPromise> promise = Utils::OpenHandle(*GetPromise());
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   streaming_decoder_ = i_isolate->wasm_engine()->StartStreamingCompilation(
-      i_isolate, handle(i_isolate->context()), promise);
+      i_isolate, handle(i_isolate->context()),
+      base::make_unique<AsyncCompilationResolver>(isolate, GetPromise()));
 }
 
 Local<Promise> WasmModuleObjectBuilderStreaming::GetPromise() {

src/wasm/module-compiler.cc

@@ -2717,21 +2717,21 @@ void InstanceBuilder::LoadTableSegments(Handle<WasmInstanceObject> instance) {
   }
 }
 
-AsyncCompileJob::AsyncCompileJob(Isolate* isolate,
-                                 std::unique_ptr<byte[]> bytes_copy,
-                                 size_t length, Handle<Context> context,
-                                 Handle<JSPromise> promise)
+AsyncCompileJob::AsyncCompileJob(
+    Isolate* isolate, std::unique_ptr<byte[]> bytes_copy, size_t length,
+    Handle<Context> context,
+    std::unique_ptr<CompilationResultResolver> resolver)
     : isolate_(isolate),
       async_counters_(isolate->async_counters()),
       bytes_copy_(std::move(bytes_copy)),
-      wire_bytes_(bytes_copy_.get(), bytes_copy_.get() + length) {
+      wire_bytes_(bytes_copy_.get(), bytes_copy_.get() + length),
+      resolver_(std::move(resolver)) {
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
   v8::Platform* platform = V8::GetCurrentPlatform();
   foreground_task_runner_ = platform->GetForegroundTaskRunner(v8_isolate);
   // The handles for the context and promise must be deferred.
   DeferredHandleScope deferred(isolate);
   context_ = Handle<Context>(*context);
-  module_promise_ = Handle<JSPromise>(*promise);
   deferred_handles_.push_back(deferred.Detach());
 }
 
@@ -2861,15 +2861,11 @@ void AsyncCompileJob::AsyncCompileFailed(Handle<Object> error_reason) {
   // {job} keeps the {this} pointer alive.
   std::shared_ptr<AsyncCompileJob> job =
       isolate_->wasm_engine()->RemoveCompileJob(this);
-  MaybeHandle<Object> promise_result =
-      JSPromise::Reject(module_promise_, error_reason);
-  CHECK_EQ(promise_result.is_null(), isolate_->has_pending_exception());
+  resolver_->OnCompilationFailed(error_reason);
 }
 
-void AsyncCompileJob::AsyncCompileSucceeded(Handle<Object> result) {
-  MaybeHandle<Object> promise_result =
-      JSPromise::Resolve(module_promise_, result);
-  CHECK_EQ(promise_result.is_null(), isolate_->has_pending_exception());
+void AsyncCompileJob::AsyncCompileSucceeded(Handle<WasmModuleObject> result) {
+  resolver_->OnCompilationSucceeded(result);
 }
 
 // A closure to run a compilation step (either as foreground or background

src/wasm/module-compiler.h

@@ -27,6 +27,7 @@ class Vector;
 
 namespace wasm {
 
+class CompilationResultResolver;
 class CompilationState;
 class ErrorThrower;
 class ModuleCompiler;
@@ -85,7 +86,7 @@ class AsyncCompileJob {
  public:
   explicit AsyncCompileJob(Isolate* isolate, std::unique_ptr<byte[]> bytes_copy,
                            size_t length, Handle<Context> context,
-                           Handle<JSPromise> promise);
+                           std::unique_ptr<CompilationResultResolver> resolver);
 
   void Start();
 
@@ -118,7 +119,7 @@ class AsyncCompileJob {
 
   void AsyncCompileFailed(Handle<Object> error_reason);
 
-  void AsyncCompileSucceeded(Handle<Object> result);
+  void AsyncCompileSucceeded(Handle<WasmModuleObject> result);
 
   void StartForegroundTask();
 
@@ -148,7 +149,7 @@ class AsyncCompileJob {
   std::unique_ptr<byte[]> bytes_copy_;
   ModuleWireBytes wire_bytes_;
   Handle<Context> context_;
-  Handle<JSPromise> module_promise_;
+  std::unique_ptr<CompilationResultResolver> resolver_;
   std::unique_ptr<WasmModule> module_;
 
   std::vector<DeferredHandles*> deferred_handles_;

src/wasm/wasm-engine.cc

@@ -60,24 +60,23 @@ MaybeHandle<WasmInstanceObject> WasmEngine::SyncInstantiate(
                                      memory);
 }
 
-void WasmEngine::AsyncInstantiate(Isolate* isolate, Handle<JSPromise> promise,
-                                  Handle<WasmModuleObject> module_object,
-                                  MaybeHandle<JSReceiver> imports) {
+void WasmEngine::AsyncInstantiate(
+    Isolate* isolate, std::unique_ptr<InstantiationResultResolver> resolver,
+    Handle<WasmModuleObject> module_object, MaybeHandle<JSReceiver> imports) {
   ErrorThrower thrower(isolate, nullptr);
   MaybeHandle<WasmInstanceObject> instance_object = SyncInstantiate(
       isolate, &thrower, module_object, imports, Handle<JSArrayBuffer>::null());
   if (thrower.error()) {
-    MaybeHandle<Object> result = JSPromise::Reject(promise, thrower.Reify());
-    CHECK_EQ(result.is_null(), isolate->has_pending_exception());
+    resolver->OnInstantiationFailed(thrower.Reify());
     return;
   }
   Handle<WasmInstanceObject> instance = instance_object.ToHandleChecked();
-  MaybeHandle<Object> result = JSPromise::Resolve(promise, instance);
-  CHECK_EQ(result.is_null(), isolate->has_pending_exception());
+  resolver->OnInstantiationSucceeded(instance);
 }
 
-void WasmEngine::AsyncCompile(Isolate* isolate, Handle<JSPromise> promise,
-                              const ModuleWireBytes& bytes, bool is_shared) {
+void WasmEngine::AsyncCompile(
+    Isolate* isolate, std::unique_ptr<CompilationResultResolver> resolver,
+    const ModuleWireBytes& bytes, bool is_shared) {
   if (!FLAG_wasm_async_compilation) {
     // Asynchronous compilation disabled; fall back on synchronous compilation.
     ErrorThrower thrower(isolate, "WasmCompile");
@@ -94,21 +93,18 @@ void WasmEngine::AsyncCompile(Isolate* isolate, Handle<JSPromise> promise,
       module_object = SyncCompile(isolate, &thrower, bytes);
     }
     if (thrower.error()) {
-      MaybeHandle<Object> result = JSPromise::Reject(promise, thrower.Reify());
-      CHECK_EQ(result.is_null(), isolate->has_pending_exception());
+      resolver->OnCompilationFailed(thrower.Reify());
       return;
     }
     Handle<WasmModuleObject> module = module_object.ToHandleChecked();
-    MaybeHandle<Object> result = JSPromise::Resolve(promise, module);
-    CHECK_EQ(result.is_null(), isolate->has_pending_exception());
+    resolver->OnCompilationSucceeded(module);
     return;
   }
 
   if (FLAG_wasm_test_streaming) {
     std::shared_ptr<StreamingDecoder> streaming_decoder =
-        isolate->wasm_engine()
-            ->StartStreamingCompilation(isolate, handle(isolate->context()),
-                                        promise);
+        isolate->wasm_engine()->StartStreamingCompilation(
+            isolate, handle(isolate->context()), std::move(resolver));
     streaming_decoder->OnBytesReceived(bytes.module_bytes());
     streaming_decoder->Finish();
     return;
@@ -120,14 +116,16 @@ void WasmEngine::AsyncCompile(Isolate* isolate, Handle<JSPromise> promise,
 
   AsyncCompileJob* job =
       CreateAsyncCompileJob(isolate, std::move(copy), bytes.length(),
-                            handle(isolate->context()), promise);
+                            handle(isolate->context()), std::move(resolver));
   job->Start();
 }
 
 std::shared_ptr<StreamingDecoder> WasmEngine::StartStreamingCompilation(
-    Isolate* isolate, Handle<Context> context, Handle<JSPromise> promise) {
-  AsyncCompileJob* job = CreateAsyncCompileJob(
-      isolate, std::unique_ptr<byte[]>(nullptr), 0, context, promise);
+    Isolate* isolate, Handle<Context> context,
+    std::unique_ptr<CompilationResultResolver> resolver) {
+  AsyncCompileJob* job =
+      CreateAsyncCompileJob(isolate, std::unique_ptr<byte[]>(nullptr), 0,
+                            context, std::move(resolver));
   return job->CreateStreamingDecoder();
 }
 
@@ -141,9 +139,10 @@ void WasmEngine::Unregister(CancelableTaskManager* task_manager) {
 
 AsyncCompileJob* WasmEngine::CreateAsyncCompileJob(
     Isolate* isolate, std::unique_ptr<byte[]> bytes_copy, size_t length,
-    Handle<Context> context, Handle<JSPromise> promise) {
-  AsyncCompileJob* job = new AsyncCompileJob(isolate, std::move(bytes_copy),
-                                             length, context, promise);
+    Handle<Context> context,
+    std::unique_ptr<CompilationResultResolver> resolver) {
+  AsyncCompileJob* job = new AsyncCompileJob(
+      isolate, std::move(bytes_copy), length, context, std::move(resolver));
   // Pass ownership to the unique_ptr in {jobs_}.
   jobs_[job] = std::unique_ptr<AsyncCompileJob>(job);
   return job;

src/wasm/wasm-engine.h

@@ -21,6 +21,20 @@ namespace wasm {
 class ErrorThrower;
 struct ModuleWireBytes;
 
+class V8_EXPORT_PRIVATE CompilationResultResolver {
+ public:
+  virtual void OnCompilationSucceeded(Handle<WasmModuleObject> result) = 0;
+  virtual void OnCompilationFailed(Handle<Object> error_reason) = 0;
+  virtual ~CompilationResultResolver() {}
+};
+
+class V8_EXPORT_PRIVATE InstantiationResultResolver {
+ public:
+  virtual void OnInstantiationSucceeded(Handle<WasmInstanceObject> result) = 0;
+  virtual void OnInstantiationFailed(Handle<Object> error_reason) = 0;
+  virtual ~InstantiationResultResolver() {}
+};
+
 // The central data structure that represents an engine instance capable of
 // loading, instantiating, and executing WASM code.
 class V8_EXPORT_PRIVATE WasmEngine {
@@ -54,20 +68,22 @@ class V8_EXPORT_PRIVATE WasmEngine {
       MaybeHandle<JSArrayBuffer> memory);
 
   // Begin an asynchronous compilation of the given bytes that represent an
-  // encoded WASM module, placing the result in the supplied {promise}.
+  // encoded WASM module.
   // The {is_shared} flag indicates if the bytes backing the module could
   // be shared across threads, i.e. could be concurrently modified.
-  void AsyncCompile(Isolate* isolate, Handle<JSPromise> promise,
+  void AsyncCompile(Isolate* isolate,
+                    std::unique_ptr<CompilationResultResolver> resolver,
                     const ModuleWireBytes& bytes, bool is_shared);
 
-  // Begin an asynchronous instantiation of the given WASM module, placing the
-  // result in the supplied {promise}.
-  void AsyncInstantiate(Isolate* isolate, Handle<JSPromise> promise,
+  // Begin an asynchronous instantiation of the given WASM module.
+  void AsyncInstantiate(Isolate* isolate,
+                        std::unique_ptr<InstantiationResultResolver> resolver,
                         Handle<WasmModuleObject> module_object,
                         MaybeHandle<JSReceiver> imports);
 
   std::shared_ptr<StreamingDecoder> StartStreamingCompilation(
-      Isolate* isolate, Handle<Context> context, Handle<JSPromise> promise);
+      Isolate* isolate, Handle<Context> context,
+      std::unique_ptr<CompilationResultResolver> resolver);
 
   WasmCodeManager* code_manager() const { return code_manager_.get(); }
 
@@ -94,10 +110,10 @@ class V8_EXPORT_PRIVATE WasmEngine {
   void TearDown();
 
  private:
-  AsyncCompileJob* CreateAsyncCompileJob(Isolate* isolate,
-                                         std::unique_ptr<byte[]> bytes_copy,
-                                         size_t length, Handle<Context> context,
-                                         Handle<JSPromise> promise);
+  AsyncCompileJob* CreateAsyncCompileJob(
+      Isolate* isolate, std::unique_ptr<byte[]> bytes_copy, size_t length,
+      Handle<Context> context,
+      std::unique_ptr<CompilationResultResolver> resolver);
 
   // We use an AsyncCompileJob as the key for itself so that we can delete the
   // job from the map when it is finished.

test/cctest/wasm/test-streaming-compilation.cc

@@ -82,24 +82,39 @@ class MockPlatform final : public TestPlatform {
 
 namespace {
 
+enum class CompilationState {
+  kPending,
+  kFinished,
+  kFailed,
+};
+
+class TestResolver : public i::wasm::CompilationResultResolver {
+ public:
+  explicit TestResolver(CompilationState* state) : state_(state) {}
+
+  void OnCompilationSucceeded(i::Handle<i::WasmModuleObject> module) override {
+    *state_ = CompilationState::kFinished;
+  }
+
+  void OnCompilationFailed(i::Handle<i::Object> error_reason) override {
+    *state_ = CompilationState::kFailed;
+  }
+
+ private:
+  CompilationState* state_;
+};
+
 class StreamTester {
  public:
   StreamTester() : zone_(&allocator_, "StreamTester") {
     v8::Isolate* isolate = CcTest::isolate();
     i::Isolate* i_isolate = CcTest::i_isolate();
 
-    // Create the promise for the streaming compilation.
     v8::Local<v8::Context> context = isolate->GetCurrentContext();
-    v8::Local<Promise::Resolver> resolver;
-    CHECK(Promise::Resolver::New(context).ToLocal(&resolver));
-    CHECK(!i_isolate->has_scheduled_exception());
-    promise_ = resolver->GetPromise();
 
-    i::Handle<i::JSPromise> i_promise = v8::Utils::OpenHandle(*promise_);
-
-    stream_ = i_isolate->wasm_engine()
-                  ->StartStreamingCompilation(
-                      i_isolate, v8::Utils::OpenHandle(*context), i_promise);
+    stream_ = i_isolate->wasm_engine()->StartStreamingCompilation(
+        i_isolate, v8::Utils::OpenHandle(*context),
+        base::make_unique<TestResolver>(&state_));
   }
 
   std::shared_ptr<StreamingDecoder> stream() { return stream_; }
@@ -109,15 +124,11 @@ class StreamTester {
     static_cast<MockPlatform*>(i::V8::GetCurrentPlatform())->ExecuteTasks();
   }
 
-  bool IsPromiseFulfilled() {
-    return promise_->State() == v8::Promise::kFulfilled;
-  }
+  bool IsPromiseFulfilled() { return state_ == CompilationState::kFinished; }
 
-  bool IsPromiseRejected() {
-    return promise_->State() == v8::Promise::kRejected;
-  }
+  bool IsPromiseRejected() { return state_ == CompilationState::kFailed; }
 
-  bool IsPromisePending() { return promise_->State() == v8::Promise::kPending; }
+  bool IsPromisePending() { return state_ == CompilationState::kPending; }
 
   void OnBytesReceived(const uint8_t* start, size_t length) {
     stream_->OnBytesReceived(Vector<const uint8_t>(start, length));
@@ -130,7 +141,7 @@ class StreamTester {
  private:
   AccountingAllocator allocator_;
   Zone zone_;
-  v8::Local<v8::Promise> promise_;
+  CompilationState state_ = CompilationState::kPending;
   std::shared_ptr<StreamingDecoder> stream_;
 };
 }  // namespace

test/fuzzer/wasm-async.cc

@@ -25,42 +25,24 @@ class WasmModuleObject;
 namespace wasm {
 namespace fuzzer {
 
-#define ASSIGN(type, var, expr)                      \
-  v8::Local<type> var;                               \
-  do {                                               \
-    if (!expr.ToLocal(&var)) {                       \
-      DCHECK(i_isolate->has_scheduled_exception());  \
-      return 0;                                      \
-    } else {                                         \
-      DCHECK(!i_isolate->has_scheduled_exception()); \
-    }                                                \
-  } while (false)
-
-namespace {
-// We need this helper function because we cannot use
-// Handle<WasmModuleObject>::cast here. To use this function we would have to
-// mark it with V8_EXPORT_PRIVATE, which is quite ugly in this case.
-Handle<WasmModuleObject> ToWasmModuleObjectUnchecked(Handle<Object> that) {
-  return handle(reinterpret_cast<WasmModuleObject*>(*that));
-}
-}
-
-void InstantiateCallback(const FunctionCallbackInfo<Value>& args) {
-  DCHECK_GE(args.Length(), 1);
-  v8::Isolate* isolate = args.GetIsolate();
-  MicrotasksScope does_not_run_microtasks(
-      isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
-
-  v8::HandleScope scope(isolate);
-
-  Local<v8::Value> module = args[0];
+class AsyncFuzzerResolver : public i::wasm::CompilationResultResolver {
+ public:
+  AsyncFuzzerResolver(i::Isolate* isolate, bool* done)
+      : isolate_(isolate), done_(done) {}
+
+  void OnCompilationSucceeded(i::Handle<i::WasmModuleObject> module) override {
+    *done_ = true;
+    InterpretAndExecuteModule(isolate_, module);
+  }
 
-  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  void OnCompilationFailed(i::Handle<i::Object> error_reason) override {
+    *done_ = true;
+  }
 
-  Handle<WasmModuleObject> module_obj =
-      ToWasmModuleObjectUnchecked(Utils::OpenHandle(v8::Object::Cast(*module)));
-  InterpretAndExecuteModule(i_isolate, module_obj);
-}
+ private:
+  i::Isolate* isolate_;
+  bool* done_;
+};
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   FlagScope<bool> turn_on_async_compile(
@@ -85,32 +67,19 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   TryCatch try_catch(isolate);
   testing::SetupIsolateForWasmModule(i_isolate);
 
-  // Get the promise for async compilation.
-  ASSIGN(Promise::Resolver, resolver,
-         Promise::Resolver::New(support->GetContext()));
-  Local<Promise> promise = resolver->GetPromise();
-
-  i_isolate->wasm_engine()->AsyncCompile(i_isolate, Utils::OpenHandle(*promise),
-                                         ModuleWireBytes(data, data + size),
-                                         false);
-
-  ASSIGN(Function, instantiate_impl,
-         Function::New(support->GetContext(), &InstantiateCallback,
-                       Undefined(isolate)));
-
-  ASSIGN(Promise, result,
-         promise->Then(support->GetContext(), instantiate_impl));
+  bool done = false;
+  i_isolate->wasm_engine()->AsyncCompile(
+      i_isolate, base::make_unique<AsyncFuzzerResolver>(i_isolate, &done),
+      ModuleWireBytes(data, data + size), false);
 
   // Wait for the promise to resolve.
-  while (result->State() == Promise::kPending) {
+  while (!done) {
     support->PumpMessageLoop(platform::MessageLoopBehavior::kWaitForWork);
     isolate->RunMicrotasks();
   }
   return 0;
 }
 
-#undef ASSIGN
-
 }  // namespace fuzzer
 }  // namespace wasm
 }  // namespace internal

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

@@ -8,16 +8,11 @@ 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
+  kExprI32Const, 12,         // i32.const 12
 ]);
 
-WebAssembly.Module.prototype.then = resolve => resolve(
-    String.fromCharCode(null, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41));
+WebAssembly.Module.prototype.then = resolve => {
+  assertUnreachable();
+};
 
-// 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);
-    });
+WebAssembly.instantiate(builder.toBuffer());