[async] Simplify async instrumentation checking a bit.

This introduces a new bit on the Isolate which tells whether promise
hooks, async event delegate or the debug delegate are enabled. Use
this new bit in places where we generally need to take the slow path
due to async instrumentation.

Bug: v8:7253, v8:7522, v8:8238
Change-Id: I8f34eeb9f8f7b56fcbb4deb59ac51b2d0907ff6c
Reviewed-on: https://chromium-review.googlesource.com/c/1296473
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56902}

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

@@ -150,30 +150,19 @@ TF_BUILTIN(AsyncFunctionEnter, AsyncFunctionBuiltinsAssembler) {
   StoreObjectFieldNoWriteBarrier(
       async_function_object, JSAsyncFunctionObject::kPromiseOffset, promise);
 
-  // Fire promise hooks if enabled.
-  Label if_hooks(this, Label::kDeferred), if_hooks_done(this);
-  Branch(IsPromiseHookEnabledOrHasAsyncEventDelegate(), &if_hooks,
-         &if_hooks_done);
-  BIND(&if_hooks);
+  // Fire promise hooks if enabled and push the Promise under construction
+  // in an async function on the catch prediction stack to handle exceptions
+  // thrown before the first await.
+  Label if_instrumentation(this, Label::kDeferred),
+      if_instrumentation_done(this);
+  Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
+         &if_instrumentation, &if_instrumentation_done);
+  BIND(&if_instrumentation);
   {
-    CallRuntime(Runtime::kPromiseHookInit, context, promise,
-                UndefinedConstant());
-    Goto(&if_hooks_done);
+    CallRuntime(Runtime::kDebugAsyncFunctionEntered, context, promise);
+    Goto(&if_instrumentation_done);
   }
-  BIND(&if_hooks_done);
-
-  // Push the Promise under construction in an async function on the
-  // catch prediction stack to handle exceptions thrown before the
-  // first await.
-  // TODO(bmeurer): Combine this with the hooks above.
-  Label if_debug(this, Label::kDeferred), if_debug_done(this);
-  Branch(IsDebugActive(), &if_debug, &if_debug_done);
-  BIND(&if_debug);
-  {
-    CallRuntime(Runtime::kDebugPushPromise, context, promise);
-    Goto(&if_debug_done);
-  }
-  BIND(&if_debug_done);
+  BIND(&if_instrumentation_done);
 
   Return(async_function_object);
 }

src/builtins/builtins-async-gen.cc

@@ -97,9 +97,8 @@ Node* AsyncBuiltinsAssembler::AwaitOld(Node* context, Node* generator,
   // also allocates the throwaway promise, which is only needed in
   // case of PromiseHooks or debugging.
   Label if_debugging(this, Label::kDeferred), do_resolve_promise(this);
-  GotoIf(IsDebugActive(), &if_debugging);
-  Branch(IsPromiseHookEnabledOrHasAsyncEventDelegate(), &if_debugging,
-         &do_resolve_promise);
+  Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
+         &if_debugging, &do_resolve_promise);
   BIND(&if_debugging);
   var_throwaway.Bind(CallRuntime(Runtime::kAwaitPromisesInitOld, context, value,
                                  wrapped_value, outer_promise, on_reject,
@@ -169,9 +168,8 @@ Node* AsyncBuiltinsAssembler::AwaitOptimized(Node* context, Node* generator,
   // also allocates the throwaway promise, which is only needed in
   // case of PromiseHooks or debugging.
   Label if_debugging(this, Label::kDeferred), do_perform_promise_then(this);
-  GotoIf(IsDebugActive(), &if_debugging);
-  Branch(IsPromiseHookEnabledOrHasAsyncEventDelegate(), &if_debugging,
-         &do_perform_promise_then);
+  Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
+         &if_debugging, &do_perform_promise_then);
   BIND(&if_debugging);
   var_throwaway.Bind(CallRuntime(Runtime::kAwaitPromisesInit, context, promise,
                                  promise, outer_promise, on_reject,

src/builtins/builtins-internal-gen.cc

@@ -813,8 +813,8 @@ void InternalBuiltinsAssembler::RunPromiseHook(
     Runtime::FunctionId id, TNode<Context> context,
     SloppyTNode<HeapObject> promise_or_capability) {
   Label hook(this, Label::kDeferred), done_hook(this);
-  GotoIf(IsDebugActive(), &hook);
-  Branch(IsPromiseHookEnabledOrHasAsyncEventDelegate(), &hook, &done_hook);
+  Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(), &hook,
+         &done_hook);
   BIND(&hook);
   {
     // Get to the underlying JSPromise instance.

src/builtins/builtins-promise-gen.cc

@@ -1094,8 +1094,8 @@ TF_BUILTIN(PromiseResolveThenableJob, PromiseBuiltinsAssembler) {
   GotoIfNot(WordEqual(then, promise_then), &if_slow);
   Node* const thenable_map = LoadMap(thenable);
   GotoIfNot(IsJSPromiseMap(thenable_map), &if_slow);
-  GotoIf(IsPromiseHookEnabled(), &if_slow);
-  GotoIf(IsDebugActive(), &if_slow);
+  GotoIf(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
+         &if_slow);
   BranchIfPromiseSpeciesLookupChainIntact(native_context, thenable_map,
                                           &if_fast, &if_slow);
 
@@ -1713,8 +1713,8 @@ TF_BUILTIN(RejectPromise, PromiseBuiltinsAssembler) {
   // the runtime handle this operation, which greatly reduces
   // the complexity here and also avoids a couple of back and
   // forth between JavaScript and C++ land.
-  GotoIf(IsPromiseHookEnabled(), &if_runtime);
-  GotoIf(IsDebugActive(), &if_runtime);
+  GotoIf(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
+         &if_runtime);
 
   // 7. If promise.[[PromiseIsHandled]] is false, perform
   //    HostPromiseRejectionTracker(promise, "reject").
@@ -1761,8 +1761,8 @@ TF_BUILTIN(ResolvePromise, PromiseBuiltinsAssembler) {
   // the runtime handle this operation, which greatly reduces
   // the complexity here and also avoids a couple of back and
   // forth between JavaScript and C++ land.
-  GotoIf(IsPromiseHookEnabled(), &if_runtime);
-  GotoIf(IsDebugActive(), &if_runtime);
+  GotoIf(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
+         &if_runtime);
 
   // 6. If SameValue(resolution, promise) is true, then
   // We can use pointer comparison here, since the {promise} is guaranteed
@@ -1861,13 +1861,12 @@ Node* PromiseBuiltinsAssembler::PerformPromiseAll(
     Variable* var_exception) {
   IteratorBuiltinsAssembler iter_assembler(state());
 
-  Node* const instrumenting = IsDebugActive();
   Node* const native_context = LoadNativeContext(context);
 
   // For catch prediction, don't treat the .then calls as handling it;
   // instead, recurse outwards.
   SetForwardingHandlerIfTrue(
-      native_context, instrumenting,
+      native_context, IsDebugActive(),
       LoadObjectField(capability, PromiseCapability::kRejectOffset));
 
   Node* const resolve_element_context =
@@ -1944,8 +1943,8 @@ Node* PromiseBuiltinsAssembler::PerformPromiseAll(
     Label if_fast(this), if_slow(this);
     GotoIfNotPromiseResolveLookupChainIntact(native_context, constructor,
                                              &if_slow);
-    GotoIf(instrumenting, &if_slow);
-    GotoIf(IsPromiseHookEnabledOrHasAsyncEventDelegate(), &if_slow);
+    GotoIf(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
+           &if_slow);
     GotoIf(TaggedIsSmi(next_value), &if_slow);
     Node* const next_value_map = LoadMap(next_value);
     BranchIfPromiseThenLookupChainIntact(native_context, next_value_map,
@@ -1986,7 +1985,7 @@ Node* PromiseBuiltinsAssembler::PerformPromiseAll(
       // For catch prediction, mark that rejections here are semantically
       // handled by the combined Promise.
       SetPromiseHandledByIfTrue(
-          native_context, instrumenting, then_call, [=]() {
+          native_context, IsDebugActive(), then_call, [=]() {
             // Load promiseCapability.[[Promise]]
             return LoadObjectField(capability,
                                    PromiseCapability::kPromiseOffset);
@@ -2267,14 +2266,12 @@ TF_BUILTIN(PromiseRace, PromiseBuiltinsAssembler) {
   Node* const reject =
       LoadObjectField(capability, PromiseCapability::kRejectOffset);
 
-  Node* const instrumenting = IsDebugActive();
-
   Label close_iterator(this, Label::kDeferred);
   Label reject_promise(this, Label::kDeferred);
 
   // For catch prediction, don't treat the .then calls as handling it;
   // instead, recurse outwards.
-  SetForwardingHandlerIfTrue(context, instrumenting, reject);
+  SetForwardingHandlerIfTrue(context, IsDebugActive(), reject);
 
   // Let iterator be GetIterator(iterable).
   // IfAbruptRejectPromise(iterator, promiseCapability).
@@ -2326,7 +2323,7 @@ TF_BUILTIN(PromiseRace, PromiseBuiltinsAssembler) {
 
       // For catch prediction, mark that rejections here are semantically
       // handled by the combined Promise.
-      SetPromiseHandledByIfTrue(context, instrumenting, then_call, [=]() {
+      SetPromiseHandledByIfTrue(context, IsDebugActive(), then_call, [=]() {
         // Load promiseCapability.[[Promise]]
         return LoadObjectField(capability, PromiseCapability::kPromiseOffset);
       });

src/code-stub-assembler.cc

@@ -12956,6 +12956,18 @@ Node* CodeStubAssembler::IsPromiseHookEnabledOrHasAsyncEventDelegate() {
   return Word32NotEqual(promise_hook_or_async_event_delegate, Int32Constant(0));
 }
 
+Node* CodeStubAssembler::
+    IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() {
+  Node* const promise_hook_or_debug_is_active_or_async_event_delegate = Load(
+      MachineType::Uint8(),
+      ExternalConstant(
+          ExternalReference::
+              promise_hook_or_debug_is_active_or_async_event_delegate_address(
+                  isolate())));
+  return Word32NotEqual(promise_hook_or_debug_is_active_or_async_event_delegate,
+                        Int32Constant(0));
+}
+
 TNode<Code> CodeStubAssembler::LoadBuiltin(TNode<Smi> builtin_id) {
   CSA_ASSERT(this, SmiGreaterThanOrEqual(builtin_id, SmiConstant(0)));
   CSA_ASSERT(this,

src/code-stub-assembler.h

@@ -3024,6 +3024,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* IsPromiseHookEnabled();
   Node* HasAsyncEventDelegate();
   Node* IsPromiseHookEnabledOrHasAsyncEventDelegate();
+  Node* IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate();
 
   // Helpers for StackFrame markers.
   Node* MarkerIsFrameType(Node* marker_or_function,

src/debug/debug.cc

@@ -1913,9 +1913,7 @@ void Debug::UpdateState() {
     Unload();
   }
   is_active_ = is_active;
-  if (is_active && isolate_->IsPromiseHookProtectorIntact()) {
-    isolate_->InvalidatePromiseHookProtector();
-  }
+  isolate_->PromiseHookStateUpdated();
 }
 
 void Debug::UpdateHookOnFunctionCall() {

src/external-reference.cc

@@ -865,6 +865,14 @@ ExternalReference::promise_hook_or_async_event_delegate_address(
       isolate->promise_hook_or_async_event_delegate_address());
 }
 
+ExternalReference ExternalReference::
+    promise_hook_or_debug_is_active_or_async_event_delegate_address(
+        Isolate* isolate) {
+  return ExternalReference(
+      isolate
+          ->promise_hook_or_debug_is_active_or_async_event_delegate_address());
+}
+
 ExternalReference ExternalReference::debug_execution_mode_address(
     Isolate* isolate) {
   return ExternalReference(isolate->debug_execution_mode_address());

src/external-reference.h

@@ -53,6 +53,9 @@ class StatsCounter;
   V(async_event_delegate_address, "Isolate::async_event_delegate_address()")   \
   V(promise_hook_or_async_event_delegate_address,                              \
     "Isolate::promise_hook_or_async_event_delegate_address()")                 \
+  V(promise_hook_or_debug_is_active_or_async_event_delegate_address,           \
+    "Isolate::promise_hook_or_debug_is_active_or_async_event_delegate_"        \
+    "address()")                                                               \
   V(debug_execution_mode_address, "Isolate::debug_execution_mode_address()")   \
   V(debug_is_active_address, "Debug::is_active_address()")                     \
   V(debug_hook_on_function_call_address,                                       \

src/isolate.cc

@@ -3622,6 +3622,8 @@ bool Isolate::IsPromiseHookProtectorIntact() {
       Smi::ToInt(promise_hook_cell->value()) == kProtectorValid;
   DCHECK_IMPLIES(is_promise_hook_protector_intact,
                  !promise_hook_or_async_event_delegate_);
+  DCHECK_IMPLIES(is_promise_hook_protector_intact,
+                 !promise_hook_or_debug_is_active_or_async_event_delegate_);
   return is_promise_hook_protector_intact;
 }
 
@@ -3937,12 +3939,18 @@ void Isolate::FireCallCompletedCallback() {
 }
 
 void Isolate::PromiseHookStateUpdated() {
-  bool is_active = promise_hook_ || async_event_delegate_;
-  if (is_active && IsPromiseHookProtectorIntact()) {
+  bool promise_hook_or_async_event_delegate =
+      promise_hook_ || async_event_delegate_;
+  bool promise_hook_or_debug_is_active_or_async_event_delegate =
+      promise_hook_or_async_event_delegate || debug()->is_active();
+  if (promise_hook_or_debug_is_active_or_async_event_delegate &&
+      IsPromiseHookProtectorIntact()) {
     HandleScope scope(this);
     InvalidatePromiseHookProtector();
   }
-  promise_hook_or_async_event_delegate_ = is_active;
+  promise_hook_or_async_event_delegate_ = promise_hook_or_async_event_delegate;
+  promise_hook_or_debug_is_active_or_async_event_delegate_ =
+      promise_hook_or_debug_is_active_or_async_event_delegate;
 }
 
 namespace {

src/isolate.h

@@ -1453,6 +1453,11 @@ class Isolate : private HiddenFactory {
     return reinterpret_cast<Address>(&promise_hook_or_async_event_delegate_);
   }
 
+  Address promise_hook_or_debug_is_active_or_async_event_delegate_address() {
+    return reinterpret_cast<Address>(
+        &promise_hook_or_debug_is_active_or_async_event_delegate_);
+  }
+
   Address handle_scope_implementer_address() {
     return reinterpret_cast<Address>(&handle_scope_implementer_);
   }
@@ -1468,6 +1473,7 @@ class Isolate : private HiddenFactory {
   void SetPromiseHook(PromiseHook hook);
   void RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
                       Handle<Object> parent);
+  void PromiseHookStateUpdated();
 
   void AddDetachedContext(Handle<Context> context);
   void CheckDetachedContextsAfterGC();
@@ -1712,7 +1718,6 @@ class Isolate : private HiddenFactory {
 
   void SetTerminationOnExternalTryCatch();
 
-  void PromiseHookStateUpdated();
   void RunPromiseHookForAsyncEventDelegate(PromiseHookType type,
                                            Handle<JSPromise> promise);
 
@@ -1907,6 +1912,7 @@ class Isolate : private HiddenFactory {
 
   debug::AsyncEventDelegate* async_event_delegate_ = nullptr;
   bool promise_hook_or_async_event_delegate_ = false;
+  bool promise_hook_or_debug_is_active_or_async_event_delegate_ = false;
   int async_task_count_ = 0;
 
   v8::Isolate::AbortOnUncaughtExceptionCallback

src/runtime/runtime-debug.cc

@@ -746,11 +746,13 @@ RUNTIME_FUNCTION(Runtime_IncBlockCounter) {
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
-RUNTIME_FUNCTION(Runtime_DebugAsyncFunctionSuspended) {
+RUNTIME_FUNCTION(Runtime_DebugAsyncFunctionEntered) {
   DCHECK_EQ(1, args.length());
   HandleScope scope(isolate);
   CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
-  isolate->OnAsyncFunctionStateChanged(promise, debug::kAsyncFunctionSuspended);
+  isolate->RunPromiseHook(PromiseHookType::kInit, promise,
+                          isolate->factory()->undefined_value());
+  if (isolate->debug()->is_active()) isolate->PushPromise(promise);
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
@@ -767,6 +769,14 @@ RUNTIME_FUNCTION(Runtime_DebugAsyncFunctionFinished) {
   return *promise;
 }
 
+RUNTIME_FUNCTION(Runtime_DebugAsyncFunctionSuspended) {
+  DCHECK_EQ(1, args.length());
+  HandleScope scope(isolate);
+  CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
+  isolate->OnAsyncFunctionStateChanged(promise, debug::kAsyncFunctionSuspended);
+  return ReadOnlyRoots(isolate).undefined_value();
+}
+
 RUNTIME_FUNCTION(Runtime_LiveEditPatchScript) {
   HandleScope scope(isolate);
   DCHECK_EQ(2, args.length());

src/runtime/runtime.h

@@ -129,8 +129,9 @@ namespace internal {
   F(DebugPopPromise, 0, 1)                      \
   F(DebugPrepareStepInSuspendedGenerator, 0, 1) \
   F(DebugPushPromise, 1, 1)                     \
-  F(DebugAsyncFunctionSuspended, 1, 1)          \
+  F(DebugAsyncFunctionEntered, 1, 1)            \
   F(DebugAsyncFunctionFinished, 2, 1)           \
+  F(DebugAsyncFunctionSuspended, 1, 1)          \
   F(DebugToggleBlockCoverage, 1, 1)             \
   F(DebugTogglePreciseCoverage, 1, 1)           \
   F(FunctionGetInferredName, 1, 1)              \