Fix bug in async generators.

Async generators didn't correctly handle the situation where one calls
.return on a suspended-at-start async generator and passes a
promise-like object whose awaiting causes a new request to the
generator.

Bug: chromium:805729
Change-Id: I4da13ab5bd97f8c2a2c5373242a2d5e2ab0f7f10
Reviewed-on: https://chromium-review.googlesource.com/891231Reviewed-by: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50974}

src/builtins/builtins-async-generator-gen.cc

@@ -68,24 +68,24 @@ class AsyncGeneratorBuiltinsAssembler : public AsyncBuiltinsAssembler {
     return IsGeneratorStateNotExecuting(LoadGeneratorState(generator));
   }
 
-  inline Node* LoadGeneratorAwaitedPromise(Node* const generator) {
-    return LoadObjectField(generator,
-                           JSAsyncGeneratorObject::kAwaitedPromiseOffset);
+  inline Node* IsGeneratorAwaiting(Node* const generator) {
+    Node* is_generator_awaiting =
+        LoadObjectField(generator, JSAsyncGeneratorObject::kIsAwaitingOffset);
+    return SmiEqual(is_generator_awaiting, SmiConstant(1));
   }
 
-  inline Node* IsGeneratorNotSuspendedForAwait(Node* const generator) {
-    return IsUndefined(LoadGeneratorAwaitedPromise(generator));
-  }
-
-  inline Node* IsGeneratorSuspendedForAwait(Node* const generator) {
-    return HasInstanceType(LoadGeneratorAwaitedPromise(generator),
-                           JS_PROMISE_TYPE);
+  inline void SetGeneratorAwaiting(Node* const generator) {
+    CSA_ASSERT(this, Word32BinaryNot(IsGeneratorAwaiting(generator)));
+    StoreObjectFieldNoWriteBarrier(
+        generator, JSAsyncGeneratorObject::kIsAwaitingOffset, SmiConstant(1));
+    CSA_ASSERT(this, IsGeneratorAwaiting(generator));
   }
 
-  inline void ClearAwaitedPromise(Node* const generator) {
-    StoreObjectFieldRoot(generator,
-                         JSAsyncGeneratorObject::kAwaitedPromiseOffset,
-                         Heap::kUndefinedValueRootIndex);
+  inline void SetGeneratorNotAwaiting(Node* const generator) {
+    CSA_ASSERT(this, IsGeneratorAwaiting(generator));
+    StoreObjectFieldNoWriteBarrier(
+        generator, JSAsyncGeneratorObject::kIsAwaitingOffset, SmiConstant(0));
+    CSA_ASSERT(this, Word32BinaryNot(IsGeneratorAwaiting(generator)));
   }
 
   inline void CloseGenerator(Node* const generator) {
@@ -226,14 +226,7 @@ void AsyncGeneratorBuiltinsAssembler::AsyncGeneratorAwaitResumeClosure(
       LoadContextElement(context, AwaitContext::kGeneratorSlot);
   CSA_SLOW_ASSERT(this, TaggedIsAsyncGenerator(generator));
 
-#if defined(DEBUG) && defined(ENABLE_SLOW_DCHECKS)
-  Node* const awaited_promise = LoadGeneratorAwaitedPromise(generator);
-  CSA_SLOW_ASSERT(this, HasInstanceType(awaited_promise, JS_PROMISE_TYPE));
-  CSA_SLOW_ASSERT(this, Word32NotEqual(PromiseStatus(awaited_promise),
-                                       Int32Constant(v8::Promise::kPending)));
-#endif
-
-  ClearAwaitedPromise(generator);
+  SetGeneratorNotAwaiting(generator);
 
   CSA_SLOW_ASSERT(this, IsGeneratorSuspended(generator));
 
@@ -269,13 +262,9 @@ void AsyncGeneratorBuiltinsAssembler::AsyncGeneratorAwait(bool is_catchable) {
   const int resolve_index = Context::ASYNC_GENERATOR_AWAIT_RESOLVE_SHARED_FUN;
   const int reject_index = Context::ASYNC_GENERATOR_AWAIT_REJECT_SHARED_FUN;
 
-  Node* promise =
-      Await(context, generator, value, outer_promise, AwaitContext::kLength,
-            init_closure_context, resolve_index, reject_index, is_catchable);
-
-  CSA_SLOW_ASSERT(this, IsGeneratorNotSuspendedForAwait(generator));
-  StoreObjectField(generator, JSAsyncGeneratorObject::kAwaitedPromiseOffset,
-                   promise);
+  SetGeneratorAwaiting(generator);
+  Await(context, generator, value, outer_promise, AwaitContext::kLength,
+        init_closure_context, resolve_index, reject_index, is_catchable);
   Return(UndefinedConstant());
 }
 
@@ -435,7 +424,7 @@ TF_BUILTIN(AsyncGeneratorResumeNext, AsyncGeneratorBuiltinsAssembler) {
   CSA_ASSERT(this, IsGeneratorNotExecuting(generator));
 
   // Stop resuming if suspended for Await.
-  ReturnIf(IsGeneratorSuspendedForAwait(generator), UndefinedConstant());
+  ReturnIf(IsGeneratorAwaiting(generator), UndefinedConstant());
 
   // Stop resuming if request queue is empty.
   ReturnIf(IsUndefined(var_next.value()), UndefinedConstant());
@@ -452,10 +441,9 @@ TF_BUILTIN(AsyncGeneratorResumeNext, AsyncGeneratorBuiltinsAssembler) {
               &settle_promise);
     CloseGenerator(generator);
     var_state.Bind(SmiConstant(JSGeneratorObject::kGeneratorClosed));
-
     Goto(&settle_promise);
-    BIND(&settle_promise);
 
+    BIND(&settle_promise);
     Node* next_value = LoadValueFromAsyncGeneratorRequest(next);
     Branch(SmiEqual(resume_type, SmiConstant(JSGeneratorObject::kReturn)),
            &if_return, &if_throw);
@@ -511,7 +499,7 @@ TF_BUILTIN(AsyncGeneratorResolve, AsyncGeneratorBuiltinsAssembler) {
   Node* const context = Parameter(Descriptor::kContext);
 
   CSA_SLOW_ASSERT(this, TaggedIsAsyncGenerator(generator));
-  CSA_ASSERT(this, IsGeneratorNotSuspendedForAwait(generator));
+  CSA_ASSERT(this, Word32BinaryNot(IsGeneratorAwaiting(generator)));
 
   // If this assertion fails, the `value` component was not Awaited as it should
   // have been, per https://github.com/tc39/proposal-async-iteration/pull/102/.
@@ -574,11 +562,9 @@ TF_BUILTIN(AsyncGeneratorYield, AsyncGeneratorBuiltinsAssembler) {
   const int on_resolve = Context::ASYNC_GENERATOR_YIELD_RESOLVE_SHARED_FUN;
   const int on_reject = Context::ASYNC_GENERATOR_AWAIT_REJECT_SHARED_FUN;
 
-  Node* const promise =
-      Await(context, generator, value, outer_promise, AwaitContext::kLength,
-            init_closure_context, on_resolve, on_reject, is_caught);
-  StoreObjectField(generator, JSAsyncGeneratorObject::kAwaitedPromiseOffset,
-                   promise);
+  SetGeneratorAwaiting(generator);
+  Await(context, generator, value, outer_promise, AwaitContext::kLength,
+        init_closure_context, on_resolve, on_reject, is_caught);
   Return(UndefinedConstant());
 }
 
@@ -588,8 +574,7 @@ TF_BUILTIN(AsyncGeneratorYieldResolveClosure, AsyncGeneratorBuiltinsAssembler) {
   Node* const generator =
       LoadContextElement(context, AwaitContext::kGeneratorSlot);
 
-  CSA_SLOW_ASSERT(this, IsGeneratorSuspendedForAwait(generator));
-  ClearAwaitedPromise(generator);
+  SetGeneratorNotAwaiting(generator);
 
   // Per proposal-async-iteration/#sec-asyncgeneratoryield step 9
   // Return ! AsyncGeneratorResolve(_F_.[[Generator]], _value_, *false*).
@@ -645,16 +630,13 @@ TF_BUILTIN(AsyncGeneratorReturn, AsyncGeneratorBuiltinsAssembler) {
                                       generator);
   };
 
+  SetGeneratorAwaiting(generator);
   Node* const context = Parameter(Descriptor::kContext);
   Node* const outer_promise = LoadPromiseFromAsyncGeneratorRequest(req);
-  Node* const promise =
-      Await(context, generator, value, outer_promise, AwaitContext::kLength,
-            init_closure_context, var_on_resolve.value(), var_on_reject.value(),
-            is_caught);
-
-  CSA_SLOW_ASSERT(this, IsGeneratorNotSuspendedForAwait(generator));
-  StoreObjectField(generator, JSAsyncGeneratorObject::kAwaitedPromiseOffset,
-                   promise);
+  Await(context, generator, value, outer_promise, AwaitContext::kLength,
+        init_closure_context, var_on_resolve.value(), var_on_reject.value(),
+        is_caught);
+
   Return(UndefinedConstant());
 }
 
@@ -679,8 +661,7 @@ TF_BUILTIN(AsyncGeneratorReturnClosedResolveClosure,
   Node* const generator =
       LoadContextElement(context, AwaitContext::kGeneratorSlot);
 
-  CSA_SLOW_ASSERT(this, IsGeneratorSuspendedForAwait(generator));
-  ClearAwaitedPromise(generator);
+  SetGeneratorNotAwaiting(generator);
 
   // https://tc39.github.io/proposal-async-iteration/
   //    #async-generator-resume-next-return-processor-fulfilled step 2:
@@ -698,8 +679,7 @@ TF_BUILTIN(AsyncGeneratorReturnClosedRejectClosure,
   Node* const generator =
       LoadContextElement(context, AwaitContext::kGeneratorSlot);
 
-  CSA_SLOW_ASSERT(this, IsGeneratorSuspendedForAwait(generator));
-  ClearAwaitedPromise(generator);
+  SetGeneratorNotAwaiting(generator);
 
   // https://tc39.github.io/proposal-async-iteration/
   //    #async-generator-resume-next-return-processor-rejected step 2:

src/builtins/builtins-object-gen.cc

@@ -760,7 +760,7 @@ TF_BUILTIN(CreateGeneratorObject, ObjectBuiltinsAssembler) {
 
   // Get the initial map from the function, jumping to the runtime if we don't
   // have one.
-  Label runtime(this);
+  Label done(this), runtime(this);
   GotoIfNot(IsFunctionWithPrototypeSlotMap(LoadMap(closure)), &runtime);
   Node* maybe_map =
       LoadObjectField(closure, JSFunction::kPrototypeOrInitialMapOffset);
@@ -790,7 +790,13 @@ TF_BUILTIN(CreateGeneratorObject, ObjectBuiltinsAssembler) {
   Node* executing = SmiConstant(JSGeneratorObject::kGeneratorExecuting);
   StoreObjectFieldNoWriteBarrier(result, JSGeneratorObject::kContinuationOffset,
                                  executing);
-  Return(result);
+  GotoIfNot(HasInstanceType(maybe_map, JS_ASYNC_GENERATOR_OBJECT_TYPE), &done);
+  StoreObjectFieldNoWriteBarrier(
+      result, JSAsyncGeneratorObject::kIsAwaitingOffset, SmiConstant(0));
+  Goto(&done);
+
+  BIND(&done);
+  { Return(result); }
 
   BIND(&runtime);
   {

src/compiler/access-builder.cc

@@ -289,12 +289,12 @@ FieldAccess AccessBuilder::ForJSAsyncGeneratorObjectQueue() {
 }
 
 // static
-FieldAccess AccessBuilder::ForJSAsyncGeneratorObjectAwaitedPromise() {
+FieldAccess AccessBuilder::ForJSAsyncGeneratorObjectIsAwaiting() {
   FieldAccess access = {
-      kTaggedBase,         JSAsyncGeneratorObject::kAwaitedPromiseOffset,
+      kTaggedBase,         JSAsyncGeneratorObject::kIsAwaitingOffset,
       Handle<Name>(),      MaybeHandle<Map>(),
-      Type::NonInternal(), MachineType::AnyTagged(),
-      kFullWriteBarrier};
+      Type::SignedSmall(), MachineType::TaggedSigned(),
+      kNoWriteBarrier};
   return access;
 }
 

src/compiler/access-builder.h

@@ -109,8 +109,8 @@ class V8_EXPORT_PRIVATE AccessBuilder final
   // Provides access to JSAsyncGeneratorObject::queue() field.
   static FieldAccess ForJSAsyncGeneratorObjectQueue();
 
-  // Provides access to JSAsyncGeneratorObject::awaited_promise() field.
-  static FieldAccess ForJSAsyncGeneratorObjectAwaitedPromise();
+  // Provides access to JSAsyncGeneratorObject::is_awaiting() field.
+  static FieldAccess ForJSAsyncGeneratorObjectIsAwaiting();
 
   // Provides access to JSArray::length() field.
   static FieldAccess ForJSArrayLength(ElementsKind elements_kind);

src/compiler/js-create-lowering.cc

@@ -492,8 +492,8 @@ Reduction JSCreateLowering::ReduceJSCreateGeneratorObject(Node* node) {
 
     if (initial_map->instance_type() == JS_ASYNC_GENERATOR_OBJECT_TYPE) {
       a.Store(AccessBuilder::ForJSAsyncGeneratorObjectQueue(), undefined);
-      a.Store(AccessBuilder::ForJSAsyncGeneratorObjectAwaitedPromise(),
-              undefined);
+      a.Store(AccessBuilder::ForJSAsyncGeneratorObjectIsAwaiting(),
+              jsgraph()->ZeroConstant());
     }
 
     // Handle in-object properties, too.

src/objects-inl.h

@@ -2773,8 +2773,7 @@ bool JSGeneratorObject::is_executing() const {
 }
 
 ACCESSORS(JSAsyncGeneratorObject, queue, HeapObject, kQueueOffset)
-ACCESSORS(JSAsyncGeneratorObject, awaited_promise, HeapObject,
-          kAwaitedPromiseOffset)
+SMI_ACCESSORS(JSAsyncGeneratorObject, is_awaiting, kIsAwaitingOffset)
 
 ACCESSORS(JSValue, value, Object, kValueOffset)
 

src/objects.h

@@ -3334,14 +3334,14 @@ class JSAsyncGeneratorObject : public JSGeneratorObject {
   // undefined.
   DECL_ACCESSORS(queue, HeapObject)
 
-  // [awaited_promise]
-  // A reference to the Promise of an AwaitExpression.
-  DECL_ACCESSORS(awaited_promise, HeapObject)
+  // [is_awaiting]
+  // Whether or not the generator is currently awaiting.
+  DECL_INT_ACCESSORS(is_awaiting)
 
   // Layout description.
   static const int kQueueOffset = JSGeneratorObject::kSize;
-  static const int kAwaitedPromiseOffset = kQueueOffset + kPointerSize;
-  static const int kSize = kAwaitedPromiseOffset + kPointerSize;
+  static const int kIsAwaitingOffset = kQueueOffset + kPointerSize;
+  static const int kSize = kIsAwaitingOffset + kPointerSize;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSAsyncGeneratorObject);

src/runtime/runtime-generator.cc

@@ -30,6 +30,9 @@ RUNTIME_FUNCTION(Runtime_CreateJSGeneratorObject) {
   generator->set_receiver(*receiver);
   generator->set_register_file(*register_file);
   generator->set_continuation(JSGeneratorObject::kGeneratorExecuting);
+  if (generator->IsJSAsyncGeneratorObject()) {
+    Handle<JSAsyncGeneratorObject>::cast(generator)->set_is_awaiting(0);
+  }
   return *generator;
 }
 

test/mjsunit/regress/regress-805729.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.
+
+async function* asyncGenerator() {};
+let gen = asyncGenerator();
+gen.return({ get then() { delete this.then; gen.next(); } });