[serializer] serialize ExternalPointers in InterceptorInfo properly

joyeecheung · V8 LUCI CQ · commit d2ad518a0b57 · 2025-05-16T02:34:00.000-07:00
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: nodejs/node#58064 Change-Id: Icc62a01b006eaf68d1d2be1e3bc98b448f0c66dc Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6516091 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Reviewed-by: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/main@{#100315}
diff --git a/src/snapshot/deserializer.cc b/src/snapshot/deserializer.cc
@@ -275,7 +275,11 @@ int Deserializer<IsolateT>::WriteExternalPointer(Tagged<HeapObject> host,
   }
 #endif  // V8_ENABLE_SANDBOX
 
-  dest.init(main_thread_isolate(), host, value, tag);
+  if (tag == kExternalPointerNullTag && value == kNullAddress) {
+    dest.init_lazily_initialized();
+  } else {
+    dest.init(main_thread_isolate(), host, value, tag);
+  }
 
 #ifdef V8_ENABLE_SANDBOX
   if (managed_resource) {
diff --git a/src/snapshot/serializer.cc b/src/snapshot/serializer.cc
@@ -1074,7 +1074,8 @@ void Serializer::ObjectSerializer::OutputExternalReference(
     Address target, int target_size, bool sandboxify, ExternalPointerTag tag) {
   DCHECK_LE(target_size, sizeof(target));  // Must fit in Address.
   DCHECK_IMPLIES(sandboxify, V8_ENABLE_SANDBOX_BOOL);
-  DCHECK_IMPLIES(sandboxify, tag != kExternalPointerNullTag);
+  DCHECK_IMPLIES(sandboxify,
+                 tag != kExternalPointerNullTag || target == kNullAddress);
   ExternalReferenceEncoder::Value encoded_reference;
   bool encoded_successfully;
 
@@ -1153,6 +1154,7 @@ void Serializer::ObjectSerializer::VisitExternalPointer(
   if (InstanceTypeChecker::IsForeign(instance_type) ||
       InstanceTypeChecker::IsJSExternalObject(instance_type) ||
       InstanceTypeChecker::IsAccessorInfo(instance_type) ||
+      InstanceTypeChecker::IsInterceptorInfo(instance_type) ||
       InstanceTypeChecker::IsFunctionTemplateInfo(instance_type)) {
     // Output raw data payload, if any.
     OutputRawData(slot.address());
diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
@@ -5203,6 +5203,77 @@ UNINITIALIZED_TEST(SnapshotCreatorIncludeGlobalProxy) {
   FreeCurrentEmbeddedBlob();
 }
 
+UNINITIALIZED_TEST(SnapshotCreatorSerializeInterceptorInOldSpace) {
+  DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
+  v8::StartupData blob;
+
+  {
+    SnapshotCreatorParams testing_params(original_external_references);
+    v8::SnapshotCreator creator(testing_params.create_params);
+    v8::Isolate* isolate = creator.GetIsolate();
+
+    {
+      v8::HandleScope handle_scope(isolate);
+
+      v8::Local<v8::ObjectTemplate> global_template =
+          v8::ObjectTemplate::New(isolate);
+
+      NamedPropertyHandlerConfiguration config(
+          NamedPropertyGetterForSerialization, {}, {}, {}, {}, {}, {},
+          v8_str("test"),  // Stop it from being promoted to RO space.
+          PropertyHandlerFlags::kHasNoSideEffect);
+
+      global_template->SetHandler(config);
+
+      v8::Local<v8::Context> context =
+          v8::Context::New(isolate, nullptr, global_template);
+      v8::Context::Scope context_scope(context);
+      ExpectInt32("x", 2016);
+      creator.SetDefaultContext(context);
+    }
+
+    blob =
+        creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
+  }
+
+  {
+    v8::Isolate::CreateParams params;
+    params.snapshot_blob = &blob;
+    params.array_buffer_allocator = CcTest::array_buffer_allocator();
+    params.external_references = original_external_references;
+    // Test-appropriate equivalent of v8::Isolate::New.
+    v8::Isolate* isolate = TestSerializer::NewIsolate(params);
+    {
+      v8::Isolate::Scope isolate_scope(isolate);
+      v8::HandleScope handle_scope(isolate);
+
+      v8::Local<v8::Context> context = v8::Context::New(isolate);
+      v8::Context::Scope context_scope(context);
+
+      // Check that the InterceptorInfo is not promoted to RO space after
+      // deserialization.
+      i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+      CHECK(i_isolate->global_object()->map()->has_named_interceptor());
+      CHECK(i_isolate->heap()->InOldSpace(
+          i_isolate->global_object()->GetNamedInterceptor()));
+
+      ExpectInt32("x", 2016);  // Check deserialized getter.
+      // Check the unset interceptors.
+      CompileRun(
+          "Object.defineProperty(globalThis, 'test', {"
+          "  value: 0, enumerable: true"
+          "})");
+      ExpectFalse("delete globalThis.test");
+      ExpectTrue("Object.keys(globalThis).includes('test')");
+    }
+
+    isolate->Dispose();
+  }
+  delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
+}
+
 UNINITIALIZED_TEST(ReinitializeHashSeedJSCollectionRehashable) {
   DisableAlwaysOpt();
   i::v8_flags.rehash_snapshot = true;
