deps: V8: backport 970d651e4f99

Original commit message:

    [api][module] Type-check synthetic module evaluation steps

    Synthetic module evaluation steps are required to return a Promise,
    which becomes the module's top-level capability. Their signature
    returned a MaybeLocal<Value> though, so that requirement was only
    enforced by a CHECK in SyntheticModule::Evaluate().

    SyntheticModuleEvaluationSteps now returns a MaybeLocal<Promise>, with a
    matching CreateSyntheticModule() overload. The old signature remains
    available as LegacySyntheticModuleEvaluationSteps so that embedders can
    be migrated in a separate CL; it will be
    deprecated and then removed once embedders migrate.

    d8 and the existing tests move to the Promise-returning version,
    with one cctest checking the legacy version.

    Bug: 545375591
    Change-Id: Id55db730678455f81394bf8a68f66f93d22f5547
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8223168
    Reviewed-by: Olivier Flückiger <olivf@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Commit-Queue: Caio Lima <caiolima@igalia.com>
    Cr-Commit-Position: refs/heads/main@{#109273}

Refs: v8/v8@970d651
Co-authored-by: Caio Lima <caiolima@igalia.com>

Author: caiolima

common.gypi

@@ -42,7 +42,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.29',
+    'v8_embedder_string': '-node.30',
 
     ##### V8 defaults for Node.js #####
 

deps/v8/include/v8-script.h

@@ -336,9 +336,16 @@ class V8_EXPORT Module : public Data {
    * exception was thrown) and return an empy MaybeLocal to indicate falure
    * (where an exception was thrown).
    */
-  // TODO(caiolima): Change this to `MaybeLocal<Promise>` given it's expected a
-  // Promise as return.
   using SyntheticModuleEvaluationSteps =
+      MaybeLocal<Promise> (*)(Local<Context> context, Local<Module> module);
+
+  /*
+   * Deprecated version of SyntheticModuleEvaluationSteps: the returned value is
+   * still required to be a Promise, but that is only enforced at runtime.
+   */
+  // TODO(https://crbug.com/545375591): Remove once all embedders return a
+  // MaybeLocal<Promise>.
+  using LegacySyntheticModuleEvaluationSteps =
       MaybeLocal<Value> (*)(Local<Context> context, Local<Module> module);
 
   /**
@@ -353,6 +360,16 @@ class V8_EXPORT Module : public Data {
       const MemorySpan<const Local<String>>& export_names,
       SyntheticModuleEvaluationSteps evaluation_steps);
 
+  // TODO(https://crbug.com/545375591): Advance to V8_DEPRECATED and then remove
+  // this overload once all embedders have been migrated to the one above.
+  V8_DEPRECATE_SOON(
+      "Use the CreateSyntheticModule overload whose evaluation_steps return a "
+      "MaybeLocal<Promise>")
+  static Local<Module> CreateSyntheticModule(
+      Isolate* isolate, Local<String> module_name,
+      const MemorySpan<const Local<String>>& export_names,
+      LegacySyntheticModuleEvaluationSteps evaluation_steps);
+
   /**
    * Set this module's exported value for the name export_name to the specified
    * export_value. This method must be called only on Modules created via

deps/v8/src/api/api.cc

@@ -2436,6 +2436,33 @@ Local<Module> Module::CreateSyntheticModule(
           i_module_name, i_export_names, evaluation_steps)));
 }
 
+START_ALLOW_USE_DEPRECATED()
+Local<Module> Module::CreateSyntheticModule(
+    Isolate* v8_isolate, Local<String> module_name,
+    const MemorySpan<const Local<String>>& export_names,
+    v8::Module::LegacySyntheticModuleEvaluationSteps evaluation_steps) {
+  // TODO(https://crbug.com/545375591): Remove once
+  // LegacySyntheticModuleEvaluationSteps is gone.
+#if (__GNUC__ >= 8) || defined(__clang__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-function-type"
+#endif
+  // Cast from 'v8::MaybeLocal<v8::Value> (*)(v8::Local<v8::Context>,
+  // v8::Local<v8::Module>)' to 'v8::MaybeLocal<v8::Promise>
+  // (*)(v8::Local<v8::Context>, v8::Local<v8::Module>)'. Both return types are
+  // pointer-sized, trivially copyable handle wrappers, so they share the same
+  // representation. SyntheticModule::Evaluate() checks at runtime that the
+  // returned value really is a Promise.
+  auto promise_returning_steps =
+      reinterpret_cast<SyntheticModuleEvaluationSteps>(evaluation_steps);
+#if (__GNUC__ >= 8) || defined(__clang__)
+#pragma GCC diagnostic pop
+#endif
+  return CreateSyntheticModule(v8_isolate, module_name, export_names,
+                               promise_returning_steps);
+}
+END_ALLOW_USE_DEPRECATED()
+
 Maybe<bool> Module::SetSyntheticModuleExport(Isolate* v8_isolate,
                                              Local<String> export_name,
                                              Local<v8::Value> export_value) {

deps/v8/src/d8/d8.cc

@@ -1441,7 +1441,7 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Module> referrer,
   return result;
 }
 
-MaybeLocal<Value> Shell::JSONModuleEvaluationSteps(Local<Context> context,
+MaybeLocal<Promise> Shell::JSONModuleEvaluationSteps(Local<Context> context,
                                                    Local<Module> module) {
   Isolate* isolate = Isolate::GetCurrent();
 

deps/v8/src/d8/d8.h

@@ -884,9 +884,8 @@ class Shell : public i::AllStatic {
                                             const std::string& file_name,
                                             ModuleType module_type);
 
-  static MaybeLocal<Value> JSONModuleEvaluationSteps(Local<Context> context,
-                                                     Local<Module> module);
-
+  static MaybeLocal<Promise> JSONModuleEvaluationSteps(Local<Context> context,
+                                                       Local<Module> module);
   template <class T>
   static MaybeLocal<T> CompileString(Isolate* isolate, Local<Context> context,
                                      Local<String> source,

deps/v8/src/objects/synthetic-module.cc

@@ -5,6 +5,7 @@
 #include "src/objects/synthetic-module.h"
 
 #include "src/api/api-inl.h"
+#include "src/base/macros.h"
 #include "src/builtins/accessors.h"
 #include "src/objects/js-generator-inl.h"
 #include "src/objects/module-inl.h"
@@ -105,13 +106,24 @@ bool SyntheticModule::FinishInstantiate(Isolate* isolate,
 
 // Implements Synthetic Module Record's Evaluate concrete method:
 // https://heycam.github.io/webidl/#smr-evaluate
+// The callback may have been created through the deprecated
+// v8::Module::LegacySyntheticModuleEvaluationSteps overload, in which case it
+// actually returns a v8::MaybeLocal<v8::Value> and is called here through a
+// mismatching signature. Both return types are pointer-sized, trivially
+// copyable handle wrappers, so this is safe in practice, but it does trip
+// CFI's and UBSan's indirect call checks.
+// TODO(https://crbug.com/545375591): Remove DISABLE_CFI_ICALL once the
+// deprecated overload is gone.
+DISABLE_CFI_ICALL
 MaybeDirectHandle<JSPromise> SyntheticModule::Evaluate(
     Isolate* isolate, DirectHandle<SyntheticModule> module) {
   module->SetStatus(kEvaluating);
 
   v8::Module::SyntheticModuleEvaluationSteps evaluation_steps =
       FUNCTION_CAST<v8::Module::SyntheticModuleEvaluationSteps>(
           module->evaluation_steps()->foreign_address<kSyntheticModuleTag>());
+  // Deliberately received as a v8::Local<v8::Value>: the deprecated callback
+  // signature only promises a Promise, it doesn't guarantee one.
   v8::Local<v8::Value> result;
   if (!evaluation_steps(Utils::ToLocal(isolate->native_context()),
                         Utils::ToLocal(Cast<Module>(module)))

deps/v8/test/cctest/test-api.cc

@@ -24701,37 +24701,48 @@ TEST(CodeCache) {
   isolate2->Dispose();
 }
 
-v8::MaybeLocal<Value> UnexpectedSyntheticModuleEvaluationStepsCallback(
+v8::MaybeLocal<Promise> UnexpectedSyntheticModuleEvaluationStepsCallback(
     Local<Context> context, Local<Module> module) {
   CHECK_WITH_MSG(false, "Unexpected call to synthetic module re callback");
 }
 
 static int synthetic_module_callback_count;
 
-v8::MaybeLocal<Value> SyntheticModuleEvaluationStepsCallback(
+v8::MaybeLocal<Promise> SyntheticModuleEvaluationStepsCallback(
     Local<Context> context, Local<Module> module) {
   synthetic_module_callback_count++;
-  // Synthetic module evaluation steps must return a Promise.
   Local<v8::Promise::Resolver> resolver =
       v8::Promise::Resolver::New(context).ToLocalChecked();
   resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check();
   return resolver->GetPromise();
 }
 
-v8::MaybeLocal<Value> SyntheticModuleEvaluationStepsCallbackFail(
+v8::MaybeLocal<Promise> SyntheticModuleEvaluationStepsCallbackFail(
     Local<Context> context, Local<Module> module) {
   synthetic_module_callback_count++;
   CcTest::isolate()->ThrowException(
       v8_str("SyntheticModuleEvaluationStepsCallbackFail exception"));
-  return v8::MaybeLocal<Value>();
+  return v8::MaybeLocal<Promise>();
 }
 
-v8::MaybeLocal<Value> SyntheticModuleEvaluationStepsCallbackSetExport(
+// Deprecated version of the evaluation steps, returning a MaybeLocal<Value>
+// that holds a Promise.
+// TODO(https://crbug.com/545375591): Remove together with
+// v8::Module::LegacySyntheticModuleEvaluationSteps.
+v8::MaybeLocal<Value> LegacySyntheticModuleEvaluationStepsCallback(
+    Local<Context> context, Local<Module> module) {
+  synthetic_module_callback_count++;
+  Local<v8::Promise::Resolver> resolver =
+      v8::Promise::Resolver::New(context).ToLocalChecked();
+  resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check();
+  return resolver->GetPromise();
+}
+
+v8::MaybeLocal<Promise> SyntheticModuleEvaluationStepsCallbackSetExport(
     Local<Context> context, Local<Module> module) {
   Maybe<bool> set_export_result = module->SetSyntheticModuleExport(
       CcTest::isolate(), v8_str("test_export"), v8_num(42));
   CHECK(set_export_result.FromJust());
-  // Synthetic module evaluation steps must return a Promise.
   Local<v8::Promise::Resolver> resolver =
       v8::Promise::Resolver::New(context).ToLocalChecked();
   resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check();
@@ -25074,6 +25085,40 @@ TEST(SyntheticModuleEvaluationStepsNoThrow) {
   CHECK_EQ(module->GetStatus(), Module::kEvaluated);
 }
 
+// Covers the deprecated evaluation steps version, where the returned Promise is
+// only checked at runtime.
+// TODO(https://crbug.com/545375591): Remove together with
+// v8::Module::LegacySyntheticModuleEvaluationSteps.
+TEST(SyntheticModuleEvaluationStepsLegacyCallback) {
+  synthetic_module_callback_count = 0;
+  LocalContext env;
+  v8::Isolate* isolate = env.isolate();
+  v8::Isolate::Scope iscope(isolate);
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Context> context = v8::Context::New(isolate);
+  v8::Context::Scope cscope(context);
+
+  auto export_names = std::to_array<Local<v8::String>>({v8_str("default")});
+
+  START_ALLOW_USE_DEPRECATED()
+  Local<Module> module = v8::Module::CreateSyntheticModule(
+      isolate,
+      v8_str("SyntheticModuleEvaluationStepsLegacyCallback-"
+             "TestSyntheticModule"),
+      export_names, LegacySyntheticModuleEvaluationStepsCallback);
+  END_ALLOW_USE_DEPRECATED()
+  module->InstantiateModule(context, UnexpectedModuleResolveCallback)
+      .ToChecked();
+
+  CHECK_EQ(synthetic_module_callback_count, 0);
+  Local<Value> completion_value = module->Evaluate(context).ToLocalChecked();
+  CHECK(completion_value->IsPromise());
+  Local<v8::Promise> promise(Local<v8::Promise>::Cast(completion_value));
+  CHECK_EQ(promise->State(), v8::Promise::kFulfilled);
+  CHECK_EQ(synthetic_module_callback_count, 1);
+  CHECK_EQ(module->GetStatus(), Module::kEvaluated);
+}
+
 TEST(SyntheticModuleEvaluationStepsThrow) {
   synthetic_module_callback_count = 0;
   LocalContext env;
@@ -27098,7 +27143,7 @@ MaybeLocal<Module> CheckResolveModuleWithImportSource(
 
   return v8::Module::CreateSyntheticModule(
       isolate, v8_str("my-mod"), {},
-      [](Local<Context> context, Local<Module> module) -> MaybeLocal<Value> {
+      [](Local<Context> context, Local<Module> module) -> MaybeLocal<Promise> {
         // Do nothing.
         Local<v8::Promise::Resolver> resolver =
             v8::Promise::Resolver::New(context).ToLocalChecked();

deps/v8/test/unittests/objects/modules-unittest.cc

@@ -1686,7 +1686,7 @@ TEST_F(ModuleTest, SyntheticModuleGetResourceName) {
   Local<String> resource_name = NewString("synthetic-module");
   Local<Module> module = Module::CreateSyntheticModule(
       isolate(), resource_name, {},
-      [](Local<Context> context, Local<Module> module) -> MaybeLocal<Value> {
+      [](Local<Context> context, Local<Module> module) -> MaybeLocal<Promise> {
         // Do nothing.
         Local<v8::Promise::Resolver> resolver =
             v8::Promise::Resolver::New(context).ToLocalChecked();
@@ -1716,12 +1716,12 @@ TEST_F(ModuleTest, SyntheticModuleGetResourceNameInError) {
   Local<String> resource_name = NewString("synthetic-module");
   Local<Module> module = Module::CreateSyntheticModule(
       isolate(), resource_name, {},
-      [](Local<Context> context, Local<Module> module) -> MaybeLocal<Value> {
+      [](Local<Context> context, Local<Module> module) -> MaybeLocal<Promise> {
         // Throw an error.
         Isolate* isolate = Isolate::GetCurrent();
         isolate->ThrowException(
             v8::String::NewFromUtf8Literal(isolate, "synthetic module error"));
-        return MaybeLocal<Value>();
+        return MaybeLocal<Promise>();
       });
 
   CHECK_EQ(Module::kUninstantiated, module->GetStatus());