Skip to content

Commit 6bfc525

Browse files
joyeecheungnodejs-github-bot
authored andcommitted
deps: V8: cherry-pick d2ad518a0b57
Original commit message: [serializer] serialize ExternalPointers in InterceptorInfo properly Previously the ObjectSerializer didn't serialize the ExternalPointers in the InterceptorInfo properly, but this case can be shadowed by the fact that they get promoted to RO space by default and don't get serialized by ObjectSerializer. This patch fixes up the missing handling in ObjectSerializer and adds a test case for this path. Refs: #58064 Change-Id: Icc62a01b006eaf68d1d2be1e3bc98b448f0c66dc Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6516091 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/main@{#100315} Refs: v8/v8@d2ad518 PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
1 parent 754d28e commit 6bfc525

File tree

4 files changed

+80
-3
lines changed

4 files changed

+80
-3
lines changed

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.8',
41+
'v8_embedder_string': '-node.9',
4242

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

deps/v8/src/snapshot/deserializer.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,11 @@ int Deserializer<IsolateT>::WriteExternalPointer(Tagged<HeapObject> host,
275275
}
276276
#endif // V8_ENABLE_SANDBOX
277277

278-
dest.init(main_thread_isolate(), host, value, tag);
278+
if (tag == kExternalPointerNullTag && value == kNullAddress) {
279+
dest.init_lazily_initialized();
280+
} else {
281+
dest.init(main_thread_isolate(), host, value, tag);
282+
}
279283

280284
#ifdef V8_ENABLE_SANDBOX
281285
if (managed_resource) {

deps/v8/src/snapshot/serializer.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,8 @@ void Serializer::ObjectSerializer::OutputExternalReference(
10741074
Address target, int target_size, bool sandboxify, ExternalPointerTag tag) {
10751075
DCHECK_LE(target_size, sizeof(target)); // Must fit in Address.
10761076
DCHECK_IMPLIES(sandboxify, V8_ENABLE_SANDBOX_BOOL);
1077-
DCHECK_IMPLIES(sandboxify, tag != kExternalPointerNullTag);
1077+
DCHECK_IMPLIES(sandboxify,
1078+
tag != kExternalPointerNullTag || target == kNullAddress);
10781079
ExternalReferenceEncoder::Value encoded_reference;
10791080
bool encoded_successfully;
10801081

@@ -1153,6 +1154,7 @@ void Serializer::ObjectSerializer::VisitExternalPointer(
11531154
if (InstanceTypeChecker::IsForeign(instance_type) ||
11541155
InstanceTypeChecker::IsJSExternalObject(instance_type) ||
11551156
InstanceTypeChecker::IsAccessorInfo(instance_type) ||
1157+
InstanceTypeChecker::IsInterceptorInfo(instance_type) ||
11561158
InstanceTypeChecker::IsFunctionTemplateInfo(instance_type)) {
11571159
// Output raw data payload, if any.
11581160
OutputRawData(slot.address());

deps/v8/test/cctest/test-serialize.cc

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5203,6 +5203,77 @@ UNINITIALIZED_TEST(SnapshotCreatorIncludeGlobalProxy) {
52035203
FreeCurrentEmbeddedBlob();
52045204
}
52055205

5206+
UNINITIALIZED_TEST(SnapshotCreatorSerializeInterceptorInOldSpace) {
5207+
DisableAlwaysOpt();
5208+
DisableEmbeddedBlobRefcounting();
5209+
v8::StartupData blob;
5210+
5211+
{
5212+
SnapshotCreatorParams testing_params(original_external_references);
5213+
v8::SnapshotCreator creator(testing_params.create_params);
5214+
v8::Isolate* isolate = creator.GetIsolate();
5215+
5216+
{
5217+
v8::HandleScope handle_scope(isolate);
5218+
5219+
v8::Local<v8::ObjectTemplate> global_template =
5220+
v8::ObjectTemplate::New(isolate);
5221+
5222+
NamedPropertyHandlerConfiguration config(
5223+
NamedPropertyGetterForSerialization, {}, {}, {}, {}, {}, {},
5224+
v8_str("test"), // Stop it from being promoted to RO space.
5225+
PropertyHandlerFlags::kHasNoSideEffect);
5226+
5227+
global_template->SetHandler(config);
5228+
5229+
v8::Local<v8::Context> context =
5230+
v8::Context::New(isolate, nullptr, global_template);
5231+
v8::Context::Scope context_scope(context);
5232+
ExpectInt32("x", 2016);
5233+
creator.SetDefaultContext(context);
5234+
}
5235+
5236+
blob =
5237+
creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
5238+
}
5239+
5240+
{
5241+
v8::Isolate::CreateParams params;
5242+
params.snapshot_blob = &blob;
5243+
params.array_buffer_allocator = CcTest::array_buffer_allocator();
5244+
params.external_references = original_external_references;
5245+
// Test-appropriate equivalent of v8::Isolate::New.
5246+
v8::Isolate* isolate = TestSerializer::NewIsolate(params);
5247+
{
5248+
v8::Isolate::Scope isolate_scope(isolate);
5249+
v8::HandleScope handle_scope(isolate);
5250+
5251+
v8::Local<v8::Context> context = v8::Context::New(isolate);
5252+
v8::Context::Scope context_scope(context);
5253+
5254+
// Check that the InterceptorInfo is not promoted to RO space after
5255+
// deserialization.
5256+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
5257+
CHECK(i_isolate->global_object()->map()->has_named_interceptor());
5258+
CHECK(i_isolate->heap()->InOldSpace(
5259+
i_isolate->global_object()->GetNamedInterceptor()));
5260+
5261+
ExpectInt32("x", 2016); // Check deserialized getter.
5262+
// Check the unset interceptors.
5263+
CompileRun(
5264+
"Object.defineProperty(globalThis, 'test', {"
5265+
" value: 0, enumerable: true"
5266+
"})");
5267+
ExpectFalse("delete globalThis.test");
5268+
ExpectTrue("Object.keys(globalThis).includes('test')");
5269+
}
5270+
5271+
isolate->Dispose();
5272+
}
5273+
delete[] blob.data;
5274+
FreeCurrentEmbeddedBlob();
5275+
}
5276+
52065277
UNINITIALIZED_TEST(ReinitializeHashSeedJSCollectionRehashable) {
52075278
DisableAlwaysOpt();
52085279
i::v8_flags.rehash_snapshot = true;

0 commit comments

Comments
 (0)