From f970087147a30e62cdd1ebe8396d745195b8b473 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 11 Sep 2023 13:29:02 +0200 Subject: [PATCH] deps: V8: backport 93b1a74cbc9b Original commit message: Reland "[api] allow v8::Data as internal field" This is a reland of commit 0aa622e12893e9921c01a34ce9507b544e599c4a The original patch tried to run a test that calls exit() in the fatal error handler in parallel, which would not work. This marked the test with TEST() to avoid running it in parallel. Original change's description: > [api] allow v8::Data as internal field > > Previously only v8::Value can be stored as internal fields. > In some cases, however, it's necessary for the embedder to > tie the lifetime of a v8::Data with the lifetime of a > JS object, and that v8::Data may not be a v8::Value, as > it can be something returned from the V8 API. One way to > keep the v8::Data alive may be to use a v8::Persistent > but that can easily lead to leaks. > > This patch changes v8::Object::GetInternalField() and > v8::Object::SetInernalField() to accept v8::Data instead of just > v8::Value, so that v8::Data can kept alive by a JS object in > a way that the GC can be aware of to address this problem. > This is a breaking change for embedders > using v8::Object::GetInternalField() as it changes the return > type. Since most v8::Value subtypes only support direct casts > from v8::Value but not v8::Data, calls like > > object->GetInternalField(index).As() > > needs to be updated to cast the value to v8::Value first: > > object->GetInternalField(index).As().As() > > Bug: v8:14120 > Change-Id: I731c958d1756b9d5ee4a3e78813416cd60d1b7ca > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4707972 > Reviewed-by: Michael Lippautz > Commit-Queue: Joyee Cheung > Cr-Commit-Position: refs/heads/main@{#89718} Bug: v8:14120 Change-Id: I3e45d09b5c300d5eefc73e380ef21ac2bd61760c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4834471 Commit-Queue: Joyee Cheung Reviewed-by: Camillo Bruni Cr-Commit-Position: refs/heads/main@{#89824} Refs: https://github.com/v8/v8/commit/93b1a74cbc9b5382a145051b9475433bc6107f1e PR-URL: https://github.com/nodejs/node/pull/49419 Refs: https://github.com/v8/v8/commit/0aa622e12893e9921c01a34ce9507b544e599c4a Reviewed-By: Jiawen Geng Reviewed-By: Yagiz Nizipli --- common.gypi | 2 +- deps/v8/include/v8-object.h | 25 +++++--- deps/v8/samples/process.cc | 4 +- deps/v8/src/api/api.cc | 6 +- deps/v8/test/cctest/test-api.cc | 64 ++++++++++++++++--- deps/v8/test/cctest/test-api.h | 8 ++- deps/v8/test/cctest/test-serialize.cc | 20 +++--- .../objects/value-serializer-unittest.cc | 10 ++- 8 files changed, 103 insertions(+), 36 deletions(-) diff --git a/common.gypi b/common.gypi index c6d968c5e7447d..d783c7f970237a 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,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.15', + 'v8_embedder_string': '-node.16', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-object.h b/deps/v8/include/v8-object.h index d805dbe9e7d818..b34253d7ac5a8e 100644 --- a/deps/v8/include/v8-object.h +++ b/deps/v8/include/v8-object.h @@ -474,11 +474,20 @@ class V8_EXPORT Object : public Value { return object->InternalFieldCount(); } - /** Gets the value from an internal field. */ - V8_INLINE Local GetInternalField(int index); + /** + * Gets the data from an internal field. + * To cast the return value into v8::Value subtypes, it needs to be + * casted to a v8::Value first. For example, to cast it into v8::External: + * + * object->GetInternalField(index).As().As(); + * + * The embedder should make sure that the internal field being retrieved + * using this method has already been set with SetInternalField() before. + **/ + V8_INLINE Local GetInternalField(int index); - /** Sets the value in an internal field. */ - void SetInternalField(int index, Local value); + /** Sets the data in an internal field. */ + void SetInternalField(int index, Local data); /** * Gets a 2-byte-aligned native pointer from an internal field. This field @@ -710,13 +719,13 @@ class V8_EXPORT Object : public Value { private: Object(); static void CheckCast(Value* obj); - Local SlowGetInternalField(int index); + Local SlowGetInternalField(int index); void* SlowGetAlignedPointerFromInternalField(int index); }; // --- Implementation --- -Local Object::GetInternalField(int index) { +Local Object::GetInternalField(int index) { #ifndef V8_ENABLE_CHECKS using A = internal::Address; using I = internal::Internals; @@ -734,12 +743,12 @@ Local Object::GetInternalField(int index) { #endif #ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING - return Local(reinterpret_cast(value)); + return Local(reinterpret_cast(value)); #else internal::Isolate* isolate = internal::IsolateFromNeverReadOnlySpaceObject(obj); A* result = HandleScope::CreateHandle(isolate, value); - return Local(reinterpret_cast(result)); + return Local(reinterpret_cast(result)); #endif } #endif diff --git a/deps/v8/samples/process.cc b/deps/v8/samples/process.cc index 28b6f119c3acd2..32e87b854be803 100644 --- a/deps/v8/samples/process.cc +++ b/deps/v8/samples/process.cc @@ -388,7 +388,7 @@ Local JsHttpRequestProcessor::WrapMap(map* obj) { // Utility function that extracts the C++ map pointer from a wrapper // object. map* JsHttpRequestProcessor::UnwrapMap(Local obj) { - Local field = obj->GetInternalField(0).As(); + Local field = obj->GetInternalField(0).As().As(); void* ptr = field->Value(); return static_cast*>(ptr); } @@ -504,7 +504,7 @@ Local JsHttpRequestProcessor::WrapRequest(HttpRequest* request) { * wrapper object. */ HttpRequest* JsHttpRequestProcessor::UnwrapRequest(Local obj) { - Local field = obj->GetInternalField(0).As(); + Local field = obj->GetInternalField(0).As().As(); void* ptr = field->Value(); return static_cast(ptr); } diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index 10e342d496a703..7071b221a7afe0 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -6342,16 +6342,16 @@ static bool InternalFieldOK(i::Handle obj, int index, location, "Internal field out of bounds"); } -Local v8::Object::SlowGetInternalField(int index) { +Local v8::Object::SlowGetInternalField(int index) { i::Handle obj = Utils::OpenHandle(this); const char* location = "v8::Object::GetInternalField()"; if (!InternalFieldOK(obj, index, location)) return Local(); i::Handle value(i::JSObject::cast(*obj).GetEmbedderField(index), obj->GetIsolate()); - return Utils::ToLocal(value); + return ToApiHandle(value); } -void v8::Object::SetInternalField(int index, v8::Local value) { +void v8::Object::SetInternalField(int index, v8::Local value) { i::Handle obj = Utils::OpenHandle(this); const char* location = "v8::Object::SetInternalField()"; if (!InternalFieldOK(obj, index, location)) return; diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index af7dfbf03ce2d8..230e57c4795a78 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -2874,6 +2874,43 @@ THREADED_TEST(FunctionPrototype) { CHECK_EQ(v8_run_int32value(script), 321); } +bool internal_field_check_called = false; +void OnInternalFieldCheck(const char* location, const char* message) { + internal_field_check_called = true; + exit(strcmp(location, "v8::Value::Cast") + + strcmp(message, "Data is not a Value")); +} + +THREADED_TEST(InternalDataFields) { + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope scope(isolate); + + Local templ = v8::FunctionTemplate::New(isolate); + Local instance_templ = templ->InstanceTemplate(); + instance_templ->SetInternalFieldCount(1); + Local obj = templ->GetFunction(env.local()) + .ToLocalChecked() + ->NewInstance(env.local()) + .ToLocalChecked(); + CHECK_EQ(1, obj->InternalFieldCount()); + Local data = obj->GetInternalField(0); + CHECK(data->IsValue() && data.As()->IsUndefined()); + Local sym = v8::Private::New(isolate, v8_str("Foo")); + obj->SetInternalField(0, sym); + Local field = obj->GetInternalField(0); + CHECK(!field->IsValue()); + CHECK(field->IsPrivate()); + CHECK_EQ(sym, field); + +#ifdef V8_ENABLE_CHECKS + isolate->SetFatalErrorHandler(OnInternalFieldCheck); + USE(obj->GetInternalField(0).As()); + // If it's never called this would fail. + CHECK(internal_field_check_called); +#endif +} + THREADED_TEST(InternalFields) { LocalContext env; v8::Isolate* isolate = env->GetIsolate(); @@ -2887,9 +2924,12 @@ THREADED_TEST(InternalFields) { ->NewInstance(env.local()) .ToLocalChecked(); CHECK_EQ(1, obj->InternalFieldCount()); - CHECK(obj->GetInternalField(0)->IsUndefined()); + CHECK(obj->GetInternalField(0).As()->IsUndefined()); obj->SetInternalField(0, v8_num(17)); - CHECK_EQ(17, obj->GetInternalField(0)->Int32Value(env.local()).FromJust()); + CHECK_EQ(17, obj->GetInternalField(0) + .As() + ->Int32Value(env.local()) + .FromJust()); } TEST(InternalFieldsSubclassing) { @@ -2915,14 +2955,16 @@ TEST(InternalFieldsSubclassing) { CHECK_EQ(0, i_obj->map().GetInObjectProperties()); // Check writing and reading internal fields. for (int j = 0; j < nof_embedder_fields; j++) { - CHECK(obj->GetInternalField(j)->IsUndefined()); + CHECK(obj->GetInternalField(j).As()->IsUndefined()); int value = 17 + j; obj->SetInternalField(j, v8_num(value)); } for (int j = 0; j < nof_embedder_fields; j++) { int value = 17 + j; - CHECK_EQ(value, - obj->GetInternalField(j)->Int32Value(env.local()).FromJust()); + CHECK_EQ(value, obj->GetInternalField(j) + .As() + ->Int32Value(env.local()) + .FromJust()); } CHECK(env->Global() ->Set(env.local(), v8_str("BaseClass"), constructor) @@ -3022,9 +3064,12 @@ THREADED_TEST(GlobalObjectInternalFields) { v8::Local global_proxy = env->Global(); v8::Local global = global_proxy->GetPrototype().As(); CHECK_EQ(1, global->InternalFieldCount()); - CHECK(global->GetInternalField(0)->IsUndefined()); + CHECK(global->GetInternalField(0).As()->IsUndefined()); global->SetInternalField(0, v8_num(17)); - CHECK_EQ(17, global->GetInternalField(0)->Int32Value(env.local()).FromJust()); + CHECK_EQ(17, global->GetInternalField(0) + .As() + ->Int32Value(env.local()) + .FromJust()); } @@ -7766,7 +7811,7 @@ void InternalFieldCallback(bool global_gc) { .ToLocalChecked(); handle.Reset(isolate, obj); CHECK_EQ(2, obj->InternalFieldCount()); - CHECK(obj->GetInternalField(0)->IsUndefined()); + CHECK(obj->GetInternalField(0).As()->IsUndefined()); t1 = new Trivial(42); t2 = new Trivial2(103, 9); @@ -29858,7 +29903,8 @@ class HiddenDataDelegate : public v8::Context::DeepFreezeDelegate { std::vector>& children_out) override { int fields = obj->InternalFieldCount(); for (int idx = 0; idx < fields; idx++) { - v8::Local child_value = obj->GetInternalField(idx); + v8::Local child_value = + obj->GetInternalField(idx).As(); if (child_value->IsExternal()) { if (!FreezeExternal(v8::Local::Cast(child_value), children_out)) { diff --git a/deps/v8/test/cctest/test-api.h b/deps/v8/test/cctest/test-api.h index 0604deb9af8810..ba443198dbce2d 100644 --- a/deps/v8/test/cctest/test-api.h +++ b/deps/v8/test/cctest/test-api.h @@ -44,9 +44,11 @@ template static void CheckInternalFieldsAreZero(v8::Local value) { CHECK_EQ(T::kInternalFieldCount, value->InternalFieldCount()); for (int i = 0; i < value->InternalFieldCount(); i++) { - CHECK_EQ(0, value->GetInternalField(i) - ->Int32Value(CcTest::isolate()->GetCurrentContext()) - .FromJust()); + v8::Local field = + value->GetInternalField(i).template As(); + CHECK_EQ( + 0, + field->Int32Value(CcTest::isolate()->GetCurrentContext()).FromJust()); } } diff --git a/deps/v8/test/cctest/test-serialize.cc b/deps/v8/test/cctest/test-serialize.cc index ee9cb5493bee07..e5135a5b04d390 100644 --- a/deps/v8/test/cctest/test-serialize.cc +++ b/deps/v8/test/cctest/test-serialize.cc @@ -3577,23 +3577,27 @@ UNINITIALIZED_TEST(SnapshotCreatorTemplates) { .ToLocalChecked() ->ToObject(context) .ToLocalChecked(); - v8::Local b = - a->GetInternalField(0)->ToObject(context).ToLocalChecked(); - v8::Local c = - b->GetInternalField(0)->ToObject(context).ToLocalChecked(); + v8::Local b = a->GetInternalField(0) + .As() + ->ToObject(context) + .ToLocalChecked(); + v8::Local c = b->GetInternalField(0) + .As() + ->ToObject(context) + .ToLocalChecked(); InternalFieldData* a1 = reinterpret_cast( a->GetAlignedPointerFromInternalField(1)); - v8::Local a2 = a->GetInternalField(2); + v8::Local a2 = a->GetInternalField(2).As(); InternalFieldData* b1 = reinterpret_cast( b->GetAlignedPointerFromInternalField(1)); - v8::Local b2 = b->GetInternalField(2); + v8::Local b2 = b->GetInternalField(2).As(); - v8::Local c0 = c->GetInternalField(0); + v8::Local c0 = c->GetInternalField(0).As(); InternalFieldData* c1 = reinterpret_cast( c->GetAlignedPointerFromInternalField(1)); - v8::Local c2 = c->GetInternalField(2); + v8::Local c2 = c->GetInternalField(2).As(); CHECK(c0->IsUndefined()); diff --git a/deps/v8/test/unittests/objects/value-serializer-unittest.cc b/deps/v8/test/unittests/objects/value-serializer-unittest.cc index bf81b745dea261..8298dc9decd888 100644 --- a/deps/v8/test/unittests/objects/value-serializer-unittest.cc +++ b/deps/v8/test/unittests/objects/value-serializer-unittest.cc @@ -61,12 +61,14 @@ class ValueSerializerTest : public TestWithIsolate { function_template->InstanceTemplate()->SetAccessor( StringFromUtf8("value"), [](Local property, const PropertyCallbackInfo& args) { - args.GetReturnValue().Set(args.Holder()->GetInternalField(0)); + args.GetReturnValue().Set( + args.Holder()->GetInternalField(0).As()); }); function_template->InstanceTemplate()->SetAccessor( StringFromUtf8("value2"), [](Local property, const PropertyCallbackInfo& args) { - args.GetReturnValue().Set(args.Holder()->GetInternalField(1)); + args.GetReturnValue().Set( + args.Holder()->GetInternalField(1).As()); }); for (Local context : {serialization_context_, deserialization_context_}) { @@ -2897,6 +2899,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint32) { .WillRepeatedly(Invoke([this](Isolate*, Local object) { uint32_t value = 0; EXPECT_TRUE(object->GetInternalField(0) + .As() ->Uint32Value(serialization_context()) .To(&value)); WriteExampleHostObjectTag(); @@ -2928,9 +2931,11 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint64) { .WillRepeatedly(Invoke([this](Isolate*, Local object) { uint32_t value = 0, value2 = 0; EXPECT_TRUE(object->GetInternalField(0) + .As() ->Uint32Value(serialization_context()) .To(&value)); EXPECT_TRUE(object->GetInternalField(1) + .As() ->Uint32Value(serialization_context()) .To(&value2)); WriteExampleHostObjectTag(); @@ -2968,6 +2973,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripDouble) { .WillRepeatedly(Invoke([this](Isolate*, Local object) { double value = 0; EXPECT_TRUE(object->GetInternalField(0) + .As() ->NumberValue(serialization_context()) .To(&value)); WriteExampleHostObjectTag();