Skip to content

Commit c4074e3

Browse files
targosBethGriggs
targos
authored and
BethGriggs
committed
deps: V8: backport 777fa98
Original commit message: Make SetSyntheticModuleExport throw instead of crash for nonexistent export name Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError when called with an export name that was not supplied when constructing that SyntheticModule. Instead, the current implementation crashes with a failed CHECK(). Add a new Module::SyntheticModuleSetExport that throws (without an ensuing crash) for this case, and deprecate the old Module::SetSyntheticModuleExport. Bug: v8:9828 Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996 Reviewed-by: Adam Klein <[email protected]> Reviewed-by: Georg Neis <[email protected]> Commit-Queue: Dan Clark <[email protected]> Cr-Commit-Position: refs/heads/master@{#64438} Refs: v8/v8@777fa98 PR-URL: #30062 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent b09eda5 commit c4074e3

7 files changed

Lines changed: 114 additions & 19 deletions

File tree

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838

3939
# Reset this number to 0 on major V8 upgrades.
4040
# Increment by one for each non-official patch applied to deps/v8.
41-
'v8_embedder_string': '-node.22',
41+
'v8_embedder_string': '-node.23',
4242

4343
##### V8 defaults for Node.js #####
4444

‎deps/v8/include/v8.h‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,11 +1473,19 @@ class V8_EXPORT Module {
14731473
/**
14741474
* Set this module's exported value for the name export_name to the specified
14751475
* export_value. This method must be called only on Modules created via
1476-
* CreateSyntheticModule. export_name must be one of the export_names that
1477-
* were passed in that CreateSyntheticModule call.
1478-
*/
1479-
void SetSyntheticModuleExport(Local<String> export_name,
1480-
Local<Value> export_value);
1476+
* CreateSyntheticModule. An error will be thrown if export_name is not one
1477+
* of the export_names that were passed in that CreateSyntheticModule call.
1478+
* Returns Just(true) on success, Nothing<bool>() if an error was thrown.
1479+
*/
1480+
V8_WARN_UNUSED_RESULT Maybe<bool> SetSyntheticModuleExport(
1481+
Isolate* isolate, Local<String> export_name, Local<Value> export_value);
1482+
V8_DEPRECATE_SOON(
1483+
"Use the preceding SetSyntheticModuleExport with an Isolate parameter, "
1484+
"instead of the one that follows. The former will throw a runtime "
1485+
"error if called for an export that doesn't exist (as per spec); "
1486+
"the latter will crash with a failed CHECK().",
1487+
void SetSyntheticModuleExport(Local<String> export_name,
1488+
Local<Value> export_value));
14811489
};
14821490

14831491
/**

‎deps/v8/src/api/api.cc‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2351,6 +2351,28 @@ Local<Module> Module::CreateSyntheticModule(
23512351
i_module_name, i_export_names, evaluation_steps)));
23522352
}
23532353

2354+
Maybe<bool> Module::SetSyntheticModuleExport(Isolate* isolate,
2355+
Local<String> export_name,
2356+
Local<v8::Value> export_value) {
2357+
auto i_isolate = reinterpret_cast<i::Isolate*>(isolate);
2358+
i::Handle<i::String> i_export_name = Utils::OpenHandle(*export_name);
2359+
i::Handle<i::Object> i_export_value = Utils::OpenHandle(*export_value);
2360+
i::Handle<i::Module> self = Utils::OpenHandle(this);
2361+
Utils::ApiCheck(self->IsSyntheticModule(),
2362+
"v8::Module::SyntheticModuleSetExport",
2363+
"v8::Module::SyntheticModuleSetExport must only be called on "
2364+
"a SyntheticModule");
2365+
ENTER_V8_NO_SCRIPT(i_isolate, isolate->GetCurrentContext(), Module,
2366+
SetSyntheticModuleExport, Nothing<bool>(), i::HandleScope);
2367+
has_pending_exception =
2368+
i::SyntheticModule::SetExport(i_isolate,
2369+
i::Handle<i::SyntheticModule>::cast(self),
2370+
i_export_name, i_export_value)
2371+
.IsNothing();
2372+
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
2373+
return Just(true);
2374+
}
2375+
23542376
void Module::SetSyntheticModuleExport(Local<String> export_name,
23552377
Local<v8::Value> export_value) {
23562378
i::Handle<i::String> i_export_name = Utils::OpenHandle(*export_name);
@@ -2360,9 +2382,9 @@ void Module::SetSyntheticModuleExport(Local<String> export_name,
23602382
"v8::Module::SetSyntheticModuleExport",
23612383
"v8::Module::SetSyntheticModuleExport must only be called on "
23622384
"a SyntheticModule");
2363-
i::SyntheticModule::SetExport(self->GetIsolate(),
2364-
i::Handle<i::SyntheticModule>::cast(self),
2365-
i_export_name, i_export_value);
2385+
i::SyntheticModule::SetExportStrict(self->GetIsolate(),
2386+
i::Handle<i::SyntheticModule>::cast(self),
2387+
i_export_name, i_export_value);
23662388
}
23672389

23682390
namespace {

‎deps/v8/src/logging/counters.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,7 @@ class RuntimeCallTimer final {
780780
V(Message_GetStartColumn) \
781781
V(Module_Evaluate) \
782782
V(Module_InstantiateModule) \
783+
V(Module_SetSyntheticModuleExport) \
783784
V(NumberObject_New) \
784785
V(NumberObject_NumberValue) \
785786
V(Object_CallAsConstructor) \

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

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,36 @@ namespace internal {
1717

1818
// Implements SetSyntheticModuleBinding:
1919
// https://heycam.github.io/webidl/#setsyntheticmoduleexport
20-
void SyntheticModule::SetExport(Isolate* isolate,
21-
Handle<SyntheticModule> module,
22-
Handle<String> export_name,
23-
Handle<Object> export_value) {
20+
Maybe<bool> SyntheticModule::SetExport(Isolate* isolate,
21+
Handle<SyntheticModule> module,
22+
Handle<String> export_name,
23+
Handle<Object> export_value) {
2424
Handle<ObjectHashTable> exports(module->exports(), isolate);
2525
Handle<Object> export_object(exports->Lookup(export_name), isolate);
26-
CHECK(export_object->IsCell());
26+
27+
if (!export_object->IsCell()) {
28+
isolate->Throw(*isolate->factory()->NewReferenceError(
29+
MessageTemplate::kModuleExportUndefined, export_name));
30+
return Nothing<bool>();
31+
}
32+
2733
Handle<Cell> export_cell(Handle<Cell>::cast(export_object));
2834
// Spec step 2: Set the mutable binding of export_name to export_value
2935
export_cell->set_value(*export_value);
36+
37+
return Just(true);
38+
}
39+
40+
void SyntheticModule::SetExportStrict(Isolate* isolate,
41+
Handle<SyntheticModule> module,
42+
Handle<String> export_name,
43+
Handle<Object> export_value) {
44+
Handle<ObjectHashTable> exports(module->exports(), isolate);
45+
Handle<Object> export_object(exports->Lookup(export_name), isolate);
46+
CHECK(export_object->IsCell());
47+
Maybe<bool> set_export_result =
48+
SetExport(isolate, module, export_name, export_value);
49+
CHECK(set_export_result.FromJust());
3050
}
3151

3252
// Implements Synthetic Module Record's ResolveExport concrete method:

‎deps/v8/src/objects/synthetic-module.h‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,21 @@ class SyntheticModule
2424
DECL_VERIFIER(SyntheticModule)
2525
DECL_PRINTER(SyntheticModule)
2626

27-
static void SetExport(Isolate* isolate, Handle<SyntheticModule> module,
28-
Handle<String> export_name,
29-
Handle<Object> export_value);
27+
// Set module's exported value for the specified export_name to the specified
28+
// export_value. An error will be thrown if export_name is not one
29+
// of the export_names that were supplied during module construction.
30+
// Returns Just(true) on success, Nothing<bool>() if an error was thrown.
31+
static Maybe<bool> SetExport(Isolate* isolate, Handle<SyntheticModule> module,
32+
Handle<String> export_name,
33+
Handle<Object> export_value);
34+
// The following redundant method should be deleted when the deprecated
35+
// version of v8::SetSyntheticModuleExport is removed. It differs from
36+
// SetExport in that it crashes rather than throwing an error if the caller
37+
// attempts to set an export_name that was not present during construction of
38+
// the module.
39+
static void SetExportStrict(Isolate* isolate, Handle<SyntheticModule> module,
40+
Handle<String> export_name,
41+
Handle<Object> export_value);
3042

3143
using BodyDescriptor = SubclassBodyDescriptor<
3244
Module::BodyDescriptor,

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

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23693,7 +23693,9 @@ v8::MaybeLocal<Value> SyntheticModuleEvaluationStepsCallbackFail(
2369323693

2369423694
v8::MaybeLocal<Value> SyntheticModuleEvaluationStepsCallbackSetExport(
2369523695
Local<Context> context, Local<Module> module) {
23696-
module->SetSyntheticModuleExport(v8_str("test_export"), v8_num(42));
23696+
Maybe<bool> set_export_result = module->SetSyntheticModuleExport(
23697+
context->GetIsolate(), v8_str("test_export"), v8_num(42));
23698+
CHECK(set_export_result.FromJust());
2369723699
return v8::Undefined(reinterpret_cast<v8::Isolate*>(context->GetIsolate()));
2369823700
}
2369923701

@@ -23900,7 +23902,9 @@ TEST(SyntheticModuleSetExports) {
2390023902
// undefined.
2390123903
CHECK(foo_cell->value().IsUndefined());
2390223904

23903-
module->SetSyntheticModuleExport(foo_string, bar_string);
23905+
Maybe<bool> set_export_result =
23906+
module->SetSyntheticModuleExport(isolate, foo_string, bar_string);
23907+
CHECK(set_export_result.FromJust());
2390423908

2390523909
// After setting the export the Cell should still have the same idenitity.
2390623910
CHECK_EQ(exports->Lookup(v8::Utils::OpenHandle(*foo_string)), *foo_cell);
@@ -23911,6 +23915,34 @@ TEST(SyntheticModuleSetExports) {
2391123915
->Equals(*v8::Utils::OpenHandle(*bar_string)));
2391223916
}
2391323917

23918+
TEST(SyntheticModuleSetMissingExport) {
23919+
LocalContext env;
23920+
v8::Isolate* isolate = env->GetIsolate();
23921+
auto i_isolate = reinterpret_cast<i::Isolate*>(isolate);
23922+
v8::Isolate::Scope iscope(isolate);
23923+
v8::HandleScope scope(isolate);
23924+
v8::Local<v8::Context> context = v8::Context::New(isolate);
23925+
v8::Context::Scope cscope(context);
23926+
23927+
Local<String> foo_string = v8_str("foo");
23928+
Local<String> bar_string = v8_str("bar");
23929+
23930+
Local<Module> module = CreateAndInstantiateSyntheticModule(
23931+
isolate, v8_str("SyntheticModuleSetExports-TestSyntheticModule"), context,
23932+
std::vector<v8::Local<v8::String>>(),
23933+
UnexpectedSyntheticModuleEvaluationStepsCallback);
23934+
23935+
i::Handle<i::SyntheticModule> i_module =
23936+
i::Handle<i::SyntheticModule>::cast(v8::Utils::OpenHandle(*module));
23937+
i::Handle<i::ObjectHashTable> exports(i_module->exports(), i_isolate);
23938+
23939+
TryCatch try_catch(isolate);
23940+
Maybe<bool> set_export_result =
23941+
module->SetSyntheticModuleExport(isolate, foo_string, bar_string);
23942+
CHECK(set_export_result.IsNothing());
23943+
CHECK(try_catch.HasCaught());
23944+
}
23945+
2391423946
TEST(SyntheticModuleEvaluationStepsNoThrow) {
2391523947
synthetic_module_callback_count = 0;
2391623948
LocalContext env;

0 commit comments

Comments
 (0)