diff --git a/common.gypi b/common.gypi index 0b01ec8c49fe..377377b65c5d 100644 --- a/common.gypi +++ b/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.28', + 'v8_embedder_string': '-node.30', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-script.h b/deps/v8/include/v8-script.h index c3a2274d4333..f7a97a0fc370 100644 --- a/deps/v8/include/v8-script.h +++ b/deps/v8/include/v8-script.h @@ -280,7 +280,7 @@ class V8_EXPORT Module : public Data { * * If IsGraphAsync() is false, the returned Promise is settled. */ - V8_WARN_UNUSED_RESULT MaybeLocal Evaluate(Local context); + V8_WARN_UNUSED_RESULT MaybeLocal Evaluate(Local context); /** * Returns the namespace object of this module. @@ -337,6 +337,15 @@ class V8_EXPORT Module : public Data { * (where an exception was thrown). */ using SyntheticModuleEvaluationSteps = + MaybeLocal (*)(Local context, Local 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. + using LegacySyntheticModuleEvaluationSteps = MaybeLocal (*)(Local context, Local module); /** @@ -351,6 +360,16 @@ class V8_EXPORT Module : public Data { const MemorySpan>& 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") + static Local CreateSyntheticModule( + Isolate* isolate, Local module_name, + const MemorySpan>& 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 diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index fbd628370c0b..ce0017fb57be 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -2398,7 +2398,7 @@ Maybe Module::InstantiateModule(Local context, return Just(true); } -MaybeLocal Module::Evaluate(Local context) { +MaybeLocal Module::Evaluate(Local context) { auto i_isolate = i::Isolate::Current(); TRACE_EVENT_CALL_STATS_SCOPED(i_isolate, "v8", "V8.Execute"); EnterV8Scope api_scope{i_isolate, context, @@ -2436,6 +2436,33 @@ Local Module::CreateSyntheticModule( i_module_name, i_export_names, evaluation_steps))); } +START_ALLOW_USE_DEPRECATED() +Local Module::CreateSyntheticModule( + Isolate* v8_isolate, Local module_name, + const MemorySpan>& 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::Local, + // v8::Local)' to 'v8::MaybeLocal + // (*)(v8::Local, v8::Local)'. 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(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 Module::SetSyntheticModuleExport(Isolate* v8_isolate, Local export_name, Local export_value) { diff --git a/deps/v8/src/d8/d8.cc b/deps/v8/src/d8/d8.cc index 1c33ca83c369..67a43a3f653f 100644 --- a/deps/v8/src/d8/d8.cc +++ b/deps/v8/src/d8/d8.cc @@ -1441,7 +1441,7 @@ MaybeLocal Shell::FetchModuleTree(Local referrer, return result; } -MaybeLocal Shell::JSONModuleEvaluationSteps(Local context, +MaybeLocal Shell::JSONModuleEvaluationSteps(Local context, Local module) { Isolate* isolate = Isolate::GetCurrent(); diff --git a/deps/v8/src/d8/d8.h b/deps/v8/src/d8/d8.h index 8d57ddac8240..87c30015d211 100644 --- a/deps/v8/src/d8/d8.h +++ b/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 JSONModuleEvaluationSteps(Local context, - Local module); - + static MaybeLocal JSONModuleEvaluationSteps(Local context, + Local module); template static MaybeLocal CompileString(Isolate* isolate, Local context, Local source, diff --git a/deps/v8/src/objects/module.cc b/deps/v8/src/objects/module.cc index 147220347763..557decca5225 100644 --- a/deps/v8/src/objects/module.cc +++ b/deps/v8/src/objects/module.cc @@ -265,8 +265,8 @@ bool Module::FinishInstantiate(Isolate* isolate, Handle module, } } -MaybeDirectHandle Module::Evaluate(Isolate* isolate, - Handle module) { +MaybeDirectHandle Module::Evaluate(Isolate* isolate, + Handle module) { #ifdef DEBUG PrintStatusMessage(*module, "Evaluating module "); #endif // DEBUG @@ -492,16 +492,13 @@ void JSDeferredModuleNamespace::EvaluateModuleSync( return; } - MaybeDirectHandle maybe_result = Module::Evaluate(isolate, module); - DirectHandle result; - if (!maybe_result.ToHandle(&result)) { + MaybeDirectHandle maybe_result = Module::Evaluate(isolate, module); + DirectHandle promise; + if (!maybe_result.ToHandle(&promise)) { return; } - // If there's a result, it needs to be a promise with either Reject or - // Fulfilled status. - DCHECK(IsJSPromise(*result)); - DirectHandle promise = Cast(result); + // The result is always the module's top-level capability promise. // 5. If promise.[[PromiseState]] is rejected, then if (promise->status() == Promise::kRejected) { // a. If promise.[[PromiseIsHandled]] is false, perform diff --git a/deps/v8/src/objects/module.h b/deps/v8/src/objects/module.h index 30e9af6a1dc7..80d23a4e0542 100644 --- a/deps/v8/src/objects/module.h +++ b/deps/v8/src/objects/module.h @@ -74,7 +74,7 @@ class Module : public TorqueGeneratedModule { const UserResolveCallbacks& callbacks); // Implementation of spec operation ModuleEvaluation. - static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( + static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( Isolate* isolate, Handle module); // Get the namespace object for [module]. If it doesn't exist yet, it is diff --git a/deps/v8/src/objects/source-text-module.cc b/deps/v8/src/objects/source-text-module.cc index 101373b83c61..7c5807616355 100644 --- a/deps/v8/src/objects/source-text-module.cc +++ b/deps/v8/src/objects/source-text-module.cc @@ -886,8 +886,8 @@ bool SourceTextModule::MaybeHandleEvaluationException( return false; } -// ES#sec-moduleevaluation -MaybeDirectHandle SourceTextModule::Evaluate( +// https://tc39.es/ecma262/#sec-moduleevaluation +MaybeDirectHandle SourceTextModule::Evaluate( Isolate* isolate, Handle module) { CHECK(module->status() == kLinked || module->status() == kEvaluatingAsync || module->status() == kEvaluated); diff --git a/deps/v8/src/objects/source-text-module.h b/deps/v8/src/objects/source-text-module.h index 93d9f8f074b7..7be59e5b37a6 100644 --- a/deps/v8/src/objects/source-text-module.h +++ b/deps/v8/src/objects/source-text-module.h @@ -192,7 +192,7 @@ class SourceTextModule AvailableAncestorsSet* exec_list); // Implementation of spec concrete method Evaluate. - static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( + static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( Isolate* isolate, Handle module); // Implementation of spec abstract operation InnerModuleEvaluation. diff --git a/deps/v8/src/objects/synthetic-module.cc b/deps/v8/src/objects/synthetic-module.cc index fd791dfbe0c1..29d15516bfab 100644 --- a/deps/v8/src/objects/synthetic-module.cc +++ b/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,41 +106,39 @@ bool SyntheticModule::FinishInstantiate(Isolate* isolate, // Implements Synthetic Module Record's Evaluate concrete method: // https://heycam.github.io/webidl/#smr-evaluate -MaybeDirectHandle SyntheticModule::Evaluate( +// The callback may have been created through the deprecated +// v8::Module::LegacySyntheticModuleEvaluationSteps overload, in which case it +// actually returns a v8::MaybeLocal 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 SyntheticModule::Evaluate( Isolate* isolate, DirectHandle module) { module->SetStatus(kEvaluating); v8::Module::SyntheticModuleEvaluationSteps evaluation_steps = FUNCTION_CAST( module->evaluation_steps()->foreign_address()); + // Deliberately received as a v8::Local: the deprecated callback + // signature only promises a Promise, it doesn't guarantee one. v8::Local result; if (!evaluation_steps(Utils::ToLocal(isolate->native_context()), Utils::ToLocal(Cast(module))) .ToLocal(&result)) { module->RecordError(isolate, isolate->exception()); - return MaybeDirectHandle(); + return MaybeDirectHandle(); } module->SetStatus(kEvaluated); DirectHandle result_from_callback = Utils::OpenDirectHandle(*result); - - DirectHandle capability; - if (IsJSPromise(*result_from_callback)) { - capability = Cast(result_from_callback); - } else { - // The host's evaluation steps should have returned a resolved Promise, - // but as an allowance to hosts that have not yet finished the migration - // to top-level await, create a Promise if the callback result didn't give - // us one. - capability = isolate->factory()->NewJSPromise(); - JSPromise::Resolve(capability, isolate->factory()->undefined_value()) - .ToHandleChecked(); - } - + CHECK(IsJSPromise(*result_from_callback)); + DirectHandle capability = Cast(result_from_callback); module->set_top_level_capability(*capability); - - return result_from_callback; + return capability; } } // namespace internal diff --git a/deps/v8/src/objects/synthetic-module.h b/deps/v8/src/objects/synthetic-module.h index 03185dc16674..3ea24be032b0 100644 --- a/deps/v8/src/objects/synthetic-module.h +++ b/deps/v8/src/objects/synthetic-module.h @@ -60,7 +60,7 @@ class SyntheticModule static V8_WARN_UNUSED_RESULT bool FinishInstantiate( Isolate* isolate, DirectHandle module); - static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( + static V8_WARN_UNUSED_RESULT MaybeDirectHandle Evaluate( Isolate* isolate, DirectHandle module); TQ_OBJECT_CONSTRUCTORS(SyntheticModule) diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index ba7e079f411c..9d69054c9043 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -24701,33 +24701,52 @@ TEST(CodeCache) { isolate2->Dispose(); } -v8::MaybeLocal UnexpectedSyntheticModuleEvaluationStepsCallback( +v8::MaybeLocal UnexpectedSyntheticModuleEvaluationStepsCallback( Local context, Local module) { CHECK_WITH_MSG(false, "Unexpected call to synthetic module re callback"); } static int synthetic_module_callback_count; -v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( +v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( Local context, Local module) { synthetic_module_callback_count++; - return v8::Undefined(reinterpret_cast(CcTest::isolate())); + Local resolver = + v8::Promise::Resolver::New(context).ToLocalChecked(); + resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); + return resolver->GetPromise(); } -v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackFail( +v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackFail( Local context, Local module) { synthetic_module_callback_count++; CcTest::isolate()->ThrowException( v8_str("SyntheticModuleEvaluationStepsCallbackFail exception")); - return v8::MaybeLocal(); + return v8::MaybeLocal(); } -v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackSetExport( +// Deprecated version of the evaluation steps, returning a MaybeLocal +// that holds a Promise. +// TODO(https://crbug.com/545375591): Remove together with +// v8::Module::LegacySyntheticModuleEvaluationSteps. +v8::MaybeLocal LegacySyntheticModuleEvaluationStepsCallback( + Local context, Local module) { + synthetic_module_callback_count++; + Local resolver = + v8::Promise::Resolver::New(context).ToLocalChecked(); + resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); + return resolver->GetPromise(); +} + +v8::MaybeLocal SyntheticModuleEvaluationStepsCallbackSetExport( Local context, Local module) { Maybe set_export_result = module->SetSyntheticModuleExport( CcTest::isolate(), v8_str("test_export"), v8_num(42)); CHECK(set_export_result.FromJust()); - return v8::Undefined(reinterpret_cast(CcTest::isolate())); + Local resolver = + v8::Promise::Resolver::New(context).ToLocalChecked(); + resolver->Resolve(context, v8::Undefined(CcTest::isolate())).Check(); + return resolver->GetPromise(); } namespace { @@ -25058,7 +25077,44 @@ TEST(SyntheticModuleEvaluationStepsNoThrow) { context, export_names, SyntheticModuleEvaluationStepsCallback); CHECK_EQ(synthetic_module_callback_count, 0); Local completion_value = module->Evaluate(context).ToLocalChecked(); - CHECK(completion_value->IsUndefined()); + CHECK(completion_value->IsPromise()); + Local promise(Local::Cast(completion_value)); + CHECK_EQ(promise->State(), v8::Promise::kFulfilled); + CHECK(promise->Result()->IsUndefined()); + CHECK_EQ(synthetic_module_callback_count, 1); + 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 context = v8::Context::New(isolate); + v8::Context::Scope cscope(context); + + auto export_names = std::to_array>({v8_str("default")}); + + START_ALLOW_USE_DEPRECATED() + Local 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 completion_value = module->Evaluate(context).ToLocalChecked(); + CHECK(completion_value->IsPromise()); + Local promise(Local::Cast(completion_value)); + CHECK_EQ(promise->State(), v8::Promise::kFulfilled); CHECK_EQ(synthetic_module_callback_count, 1); CHECK_EQ(module->GetStatus(), Module::kEvaluated); } @@ -25116,7 +25172,10 @@ TEST(SyntheticModuleEvaluationStepsSetExport) { CHECK(IsUndefined(test_export_cell->value())); Local completion_value = module->Evaluate(context).ToLocalChecked(); - CHECK(completion_value->IsUndefined()); + CHECK(completion_value->IsPromise()); + Local promise(Local::Cast(completion_value)); + CHECK_EQ(promise->State(), v8::Promise::kFulfilled); + CHECK(promise->Result()->IsUndefined()); CHECK_EQ(42, i::Object::NumberValue(test_export_cell->value())); CHECK_EQ(module->GetStatus(), Module::kEvaluated); } @@ -27084,7 +27143,7 @@ MaybeLocal CheckResolveModuleWithImportSource( return v8::Module::CreateSyntheticModule( isolate, v8_str("my-mod"), {}, - [](Local context, Local module) -> MaybeLocal { + [](Local context, Local module) -> MaybeLocal { // Do nothing. Local resolver = v8::Promise::Resolver::New(context).ToLocalChecked(); diff --git a/deps/v8/test/unittests/objects/modules-unittest.cc b/deps/v8/test/unittests/objects/modules-unittest.cc index af910fd20f58..9777d8af353c 100644 --- a/deps/v8/test/unittests/objects/modules-unittest.cc +++ b/deps/v8/test/unittests/objects/modules-unittest.cc @@ -1686,7 +1686,7 @@ TEST_F(ModuleTest, SyntheticModuleGetResourceName) { Local resource_name = NewString("synthetic-module"); Local module = Module::CreateSyntheticModule( isolate(), resource_name, {}, - [](Local context, Local module) -> MaybeLocal { + [](Local context, Local module) -> MaybeLocal { // Do nothing. Local resolver = v8::Promise::Resolver::New(context).ToLocalChecked(); @@ -1716,12 +1716,12 @@ TEST_F(ModuleTest, SyntheticModuleGetResourceNameInError) { Local resource_name = NewString("synthetic-module"); Local module = Module::CreateSyntheticModule( isolate(), resource_name, {}, - [](Local context, Local module) -> MaybeLocal { + [](Local context, Local module) -> MaybeLocal { // Throw an error. Isolate* isolate = Isolate::GetCurrent(); isolate->ThrowException( v8::String::NewFromUtf8Literal(isolate, "synthetic module error")); - return MaybeLocal(); + return MaybeLocal(); }); CHECK_EQ(Module::kUninstantiated, module->GetStatus()); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 38ac15b337f3..8056f024b321 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -1442,7 +1442,7 @@ void ModuleWrap::SetInitializeImportMetaObjectCallback( HostInitializeImportMetaObjectCallback); } -MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( +MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( Local context, Local module) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); @@ -1466,16 +1466,16 @@ MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( CHECK(!try_catch.Message().IsEmpty()); CHECK(!try_catch.Exception().IsEmpty()); try_catch.ReThrow(); - return MaybeLocal(); + return MaybeLocal(); } Local resolver; if (!Promise::Resolver::New(context).ToLocal(&resolver)) { - return MaybeLocal(); + return MaybeLocal(); } if (resolver->Resolve(context, Undefined(isolate)).IsNothing()) { - return MaybeLocal(); + return MaybeLocal(); } return resolver->GetPromise(); } diff --git a/src/module_wrap.h b/src/module_wrap.h index 14a8f1a4f2d6..f40976772cb9 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -185,7 +185,7 @@ class ModuleWrap : public BaseObject { const v8::FunctionCallbackInfo& args); static void SetInitializeImportMetaObjectCallback( const v8::FunctionCallbackInfo& args); - static v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( + static v8::MaybeLocal SyntheticModuleEvaluationStepsCallback( v8::Local context, v8::Local module); static void SetSyntheticExport( const v8::FunctionCallbackInfo& args);