[tq][runtime] Use build flags for JS context promise hooks

Use build_flags_ with @if/@ifnot in torque for the following flags:
- V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
- V8_ENABLE_SWISS_NAME_DICTIONARY

- Make sure Torque and CSA code actually respect
  V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS.
- Rename V8_ALLOW_JAVASCRIPT_IN_PROMISE_HOOKS to
  V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
- Rename gn/bazel arg v8_allow_javascript_in_promise_hooks to
  v8_enable_javascript_promise_hooks
- Unship context promise hooks in chrome and enable them only in d8
  for testing purposes
- Make sure d8 and the API throw when using promise hooks without
  the compile time feature enabled

Bug: chromium:1265186, v8:11025
Change-Id: I69834d44d683a36d0d7be3c3d68888321be0fd7f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3301474Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78362}

BUILD.bazel

@@ -84,7 +84,7 @@ config_setting(
 # v8_generate_external_defines_header
 # v8_dict_property_const_tracking
 # v8_enable_map_packing
-# v8_allow_javascript_in_promise_hooks
+# v8_enable_javascript_promise_hooks
 # v8_enable_allocation_folding
 # v8_allocation_site_tracking
 

BUILD.gn

@@ -333,7 +333,7 @@ declare_args() {
   v8_enable_map_packing = false
 
   # Allow for JS promise hooks (instead of just C++).
-  v8_allow_javascript_in_promise_hooks = false
+  v8_enable_javascript_promise_hooks = false
 
   # Enable allocation folding globally (sets -dV8_ALLOCATION_FOLDING).
   # When it's disabled, the --turbo-allocation-folding runtime flag will be ignored.
@@ -945,8 +945,8 @@ config("features") {
   if (v8_dict_property_const_tracking) {
     defines += [ "V8_DICT_PROPERTY_CONST_TRACKING" ]
   }
-  if (v8_allow_javascript_in_promise_hooks) {
-    defines += [ "V8_ALLOW_JAVASCRIPT_IN_PROMISE_HOOKS" ]
+  if (v8_enable_javascript_promise_hooks) {
+    defines += [ "V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS" ]
   }
   if (v8_enable_allocation_folding) {
     defines += [ "V8_ALLOCATION_FOLDING" ]

src/api/api.cc

@@ -6602,6 +6602,7 @@ void v8::Context::SetPromiseHooks(Local<Function> init_hook,
                                   Local<Function> before_hook,
                                   Local<Function> after_hook,
                                   Local<Function> resolve_hook) {
+#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   i::Handle<i::Context> context = Utils::OpenHandle(this);
   i::Isolate* isolate = context->GetIsolate();
 
@@ -6635,6 +6636,10 @@ void v8::Context::SetPromiseHooks(Local<Function> init_hook,
   context->native_context().set_promise_hook_before_function(*before);
   context->native_context().set_promise_hook_after_function(*after);
   context->native_context().set_promise_hook_resolve_function(*resolve);
+#else   // V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
+  Utils::ApiCheck(false, "v8::Context::SetPromiseHook",
+                  "V8 was compiled without JavaScript Promise hooks");
+#endif  // V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
 }
 
 MaybeLocal<Context> metrics::Recorder::GetContext(

src/builtins/base.tq

@@ -485,9 +485,6 @@ const kWasmArrayHeaderSize:
 const kHeapObjectHeaderSize:
     constexpr int32 generates 'HeapObject::kHeaderSize';
 
-const kDictModePrototypes:
-    constexpr bool generates 'V8_ENABLE_SWISS_NAME_DICTIONARY_BOOL';
-
 type TheHole extends Oddball;
 type Null extends Oddball;
 type Undefined extends Oddball;

src/builtins/builtins-async-gen.cc

@@ -183,6 +183,7 @@ void AsyncBuiltinsAssembler::InitAwaitPromise(
   Goto(&do_nothing);
   BIND(&not_debugging);
 
+#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   // This call to NewJSPromise is to keep behaviour parity with what happens
   // in Runtime::kAwaitPromisesInit above if native hooks are set. It will
   // create a throwaway promise that will trigger an init event and will get
@@ -191,6 +192,7 @@ void AsyncBuiltinsAssembler::InitAwaitPromise(
          &do_nothing);
   BIND(&if_promise_hook);
   *var_throwaway = NewJSPromise(context, promise);
+#endif  // V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   Goto(&do_nothing);
   BIND(&do_nothing);
 }

src/builtins/object.tq

@@ -99,10 +99,11 @@ transitioning builtin CreateObjectWithoutProperties(implicit context: Context)(
       case (Null): {
         map = *NativeContextSlot(
             ContextSlot::SLOW_OBJECT_WITH_NULL_PROTOTYPE_MAP);
-        if (kDictModePrototypes) {
+        @if(V8_ENABLE_SWISS_NAME_DICTIONARY) {
           properties =
               AllocateSwissNameDictionary(kSwissNameDictionaryInitialCapacity);
-        } else {
+        }
+        @ifnot(V8_ENABLE_SWISS_NAME_DICTIONARY) {
           properties = AllocateNameDictionary(kNameDictionaryInitialCapacity);
         }
       }

src/builtins/promise-misc.tq

@@ -120,9 +120,13 @@ transitioning macro RunContextPromiseHookInit(implicit context: Context)(
 @export
 transitioning macro RunContextPromiseHookResolve(implicit context: Context)(
     promise: JSPromise): void {
-  RunContextPromiseHook(
-      ContextSlot::PROMISE_HOOK_RESOLVE_FUNCTION_INDEX, promise,
-      PromiseHookFlags());
+  // Use potentially unused variables.
+  const _unusedPromise = promise;
+  @if(V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS) {
+    RunContextPromiseHook(
+        ContextSlot::PROMISE_HOOK_RESOLVE_FUNCTION_INDEX, promise,
+        PromiseHookFlags());
+  }
 }
 
 @export
@@ -135,9 +139,13 @@ transitioning macro RunContextPromiseHookResolve(implicit context: Context)(
 @export
 transitioning macro RunContextPromiseHookBefore(implicit context: Context)(
     promiseOrCapability: JSPromise|PromiseCapability|Undefined): void {
-  RunContextPromiseHook(
-      ContextSlot::PROMISE_HOOK_BEFORE_FUNCTION_INDEX, promiseOrCapability,
-      PromiseHookFlags());
+  // Use potentially unused variables.
+  const _unusedPromiseOrCapability = promiseOrCapability;
+  @if(V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS) {
+    RunContextPromiseHook(
+        ContextSlot::PROMISE_HOOK_BEFORE_FUNCTION_INDEX, promiseOrCapability,
+        PromiseHookFlags());
+  }
 }
 
 @export
@@ -152,9 +160,13 @@ transitioning macro RunContextPromiseHookBefore(implicit context: Context)(
 @export
 transitioning macro RunContextPromiseHookAfter(implicit context: Context)(
     promiseOrCapability: JSPromise|PromiseCapability|Undefined): void {
-  RunContextPromiseHook(
-      ContextSlot::PROMISE_HOOK_AFTER_FUNCTION_INDEX, promiseOrCapability,
-      PromiseHookFlags());
+  // Use potentially unused variables.
+  const _unusedPromiseOrCapability = promiseOrCapability;
+  @if(V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS) {
+    RunContextPromiseHook(
+        ContextSlot::PROMISE_HOOK_AFTER_FUNCTION_INDEX, promiseOrCapability,
+        PromiseHookFlags());
+  }
 }
 
 @export
@@ -170,27 +182,33 @@ transitioning macro RunContextPromiseHook(implicit context: Context)(
     slot: Slot<NativeContext, Undefined|Callable>,
     promiseOrCapability: JSPromise|PromiseCapability|Undefined,
     flags: uint32): void {
-  if (!IsContextPromiseHookEnabled(flags)) return;
-  const maybeHook = *NativeContextSlot(slot);
-  const hook = Cast<Callable>(maybeHook) otherwise return;
-
-  let promise: JSPromise;
-  typeswitch (promiseOrCapability) {
-    case (jspromise: JSPromise): {
-      promise = jspromise;
-    }
-    case (capability: PromiseCapability): {
-      promise = Cast<JSPromise>(capability.promise) otherwise return;
-    }
-    case (Undefined): {
-      return;
+  // Use potentially unused variables.
+  const _unusedSlot = slot;
+  const _unusedPromiseOrCapability = promiseOrCapability;
+  const _unusedFlags = flags;
+  @if(V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS) {
+    if (!IsContextPromiseHookEnabled(flags)) return;
+    const maybeHook = *NativeContextSlot(slot);
+    const hook = Cast<Callable>(maybeHook) otherwise return;
+
+    let promise: JSPromise;
+    typeswitch (promiseOrCapability) {
+      case (jspromise: JSPromise): {
+        promise = jspromise;
+      }
+      case (capability: PromiseCapability): {
+        promise = Cast<JSPromise>(capability.promise) otherwise return;
+      }
+      case (Undefined): {
+        return;
+      }
     }
-  }
 
-  try {
-    Call(context, hook, Undefined, promise);
-  } catch (e, _message) {
-    runtime::ReportMessageFromMicrotask(e);
+    try {
+      Call(context, hook, Undefined, promise);
+    } catch (e, _message) {
+      runtime::ReportMessageFromMicrotask(e);
+    }
   }
 }
 
@@ -199,8 +217,10 @@ transitioning macro RunAnyPromiseHookInit(implicit context: Context)(
   const promiseHookFlags = PromiseHookFlags();
   // Fast return if no hooks are set.
   if (promiseHookFlags == 0) return;
-  if (IsContextPromiseHookEnabled(promiseHookFlags)) {
-    RunContextPromiseHookInit(promise, parent);
+  @if(V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS) {
+    if (IsContextPromiseHookEnabled(promiseHookFlags)) {
+      RunContextPromiseHookInit(promise, parent);
+    }
   }
   if (IsIsolatePromiseHookEnabledOrHasAsyncEventDelegate(promiseHookFlags)) {
     runtime::PromiseHookInit(promise, parent);

src/codegen/code-stub-assembler.cc

@@ -14381,10 +14381,12 @@ TNode<BoolT> CodeStubAssembler::IsAnyPromiseHookEnabled(TNode<Uint32T> flags) {
   return IsSetWord32(flags, mask);
 }
 
+#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
 TNode<BoolT> CodeStubAssembler::IsContextPromiseHookEnabled(
     TNode<Uint32T> flags) {
   return IsSetWord32<Isolate::PromiseHookFields::HasContextPromiseHook>(flags);
 }
+#endif
 
 TNode<BoolT> CodeStubAssembler::
     IsIsolatePromiseHookEnabledOrHasAsyncEventDelegate(TNode<Uint32T> flags) {

src/codegen/code-stub-assembler.h

@@ -3660,10 +3660,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   // Promise helpers
   TNode<Uint32T> PromiseHookFlags();
   TNode<BoolT> HasAsyncEventDelegate();
+#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   TNode<BoolT> IsContextPromiseHookEnabled(TNode<Uint32T> flags);
-  TNode<BoolT> IsContextPromiseHookEnabled() {
-    return IsContextPromiseHookEnabled(PromiseHookFlags());
-  }
+#endif
   TNode<BoolT> IsAnyPromiseHookEnabled(TNode<Uint32T> flags);
   TNode<BoolT> IsAnyPromiseHookEnabled() {
     return IsAnyPromiseHookEnabled(PromiseHookFlags());

src/d8/d8.cc

@@ -2164,6 +2164,7 @@ void Shell::SetPromiseHooks(const v8::FunctionCallbackInfo<v8::Value>& args) {
         "--correctness-fuzzer-suppressions");
     return;
   }
+#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   Local<Context> context = isolate->GetCurrentContext();
   HandleScope handle_scope(isolate);
 
@@ -2174,6 +2175,11 @@ void Shell::SetPromiseHooks(const v8::FunctionCallbackInfo<v8::Value>& args) {
     args[3]->IsFunction() ? args[3].As<Function>() : Local<Function>());
 
   args.GetReturnValue().Set(v8::Undefined(isolate));
+#else   // V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
+  isolate->ThrowError(
+      "d8.promise.setHooks is disabled due to missing build flag "
+      "v8_enabale_javascript_in_promise_hooks");
+#endif  // V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
 }
 
 void WriteToFile(FILE* file, const v8::FunctionCallbackInfo<v8::Value>& args) {

src/execution/isolate.h

@@ -1569,11 +1569,13 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   }
 #endif
 
+#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   void SetHasContextPromiseHooks(bool context_promise_hook) {
     promise_hook_flags_ = PromiseHookFields::HasContextPromiseHook::update(
         promise_hook_flags_, context_promise_hook);
     PromiseHookStateUpdated();
   }
+#endif  // V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
 
   bool HasContextPromiseHooks() const {
     return PromiseHookFields::HasContextPromiseHook::decode(

src/objects/js-objects.tq

@@ -78,10 +78,11 @@ macro AllocateFastOrSlowJSObjectFromMap(implicit context: Context)(map: Map):
   let properties: EmptyFixedArray|NameDictionary|SwissNameDictionary =
       kEmptyFixedArray;
   if (IsDictionaryMap(map)) {
-    if (kDictModePrototypes) {
+    @if(V8_ENABLE_SWISS_NAME_DICTIONARY) {
       properties =
           AllocateSwissNameDictionary(kSwissNameDictionaryInitialCapacity);
-    } else {
+    }
+    @ifnot(V8_ENABLE_SWISS_NAME_DICTIONARY) {
       properties = AllocateNameDictionary(kNameDictionaryInitialCapacity);
     }
   }

src/torque/torque-parser.cc

@@ -48,6 +48,13 @@ class BuildFlags : public ContextualClass<BuildFlags> {
     build_flags_["V8_SFI_HAS_UNIQUE_ID"] = V8_SFI_HAS_UNIQUE_ID;
     build_flags_["V8_EXTERNAL_CODE_SPACE"] = V8_EXTERNAL_CODE_SPACE_BOOL;
     build_flags_["TAGGED_SIZE_8_BYTES"] = TAGGED_SIZE_8_BYTES;
+    build_flags_["V8_ENABLE_SWISS_NAME_DICTIONARY"] =
+        V8_ENABLE_SWISS_NAME_DICTIONARY_BOOL;
+#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
+    build_flags_["V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS"] = true;
+#else
+    build_flags_["V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS"] = false;
+#endif
     build_flags_["TRUE_FOR_TESTING"] = true;
     build_flags_["FALSE_FOR_TESTING"] = false;
 #ifdef V8_SCRIPTORMODULE_LEGACY_LIFETIME

test/mjsunit/promise-hooks.js

@@ -37,6 +37,14 @@ function printLog(message) {
   }
 }
 
+let has_promise_hooks = false;
+try {
+  d8.promise.setHooks();
+  has_promise_hooks = true;
+} catch {
+  has_promise_hooks = false;
+}
+
 function assertNextEvent(type, args) {
   const [ promiseOrId, parentOrId ] = args;
   const nextEvent = log.shift();
@@ -212,72 +220,75 @@ function optimizerBailout(test, verify) {
   d8.promise.setHooks();
 }
 
-optimizerBailout(async () => {
-  await Promise.resolve();
-}, () => {
-  assertNextEvent('init', [ 1 ]);
-  assertNextEvent('init', [ 2 ]);
-  assertNextEvent('resolve', [ 2 ]);
-  assertNextEvent('init', [ 3, 2 ]);
-  assertNextEvent('before', [ 3 ]);
-  assertNextEvent('resolve', [ 1 ]);
-  assertNextEvent('resolve', [ 3 ]);
-  assertNextEvent('after', [ 3 ]);
-  assertEmptyLog();
-});
-optimizerBailout(async () => {
-  await { then (cb) { cb() } };
-}, () => {
-  assertNextEvent('init', [ 1 ]);
-  assertNextEvent('init', [ 2, 1 ]);
-  assertNextEvent('init', [ 3, 2 ]);
-  assertNextEvent('before', [ 2 ]);
-  assertNextEvent('resolve', [ 2 ]);
-  assertNextEvent('after', [ 2 ]);
-  assertNextEvent('before', [ 3 ]);
-  assertNextEvent('resolve', [ 1 ]);
-  assertNextEvent('resolve', [ 3 ]);
-  assertNextEvent('after', [ 3 ]);
-  assertEmptyLog();
-});
-basicTest();
-exceptions();
-
-(function regress1126309() {
-  function __f_16(test) {
-    test();
-    d8.promise.setHooks(undefined, () => {});
+if (has_promise_hooks) {
+  optimizerBailout(async () => {
+    await Promise.resolve();
+  }, () => {
+    assertNextEvent('init', [ 1 ]);
+    assertNextEvent('init', [ 2 ]);
+    assertNextEvent('resolve', [ 2 ]);
+    assertNextEvent('init', [ 3, 2 ]);
+    assertNextEvent('before', [ 3 ]);
+    assertNextEvent('resolve', [ 1 ]);
+    assertNextEvent('resolve', [ 3 ]);
+    assertNextEvent('after', [ 3 ]);
+    assertEmptyLog();
+  });
+  optimizerBailout(async () => {
+    await { then (cb) { cb() } };
+  }, () => {
+    assertNextEvent('init', [ 1 ]);
+    assertNextEvent('init', [ 2, 1 ]);
+    assertNextEvent('init', [ 3, 2 ]);
+    assertNextEvent('before', [ 2 ]);
+    assertNextEvent('resolve', [ 2 ]);
+    assertNextEvent('after', [ 2 ]);
+    assertNextEvent('before', [ 3 ]);
+    assertNextEvent('resolve', [ 1 ]);
+    assertNextEvent('resolve', [ 3 ]);
+    assertNextEvent('after', [ 3 ]);
+    assertEmptyLog();
+  });
+  basicTest();
+  exceptions();
+
+  (function regress1126309() {
+    function __f_16(test) {
+      test();
+      d8.promise.setHooks(undefined, () => {});
+      %PerformMicrotaskCheckpoint();
+      d8.promise.setHooks();
+    }
+    __f_16(async () => { await Promise.resolve()});
+  })();
+
+  (function boundFunction() {
+    function hook() {};
+    const bound = hook.bind(this);
+    d8.promise.setHooks(bound, bound, bound, bound);
+    Promise.resolve();
+    Promise.reject();
     %PerformMicrotaskCheckpoint();
     d8.promise.setHooks();
-  }
-  __f_16(async () => { await Promise.resolve()});
-})();
-
-(function boundFunction() {
-  function hook() {};
-  const bound = hook.bind(this);
-  d8.promise.setHooks(bound, bound, bound, bound);
-  Promise.resolve();
-  Promise.reject();
-  %PerformMicrotaskCheckpoint();
-  d8.promise.setHooks();
-})();
+  })();
 
 
-(function promiseAll() {
-  let initCount = 0;
-  d8.promise.setHooks(() => { initCount++});
-  Promise.all([Promise.resolve(1)]);
-  %PerformMicrotaskCheckpoint();
-  assertEquals(initCount, 3);
+  (function promiseAll() {
+    let initCount = 0;
+    d8.promise.setHooks(() => { initCount++});
+    Promise.all([Promise.resolve(1)]);
+    %PerformMicrotaskCheckpoint();
+    assertEquals(initCount, 3);
 
-  d8.promise.setHooks();
-})();
+    d8.promise.setHooks();
+  })();
 
-(function overflow(){
-  d8.promise.setHooks(() => { new Promise(()=>{}) });
-  // Trigger overflow from JS code:
-  Promise.all([Promise.resolve(1)]);
-  %PerformMicrotaskCheckpoint();
-  d8.promise.setHooks();
-});
+  (function overflow(){
+    d8.promise.setHooks(() => { new Promise(()=>{}) });
+    // Trigger overflow from JS code:
+    Promise.all([Promise.resolve(1)]);
+    %PerformMicrotaskCheckpoint();
+    d8.promise.setHooks();
+  });
+
+}