Skip to content

Commit

Permalink
deps: V8: backport 0aa622e12893
Browse files Browse the repository at this point in the history
Original commit message:

    [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<v8::Data>
    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<v8::External>()

    needs to be updated to cast the value to v8::Value first:

    object->GetInternalField(index).As<v8::Value>().As<v8::External>()

    Bug: v8:14120
    Change-Id: I731c958d1756b9d5ee4a3e78813416cd60d1b7ca
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4707972
    Reviewed-by: Michael Lippautz <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89718}

Refs: v8/v8@0aa622e
  • Loading branch information
joyeecheung committed Aug 31, 2023
1 parent 0409cdd commit ab06393
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 36 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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.14',
'v8_embedder_string': '-node.15',

##### V8 defaults for Node.js #####

Expand Down
25 changes: 17 additions & 8 deletions deps/v8/include/v8-object.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,20 @@ class V8_EXPORT Object : public Value {
return object->InternalFieldCount();
}

/** Gets the value from an internal field. */
V8_INLINE Local<Value> 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<v8::Value>().As<v8::External>();
*
* The embedder should make sure that the internal field being retrieved
* using this method has already been set with SetInternalField() before.
**/
V8_INLINE Local<Data> GetInternalField(int index);

/** Sets the value in an internal field. */
void SetInternalField(int index, Local<Value> value);
/** Sets the data in an internal field. */
void SetInternalField(int index, Local<Data> data);

/**
* Gets a 2-byte-aligned native pointer from an internal field. This field
Expand Down Expand Up @@ -710,13 +719,13 @@ class V8_EXPORT Object : public Value {
private:
Object();
static void CheckCast(Value* obj);
Local<Value> SlowGetInternalField(int index);
Local<Data> SlowGetInternalField(int index);
void* SlowGetAlignedPointerFromInternalField(int index);
};

// --- Implementation ---

Local<Value> Object::GetInternalField(int index) {
Local<Data> Object::GetInternalField(int index) {
#ifndef V8_ENABLE_CHECKS
using A = internal::Address;
using I = internal::Internals;
Expand All @@ -734,12 +743,12 @@ Local<Value> Object::GetInternalField(int index) {
#endif

#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
return Local<Value>(reinterpret_cast<Value*>(value));
return Local<Data>(reinterpret_cast<Data*>(value));
#else
internal::Isolate* isolate =
internal::IsolateFromNeverReadOnlySpaceObject(obj);
A* result = HandleScope::CreateHandle(isolate, value);
return Local<Value>(reinterpret_cast<Value*>(result));
return Local<Data>(reinterpret_cast<Data*>(result));
#endif
}
#endif
Expand Down
4 changes: 2 additions & 2 deletions deps/v8/samples/process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ Local<Object> JsHttpRequestProcessor::WrapMap(map<string, string>* obj) {
// Utility function that extracts the C++ map pointer from a wrapper
// object.
map<string, string>* JsHttpRequestProcessor::UnwrapMap(Local<Object> obj) {
Local<External> field = obj->GetInternalField(0).As<External>();
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
void* ptr = field->Value();
return static_cast<map<string, string>*>(ptr);
}
Expand Down Expand Up @@ -504,7 +504,7 @@ Local<Object> JsHttpRequestProcessor::WrapRequest(HttpRequest* request) {
* wrapper object.
*/
HttpRequest* JsHttpRequestProcessor::UnwrapRequest(Local<Object> obj) {
Local<External> field = obj->GetInternalField(0).As<External>();
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
void* ptr = field->Value();
return static_cast<HttpRequest*>(ptr);
}
Expand Down
6 changes: 3 additions & 3 deletions deps/v8/src/api/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6342,16 +6342,16 @@ static bool InternalFieldOK(i::Handle<i::JSReceiver> obj, int index,
location, "Internal field out of bounds");
}

Local<Value> v8::Object::SlowGetInternalField(int index) {
Local<Data> v8::Object::SlowGetInternalField(int index) {
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
const char* location = "v8::Object::GetInternalField()";
if (!InternalFieldOK(obj, index, location)) return Local<Value>();
i::Handle<i::Object> value(i::JSObject::cast(*obj).GetEmbedderField(index),
obj->GetIsolate());
return Utils::ToLocal(value);
return ToApiHandle<Data>(value);
}

void v8::Object::SetInternalField(int index, v8::Local<Value> value) {
void v8::Object::SetInternalField(int index, v8::Local<Data> value) {
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
const char* location = "v8::Object::SetInternalField()";
if (!InternalFieldOK(obj, index, location)) return;
Expand Down
64 changes: 55 additions & 9 deletions deps/v8/test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
Local<v8::ObjectTemplate> instance_templ = templ->InstanceTemplate();
instance_templ->SetInternalFieldCount(1);
Local<v8::Object> obj = templ->GetFunction(env.local())
.ToLocalChecked()
->NewInstance(env.local())
.ToLocalChecked();
CHECK_EQ(1, obj->InternalFieldCount());
Local<v8::Data> data = obj->GetInternalField(0);
CHECK(data->IsValue() && data.As<v8::Value>()->IsUndefined());
Local<v8::Private> sym = v8::Private::New(isolate, v8_str("Foo"));
obj->SetInternalField(0, sym);
Local<v8::Data> 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<v8::Value>());
// If it's never called this would fail.
CHECK(internal_field_check_called);
#endif
}

THREADED_TEST(InternalFields) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
Expand All @@ -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<v8::Value>()->IsUndefined());
obj->SetInternalField(0, v8_num(17));
CHECK_EQ(17, obj->GetInternalField(0)->Int32Value(env.local()).FromJust());
CHECK_EQ(17, obj->GetInternalField(0)
.As<v8::Value>()
->Int32Value(env.local())
.FromJust());
}

TEST(InternalFieldsSubclassing) {
Expand All @@ -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<v8::Value>()->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<v8::Value>()
->Int32Value(env.local())
.FromJust());
}
CHECK(env->Global()
->Set(env.local(), v8_str("BaseClass"), constructor)
Expand Down Expand Up @@ -3022,9 +3064,12 @@ THREADED_TEST(GlobalObjectInternalFields) {
v8::Local<v8::Object> global_proxy = env->Global();
v8::Local<v8::Object> global = global_proxy->GetPrototype().As<v8::Object>();
CHECK_EQ(1, global->InternalFieldCount());
CHECK(global->GetInternalField(0)->IsUndefined());
CHECK(global->GetInternalField(0).As<v8::Value>()->IsUndefined());
global->SetInternalField(0, v8_num(17));
CHECK_EQ(17, global->GetInternalField(0)->Int32Value(env.local()).FromJust());
CHECK_EQ(17, global->GetInternalField(0)
.As<v8::Value>()
->Int32Value(env.local())
.FromJust());
}


Expand Down Expand Up @@ -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<v8::Value>()->IsUndefined());
t1 = new Trivial(42);
t2 = new Trivial2(103, 9);

Expand Down Expand Up @@ -29858,7 +29903,8 @@ class HiddenDataDelegate : public v8::Context::DeepFreezeDelegate {
std::vector<v8::Local<v8::Object>>& children_out) override {
int fields = obj->InternalFieldCount();
for (int idx = 0; idx < fields; idx++) {
v8::Local<v8::Value> child_value = obj->GetInternalField(idx);
v8::Local<v8::Value> child_value =
obj->GetInternalField(idx).As<v8::Value>();
if (child_value->IsExternal()) {
if (!FreezeExternal(v8::Local<v8::External>::Cast(child_value),
children_out)) {
Expand Down
8 changes: 5 additions & 3 deletions deps/v8/test/cctest/test-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ template <typename T>
static void CheckInternalFieldsAreZero(v8::Local<T> 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<v8::Value> field =
value->GetInternalField(i).template As<v8::Value>();
CHECK_EQ(
0,
field->Int32Value(CcTest::isolate()->GetCurrentContext()).FromJust());
}
}

Expand Down
20 changes: 12 additions & 8 deletions deps/v8/test/cctest/test-serialize.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3577,23 +3577,27 @@ UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
.ToLocalChecked()
->ToObject(context)
.ToLocalChecked();
v8::Local<v8::Object> b =
a->GetInternalField(0)->ToObject(context).ToLocalChecked();
v8::Local<v8::Object> c =
b->GetInternalField(0)->ToObject(context).ToLocalChecked();
v8::Local<v8::Object> b = a->GetInternalField(0)
.As<v8::Value>()
->ToObject(context)
.ToLocalChecked();
v8::Local<v8::Object> c = b->GetInternalField(0)
.As<v8::Value>()
->ToObject(context)
.ToLocalChecked();

InternalFieldData* a1 = reinterpret_cast<InternalFieldData*>(
a->GetAlignedPointerFromInternalField(1));
v8::Local<v8::Value> a2 = a->GetInternalField(2);
v8::Local<v8::Value> a2 = a->GetInternalField(2).As<v8::Value>();

InternalFieldData* b1 = reinterpret_cast<InternalFieldData*>(
b->GetAlignedPointerFromInternalField(1));
v8::Local<v8::Value> b2 = b->GetInternalField(2);
v8::Local<v8::Value> b2 = b->GetInternalField(2).As<v8::Value>();

v8::Local<v8::Value> c0 = c->GetInternalField(0);
v8::Local<v8::Value> c0 = c->GetInternalField(0).As<v8::Value>();
InternalFieldData* c1 = reinterpret_cast<InternalFieldData*>(
c->GetAlignedPointerFromInternalField(1));
v8::Local<v8::Value> c2 = c->GetInternalField(2);
v8::Local<v8::Value> c2 = c->GetInternalField(2).As<v8::Value>();

CHECK(c0->IsUndefined());

Expand Down
10 changes: 8 additions & 2 deletions deps/v8/test/unittests/objects/value-serializer-unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,14 @@ class ValueSerializerTest : public TestWithIsolate {
function_template->InstanceTemplate()->SetAccessor(
StringFromUtf8("value"),
[](Local<String> property, const PropertyCallbackInfo<Value>& args) {
args.GetReturnValue().Set(args.Holder()->GetInternalField(0));
args.GetReturnValue().Set(
args.Holder()->GetInternalField(0).As<v8::Value>());
});
function_template->InstanceTemplate()->SetAccessor(
StringFromUtf8("value2"),
[](Local<String> property, const PropertyCallbackInfo<Value>& args) {
args.GetReturnValue().Set(args.Holder()->GetInternalField(1));
args.GetReturnValue().Set(
args.Holder()->GetInternalField(1).As<v8::Value>());
});
for (Local<Context> context :
{serialization_context_, deserialization_context_}) {
Expand Down Expand Up @@ -2897,6 +2899,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint32) {
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
uint32_t value = 0;
EXPECT_TRUE(object->GetInternalField(0)
.As<v8::Value>()
->Uint32Value(serialization_context())
.To(&value));
WriteExampleHostObjectTag();
Expand Down Expand Up @@ -2928,9 +2931,11 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint64) {
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
uint32_t value = 0, value2 = 0;
EXPECT_TRUE(object->GetInternalField(0)
.As<v8::Value>()
->Uint32Value(serialization_context())
.To(&value));
EXPECT_TRUE(object->GetInternalField(1)
.As<v8::Value>()
->Uint32Value(serialization_context())
.To(&value2));
WriteExampleHostObjectTag();
Expand Down Expand Up @@ -2968,6 +2973,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripDouble) {
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
double value = 0;
EXPECT_TRUE(object->GetInternalField(0)
.As<v8::Value>()
->NumberValue(serialization_context())
.To(&value));
WriteExampleHostObjectTag();
Expand Down

0 comments on commit ab06393

Please sign in to comment.