summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichaël Zasso <targos@protonmail.com>2019-10-28 11:12:22 +0100
committerMichaël Zasso <targos@protonmail.com>2019-11-08 15:50:51 +0100
commitd9fab1fdb76ae3a69b5812a7f2190cf3e58f6d75 (patch)
treec1c9b3dcb745a0c7bd9cc71802692816448b2180
parent53e925a5605675c0f534128c0a317f232229b0f3 (diff)
downloadandroid-node-v8-d9fab1fdb76ae3a69b5812a7f2190cf3e58f6d75.tar.gz
android-node-v8-d9fab1fdb76ae3a69b5812a7f2190cf3e58f6d75.tar.bz2
android-node-v8-d9fab1fdb76ae3a69b5812a7f2190cf3e58f6d75.zip
deps: V8: cherry-pick 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: https://github.com/v8/v8/commit/777fa98cc47ac32f0fde3d9aafd830949deb5d23 PR-URL: https://github.com/nodejs/node/pull/30020 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
-rw-r--r--common.gypi2
-rw-r--r--deps/v8/include/v8.h12
-rw-r--r--deps/v8/src/api/api.cc28
-rw-r--r--deps/v8/src/logging/counters.h1
-rw-r--r--deps/v8/src/objects/synthetic-module.cc30
-rw-r--r--deps/v8/src/objects/synthetic-module.h18
-rw-r--r--deps/v8/test/cctest/test-api.cc36
7 files changed, 111 insertions, 16 deletions
diff --git a/common.gypi b/common.gypi
index 78a57d9291..cc9107616a 100644
--- a/common.gypi
+++ b/common.gypi
@@ -39,7 +39,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.14',
+ 'v8_embedder_string': '-node.15',
##### V8 defaults for Node.js #####
diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h
index acb9c089d6..0b7f331c8c 100644
--- a/deps/v8/include/v8.h
+++ b/deps/v8/include/v8.h
@@ -1560,9 +1560,17 @@ class V8_EXPORT Module : public Data {
/**
* 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
- * CreateSyntheticModule. export_name must be one of the export_names that
- * were passed in that CreateSyntheticModule call.
+ * CreateSyntheticModule. An error will be thrown if export_name is not one
+ * of the export_names that were passed in that CreateSyntheticModule call.
+ * Returns Just(true) on success, Nothing<bool>() if an error was thrown.
*/
+ V8_WARN_UNUSED_RESULT Maybe<bool> SetSyntheticModuleExport(
+ Isolate* isolate, Local<String> export_name, Local<Value> export_value);
+ V8_DEPRECATE_SOON(
+ "Use the preceding SetSyntheticModuleExport with an Isolate parameter, "
+ "instead of the one that follows. The former will throw a runtime "
+ "error if called for an export that doesn't exist (as per spec); "
+ "the latter will crash with a failed CHECK().")
void SetSyntheticModuleExport(Local<String> export_name,
Local<Value> export_value);
};
diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc
index cb503015c1..fffee36c5a 100644
--- a/deps/v8/src/api/api.cc
+++ b/deps/v8/src/api/api.cc
@@ -2362,6 +2362,28 @@ Local<Module> Module::CreateSyntheticModule(
i_module_name, i_export_names, evaluation_steps)));
}
+Maybe<bool> Module::SetSyntheticModuleExport(Isolate* isolate,
+ Local<String> export_name,
+ Local<v8::Value> export_value) {
+ auto i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+ i::Handle<i::String> i_export_name = Utils::OpenHandle(*export_name);
+ i::Handle<i::Object> i_export_value = Utils::OpenHandle(*export_value);
+ i::Handle<i::Module> self = Utils::OpenHandle(this);
+ Utils::ApiCheck(self->IsSyntheticModule(),
+ "v8::Module::SyntheticModuleSetExport",
+ "v8::Module::SyntheticModuleSetExport must only be called on "
+ "a SyntheticModule");
+ ENTER_V8_NO_SCRIPT(i_isolate, isolate->GetCurrentContext(), Module,
+ SetSyntheticModuleExport, Nothing<bool>(), i::HandleScope);
+ has_pending_exception =
+ i::SyntheticModule::SetExport(i_isolate,
+ i::Handle<i::SyntheticModule>::cast(self),
+ i_export_name, i_export_value)
+ .IsNothing();
+ RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
+ return Just(true);
+}
+
void Module::SetSyntheticModuleExport(Local<String> export_name,
Local<v8::Value> export_value) {
i::Handle<i::String> i_export_name = Utils::OpenHandle(*export_name);
@@ -2371,9 +2393,9 @@ void Module::SetSyntheticModuleExport(Local<String> export_name,
"v8::Module::SetSyntheticModuleExport",
"v8::Module::SetSyntheticModuleExport must only be called on "
"a SyntheticModule");
- i::SyntheticModule::SetExport(self->GetIsolate(),
- i::Handle<i::SyntheticModule>::cast(self),
- i_export_name, i_export_value);
+ i::SyntheticModule::SetExportStrict(self->GetIsolate(),
+ i::Handle<i::SyntheticModule>::cast(self),
+ i_export_name, i_export_value);
}
namespace {
diff --git a/deps/v8/src/logging/counters.h b/deps/v8/src/logging/counters.h
index 5ba1d4626e..d82515efba 100644
--- a/deps/v8/src/logging/counters.h
+++ b/deps/v8/src/logging/counters.h
@@ -783,6 +783,7 @@ class RuntimeCallTimer final {
V(Message_GetStartColumn) \
V(Module_Evaluate) \
V(Module_InstantiateModule) \
+ V(Module_SetSyntheticModuleExport) \
V(NumberObject_New) \
V(NumberObject_NumberValue) \
V(Object_CallAsConstructor) \
diff --git a/deps/v8/src/objects/synthetic-module.cc b/deps/v8/src/objects/synthetic-module.cc
index 58e0c1b58c..75e79e8955 100644
--- a/deps/v8/src/objects/synthetic-module.cc
+++ b/deps/v8/src/objects/synthetic-module.cc
@@ -17,16 +17,36 @@ namespace internal {
// Implements SetSyntheticModuleBinding:
// https://heycam.github.io/webidl/#setsyntheticmoduleexport
-void SyntheticModule::SetExport(Isolate* isolate,
- Handle<SyntheticModule> module,
- Handle<String> export_name,
- Handle<Object> export_value) {
+Maybe<bool> SyntheticModule::SetExport(Isolate* isolate,
+ Handle<SyntheticModule> module,
+ Handle<String> export_name,
+ Handle<Object> export_value) {
Handle<ObjectHashTable> exports(module->exports(), isolate);
Handle<Object> export_object(exports->Lookup(export_name), isolate);
- CHECK(export_object->IsCell());
+
+ if (!export_object->IsCell()) {
+ isolate->Throw(*isolate->factory()->NewReferenceError(
+ MessageTemplate::kModuleExportUndefined, export_name));
+ return Nothing<bool>();
+ }
+
Handle<Cell> export_cell(Handle<Cell>::cast(export_object));
// Spec step 2: Set the mutable binding of export_name to export_value
export_cell->set_value(*export_value);
+
+ return Just(true);
+}
+
+void SyntheticModule::SetExportStrict(Isolate* isolate,
+ Handle<SyntheticModule> module,
+ Handle<String> export_name,
+ Handle<Object> export_value) {
+ Handle<ObjectHashTable> exports(module->exports(), isolate);
+ Handle<Object> export_object(exports->Lookup(export_name), isolate);
+ CHECK(export_object->IsCell());
+ Maybe<bool> set_export_result =
+ SetExport(isolate, module, export_name, export_value);
+ CHECK(set_export_result.FromJust());
}
// Implements Synthetic Module Record's ResolveExport concrete method:
diff --git a/deps/v8/src/objects/synthetic-module.h b/deps/v8/src/objects/synthetic-module.h
index 6f3bb0438e..77a6eed276 100644
--- a/deps/v8/src/objects/synthetic-module.h
+++ b/deps/v8/src/objects/synthetic-module.h
@@ -24,9 +24,21 @@ class SyntheticModule
DECL_VERIFIER(SyntheticModule)
DECL_PRINTER(SyntheticModule)
- static void SetExport(Isolate* isolate, Handle<SyntheticModule> module,
- Handle<String> export_name,
- Handle<Object> export_value);
+ // Set module's exported value for the specified export_name to the specified
+ // export_value. An error will be thrown if export_name is not one
+ // of the export_names that were supplied during module construction.
+ // Returns Just(true) on success, Nothing<bool>() if an error was thrown.
+ static Maybe<bool> SetExport(Isolate* isolate, Handle<SyntheticModule> module,
+ Handle<String> export_name,
+ Handle<Object> export_value);
+ // The following redundant method should be deleted when the deprecated
+ // version of v8::SetSyntheticModuleExport is removed. It differs from
+ // SetExport in that it crashes rather than throwing an error if the caller
+ // attempts to set an export_name that was not present during construction of
+ // the module.
+ static void SetExportStrict(Isolate* isolate, Handle<SyntheticModule> module,
+ Handle<String> export_name,
+ Handle<Object> export_value);
using BodyDescriptor = SubclassBodyDescriptor<
Module::BodyDescriptor,
diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc
index 1daa19402e..7da247e3ab 100644
--- a/deps/v8/test/cctest/test-api.cc
+++ b/deps/v8/test/cctest/test-api.cc
@@ -23721,7 +23721,9 @@ v8::MaybeLocal<Value> SyntheticModuleEvaluationStepsCallbackFail(
v8::MaybeLocal<Value> SyntheticModuleEvaluationStepsCallbackSetExport(
Local<Context> context, Local<Module> module) {
- module->SetSyntheticModuleExport(v8_str("test_export"), v8_num(42));
+ Maybe<bool> set_export_result = module->SetSyntheticModuleExport(
+ context->GetIsolate(), v8_str("test_export"), v8_num(42));
+ CHECK(set_export_result.FromJust());
return v8::Undefined(reinterpret_cast<v8::Isolate*>(context->GetIsolate()));
}
@@ -23940,7 +23942,9 @@ TEST(SyntheticModuleSetExports) {
// undefined.
CHECK(foo_cell->value().IsUndefined());
- module->SetSyntheticModuleExport(foo_string, bar_string);
+ Maybe<bool> set_export_result =
+ module->SetSyntheticModuleExport(isolate, foo_string, bar_string);
+ CHECK(set_export_result.FromJust());
// After setting the export the Cell should still have the same idenitity.
CHECK_EQ(exports->Lookup(v8::Utils::OpenHandle(*foo_string)), *foo_cell);
@@ -23951,6 +23955,34 @@ TEST(SyntheticModuleSetExports) {
->Equals(*v8::Utils::OpenHandle(*bar_string)));
}
+TEST(SyntheticModuleSetMissingExport) {
+ LocalContext env;
+ v8::Isolate* isolate = env->GetIsolate();
+ auto i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+ v8::Isolate::Scope iscope(isolate);
+ v8::HandleScope scope(isolate);
+ v8::Local<v8::Context> context = v8::Context::New(isolate);
+ v8::Context::Scope cscope(context);
+
+ Local<String> foo_string = v8_str("foo");
+ Local<String> bar_string = v8_str("bar");
+
+ Local<Module> module = CreateAndInstantiateSyntheticModule(
+ isolate, v8_str("SyntheticModuleSetExports-TestSyntheticModule"), context,
+ std::vector<v8::Local<v8::String>>(),
+ UnexpectedSyntheticModuleEvaluationStepsCallback);
+
+ i::Handle<i::SyntheticModule> i_module =
+ i::Handle<i::SyntheticModule>::cast(v8::Utils::OpenHandle(*module));
+ i::Handle<i::ObjectHashTable> exports(i_module->exports(), i_isolate);
+
+ TryCatch try_catch(isolate);
+ Maybe<bool> set_export_result =
+ module->SetSyntheticModuleExport(isolate, foo_string, bar_string);
+ CHECK(set_export_result.IsNothing());
+ CHECK(try_catch.HasCaught());
+}
+
TEST(SyntheticModuleEvaluationStepsNoThrow) {
synthetic_module_callback_count = 0;
LocalContext env;