Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 42c4560

Browse filesBrowse files
targostargos
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 <adamk@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Commit-Queue: Dan Clark <daniec@microsoft.com> Cr-Commit-Position: refs/heads/master@{#64438} Refs: v8/v8@777fa98 PR-URL: #30062 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent dbb4e2a commit 42c4560
Copy full SHA for 42c4560

File tree

Expand file treeCollapse file tree

7 files changed

+114
-19
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+114
-19
lines changed
Open diff view settings
Collapse file

‎common.gypi‎

Copy file name to clipboardExpand all lines: common.gypi
+1-1Lines 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

Collapse file

‎deps/v8/include/v8.h‎

Copy file name to clipboardExpand all lines: deps/v8/include/v8.h
+13-5Lines 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
/**
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/api/api.cc
+25-3Lines 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 {
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/logging/counters.h
+1Lines 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) \
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/objects/synthetic-module.cc
+25-5Lines 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:
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/objects/synthetic-module.h
+15-3Lines 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,
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-api.cc
+34-2Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.