[snapshot] Simplify ArrayBuffer deserialization

It is no longer necessary to postpone the allocation of backing stores
to avoid triggering GC. As such, the logic around ArrayBuffer
deserialization can be simplified.

Bug: v8:10391, v8:11111
Change-Id: I7410392a6e658cd4be77e2192483c6d412b63412
Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3717982Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81384}

src/snapshot/context-deserializer.cc

@@ -52,25 +52,10 @@ MaybeHandle<Object> ContextDeserializer::Deserialize(
   }
 
   if (should_rehash()) Rehash();
-  SetupOffHeapArrayBufferBackingStores();
 
   return result;
 }
 
-void ContextDeserializer::SetupOffHeapArrayBufferBackingStores() {
-  for (Handle<JSArrayBuffer> buffer : new_off_heap_array_buffers()) {
-    uint32_t store_index = buffer->GetBackingStoreRefForDeserialization();
-    auto bs = backing_store(store_index);
-    SharedFlag shared =
-        bs && bs->is_shared() ? SharedFlag::kShared : SharedFlag::kNotShared;
-    DCHECK_IMPLIES(bs, buffer->is_resizable() == bs->is_resizable());
-    ResizableFlag resizable = bs && bs->is_resizable()
-                                  ? ResizableFlag::kResizable
-                                  : ResizableFlag::kNotResizable;
-    buffer->Setup(shared, resizable, bs);
-  }
-}
-
 void ContextDeserializer::DeserializeEmbedderFields(
     v8::DeserializeEmbedderFieldsCallback embedder_fields_deserializer) {
   if (!source()->HasMore() || source()->Get() != kEmbedderFieldsData) return;

src/snapshot/context-deserializer.h

@@ -38,8 +38,6 @@ class V8_EXPORT_PRIVATE ContextDeserializer final
 
   void DeserializeEmbedderFields(
       v8::DeserializeEmbedderFieldsCallback embedder_fields_deserializer);
-
-  void SetupOffHeapArrayBufferBackingStores();
 };
 
 }  // namespace internal

src/snapshot/deserializer.cc

@@ -372,7 +372,6 @@ void Deserializer<IsolateT>::PostProcessNewJSReceiver(
   if (InstanceTypeChecker::IsJSDataView(instance_type)) {
     auto data_view = JSDataView::cast(raw_obj);
     auto buffer = JSArrayBuffer::cast(data_view.buffer());
-    void* backing_store = EmptyBackingStoreBuffer();
     if (buffer.was_detached()) {
       // Directly set the data pointer to point to the EmptyBackingStoreBuffer.
       // Otherwise, we might end up setting it to EmptyBackingStoreBuffer() +
@@ -380,20 +379,7 @@ void Deserializer<IsolateT>::PostProcessNewJSReceiver(
       data_view.set_data_pointer(main_thread_isolate(),
                                  EmptyBackingStoreBuffer());
     } else {
-      // At this point, the backing store may already have been set if this is
-      // an empty ArrayBuffer (see the IsJSArrayBuffer case below). In that
-      // case, the backing store ref/index is no longer valid so explicitly
-      // check here if the buffer is empty before using the store index.
-      if (buffer.backing_store() != EmptyBackingStoreBuffer()) {
-        uint32_t store_index = buffer.GetBackingStoreRefForDeserialization();
-        if (store_index != kEmptyBackingStoreRefSentinel) {
-          // The backing store of the JSArrayBuffer has not been correctly
-          // restored yet, as that may trigger GC. The backing_store field
-          // currently contains a numbered reference to an already deserialized
-          // backing store.
-          backing_store = backing_stores_[store_index]->buffer_start();
-        }
-      }
+      void* backing_store = buffer.backing_store();
       data_view.set_data_pointer(
           main_thread_isolate(),
           reinterpret_cast<uint8_t*>(backing_store) + data_view.byte_offset());
@@ -416,13 +402,19 @@ void Deserializer<IsolateT>::PostProcessNewJSReceiver(
     }
   } else if (InstanceTypeChecker::IsJSArrayBuffer(instance_type)) {
     auto buffer = JSArrayBuffer::cast(raw_obj);
-    // Postpone allocation of backing store to avoid triggering the GC.
-    if (buffer.GetBackingStoreRefForDeserialization() !=
-        kEmptyBackingStoreRefSentinel) {
-      new_off_heap_array_buffers_.push_back(Handle<JSArrayBuffer>::cast(obj));
-    } else {
+    uint32_t store_index = buffer.GetBackingStoreRefForDeserialization();
+    if (store_index == kEmptyBackingStoreRefSentinel) {
       buffer.set_backing_store(main_thread_isolate(),
                                EmptyBackingStoreBuffer());
+    } else {
+      auto bs = backing_store(store_index);
+      SharedFlag shared =
+          bs && bs->is_shared() ? SharedFlag::kShared : SharedFlag::kNotShared;
+      DCHECK_IMPLIES(bs, buffer.is_resizable() == bs->is_resizable());
+      ResizableFlag resizable = bs && bs->is_resizable()
+                                    ? ResizableFlag::kResizable
+                                    : ResizableFlag::kNotResizable;
+      buffer.Setup(shared, resizable, bs);
     }
   }
 }

src/snapshot/deserializer.h

@@ -77,10 +77,6 @@ class Deserializer : public SerializerDeserializer {
     attached_objects_.push_back(attached_object);
   }
 
-  void CheckNoArrayBufferBackingStores() {
-    CHECK_EQ(new_off_heap_array_buffers().size(), 0);
-  }
-
   IsolateT* isolate() const { return isolate_; }
 
   Isolate* main_thread_isolate() const { return isolate_->AsIsolate(); }
@@ -103,10 +99,6 @@ class Deserializer : public SerializerDeserializer {
     return new_scripts_;
   }
 
-  const std::vector<Handle<JSArrayBuffer>>& new_off_heap_array_buffers() const {
-    return new_off_heap_array_buffers_;
-  }
-
   const std::vector<Handle<DescriptorArray>>& new_descriptor_arrays() const {
     return new_descriptor_arrays_;
   }
@@ -221,7 +213,6 @@ class Deserializer : public SerializerDeserializer {
   std::vector<Handle<AccessorInfo>> accessor_infos_;
   std::vector<Handle<CallHandlerInfo>> call_handler_infos_;
   std::vector<Handle<Script>> new_scripts_;
-  std::vector<Handle<JSArrayBuffer>> new_off_heap_array_buffers_;
   std::vector<Handle<DescriptorArray>> new_descriptor_arrays_;
   std::vector<std::shared_ptr<BackingStore>> backing_stores_;
 

src/snapshot/object-deserializer.cc

@@ -54,16 +54,6 @@ MaybeHandle<HeapObject> ObjectDeserializer::Deserialize() {
 }
 
 void ObjectDeserializer::CommitPostProcessedObjects() {
-  for (Handle<JSArrayBuffer> buffer : new_off_heap_array_buffers()) {
-    uint32_t store_index = buffer->GetBackingStoreRefForDeserialization();
-    auto bs = backing_store(store_index);
-    SharedFlag shared =
-        bs && bs->is_shared() ? SharedFlag::kShared : SharedFlag::kNotShared;
-    // TODO(v8:11111): Support RAB / GSAB.
-    CHECK(!bs || !bs->is_resizable());
-    buffer->Setup(shared, ResizableFlag::kNotResizable, bs);
-  }
-
   for (Handle<Script> script : new_scripts()) {
     // Assign a new script id to avoid collision.
     script->set_id(isolate()->GetNextScriptId());
@@ -131,7 +121,6 @@ MaybeHandle<HeapObject> OffThreadObjectDeserializer::Deserialize(
   }
 
   Rehash();
-  CHECK(new_off_heap_array_buffers().empty());
 
   // TODO(leszeks): Figure out a better way of dealing with scripts.
   CHECK_EQ(new_scripts().size(), 1);

src/snapshot/read-only-deserializer.cc

@@ -46,7 +46,6 @@ void ReadOnlyDeserializer::DeserializeIntoIsolate() {
       if (object->IsUndefined(roots)) break;
     }
     DeserializeDeferredObjects();
-    CheckNoArrayBufferBackingStores();
 #ifdef DEBUG
     roots.VerifyNameForProtectors();
 #endif

src/snapshot/startup-deserializer.cc

@@ -48,8 +48,6 @@ void StartupDeserializer::DeserializeIntoIsolate() {
     FlushICache();
   }
 
-  CheckNoArrayBufferBackingStores();
-
   isolate()->heap()->set_native_contexts_list(
       ReadOnlyRoots(isolate()).undefined_value());
   // The allocation site list is build during root iteration, but if no sites