ArrayBuffer refactoring in preparation for CagedPointers

The main changes of this CL are:

It should no longer be assumed that an empty ArrayBuffer has a nullptr
backing store. This is in preparation for the move to caged pointers,
which cannot represent nullptr, and will instead likely provide a
EmptyBackingStore constant pointing inside the virtual memory cage. For
that reason, a new JSArrayBuffer::IsEmpty() helper is introduced, which
should be used instead of checking against nullptr.

CodeStubAssembler::GetTypedArrayBuffer now checks for on-heap
TypedArrays instead of comparing the backing store pointer to nullptr.
This is consistent with the implementation in JSTypedArray::GetBuffer.

v8::ArrayBufferView::CopyContents now uses JSTypedArray::DataPtr instead
of relying on nullptr backing stores to handle on-heap TypedArrays.

The serializer and deserializer now check for IsEmpty() and use the
kEmptyBackingStoreRefSentinel value to serialize empty backing stores.

Empty ArrayBuffers allocated for on-heap TypedArrays now have a
byte_length of zero. This allows removing the allocation_length() (and
allocation_buffer()) methods, which were only (incorrectly, as they
don't account for GSABs) used for memory measurements.

Bug: chromium:1218005
Change-Id: Ib889ccf855f68525f7a614f3963e46ea56865fa3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3297709Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78069}

src/api/api.cc

@@ -8030,21 +8030,20 @@ Local<ArrayBuffer> v8::ArrayBufferView::Buffer() {
 
 size_t v8::ArrayBufferView::CopyContents(void* dest, size_t byte_length) {
   i::Handle<i::JSArrayBufferView> self = Utils::OpenHandle(this);
-  size_t byte_offset = self->byte_offset();
   size_t bytes_to_copy = std::min(byte_length, self->byte_length());
   if (bytes_to_copy) {
     i::DisallowGarbageCollection no_gc;
     i::Isolate* isolate = self->GetIsolate();
-    i::Handle<i::JSArrayBuffer> buffer(i::JSArrayBuffer::cast(self->buffer()),
-                                       isolate);
-    const char* source = reinterpret_cast<char*>(buffer->backing_store());
-    if (source == nullptr) {
-      DCHECK(self->IsJSTypedArray());
-      i::Handle<i::JSTypedArray> typed_array(i::JSTypedArray::cast(*self),
-                                             isolate);
-      source = reinterpret_cast<char*>(typed_array->DataPtr());
+    const char* source;
+    if (self->IsJSTypedArray()) {
+      i::Handle<i::JSTypedArray> array(i::JSTypedArray::cast(*self), isolate);
+      source = reinterpret_cast<char*>(array->DataPtr());
+    } else {
+      DCHECK(self->IsJSDataView());
+      i::Handle<i::JSDataView> data_view(i::JSDataView::cast(*self), isolate);
+      source = reinterpret_cast<char*>(data_view->data_pointer());
     }
-    memcpy(dest, source + byte_offset, bytes_to_copy);
+    memcpy(dest, source, bytes_to_copy);
   }
   return bytes_to_copy;
 }

src/builtins/builtins-typed-array-gen.cc

@@ -33,7 +33,7 @@ void TypedArrayBuiltinsAssembler::SetupTypedArrayEmbedderFields(
 // elements.
 // TODO(bmeurer,v8:4153): Rename this and maybe fix up the implementation a bit.
 TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
-    TNode<Context> context, TNode<UintPtrT> byte_length) {
+    TNode<Context> context) {
   TNode<NativeContext> native_context = LoadNativeContext(context);
   TNode<Map> map =
       CAST(LoadContextElement(native_context, Context::ARRAY_BUFFER_MAP_INDEX));
@@ -49,7 +49,7 @@ TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
   // Setup the ArrayBuffer.
   //  - Set BitField to 0.
   //  - Set IsExternal and IsDetachable bits of BitFieldSlot.
-  //  - Set the byte_length field to byte_length.
+  //  - Set the byte_length field to zero.
   //  - Set backing_store to null/Smi(0).
   //  - Set extension to null.
   //  - Set all embedder fields to Smi(0).
@@ -64,7 +64,7 @@ TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
                                  Int32Constant(bitfield_value));
 
   StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kByteLengthOffset,
-                                 byte_length);
+                                 UintPtrConstant(0));
   StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kBackingStoreOffset,
                                  PointerConstant(nullptr));
   StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,

src/builtins/builtins-typed-array-gen.h

@@ -21,8 +21,7 @@ class TypedArrayBuiltinsAssembler : public CodeStubAssembler {
                     TNode<Map> map, TNode<Smi> length,
                     TNode<UintPtrT> byte_offset);
 
-  TNode<JSArrayBuffer> AllocateEmptyOnHeapBuffer(TNode<Context> context,
-                                                 TNode<UintPtrT> byte_length);
+  TNode<JSArrayBuffer> AllocateEmptyOnHeapBuffer(TNode<Context> context);
 
   TNode<Map> LoadMapForType(TNode<JSTypedArray> array);
   TNode<BoolT> IsMockArrayBufferAllocatorFlag();

src/builtins/typed-array-createtypedarray.tq

@@ -9,7 +9,7 @@ extern builtin IterableToListMayPreserveHoles(
     Context, Object, Callable): JSArray;
 
 extern macro TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
-    implicit context: Context)(uintptr): JSArrayBuffer;
+    implicit context: Context)(): JSArrayBuffer;
 extern macro CodeStubAssembler::AllocateByteArray(uintptr): ByteArray;
 extern macro TypedArrayBuiltinsAssembler::GetDefaultConstructor(
     implicit context: Context)(JSTypedArray): JSFunction;
@@ -93,7 +93,7 @@ transitioning macro TypedArrayInitialize(implicit context: Context)(
 
     if (byteLength > kMaxTypedArrayInHeap) goto AllocateOffHeap;
 
-    const buffer = AllocateEmptyOnHeapBuffer(byteLength);
+    const buffer = AllocateEmptyOnHeapBuffer();
 
     const isOnHeap: constexpr bool = true;
     const isLengthTracking: constexpr bool = false;

src/codegen/code-stub-assembler.cc

@@ -14107,10 +14107,10 @@ TNode<JSArrayBuffer> CodeStubAssembler::GetTypedArrayBuffer(
   Label call_runtime(this), done(this);
   TVARIABLE(Object, var_result);
 
+  GotoIf(IsOnHeapTypedArray(array), &call_runtime);
+
   TNode<JSArrayBuffer> buffer = LoadJSArrayBufferViewBuffer(array);
   GotoIf(IsDetachedBuffer(buffer), &call_runtime);
-  TNode<RawPtrT> backing_store = LoadJSArrayBufferBackingStorePtr(buffer);
-  GotoIf(WordEqual(backing_store, IntPtrConstant(0)), &call_runtime);
   var_result = buffer;
   Goto(&done);
 

src/heap/memory-measurement.cc

@@ -401,7 +401,7 @@ void NativeContextStats::IncrementExternalSize(Address context, Map map,
   InstanceType instance_type = map.instance_type();
   size_t external_size = 0;
   if (instance_type == JS_ARRAY_BUFFER_TYPE) {
-    external_size = JSArrayBuffer::cast(object).allocation_length();
+    external_size = JSArrayBuffer::cast(object).GetByteLength();
   } else {
     DCHECK(InstanceTypeChecker::IsExternalString(instance_type));
     external_size = ExternalString::cast(object).ExternalPayloadSize();

src/objects/backing-store.h

@@ -98,6 +98,11 @@ class V8_EXPORT_PRIVATE BackingStore : public BackingStoreBase {
   bool has_guard_regions() const { return has_guard_regions_; }
   bool free_on_destruct() const { return free_on_destruct_; }
 
+  bool IsEmpty() const {
+    DCHECK_GE(max_byte_length_, byte_length_);
+    return max_byte_length_ == 0;
+  }
+
   enum ResizeOrGrowResult { kSuccess, kFailure, kRace };
 
   ResizeOrGrowResult ResizeInPlace(Isolate* isolate, size_t new_byte_length,

src/objects/js-array-buffer-inl.h

@@ -44,6 +44,22 @@ void JSArrayBuffer::set_backing_store(void* value) {
   WriteField<Address>(kBackingStoreOffset, reinterpret_cast<Address>(value));
 }
 
+std::shared_ptr<BackingStore> JSArrayBuffer::GetBackingStore() const {
+  if (!extension()) return nullptr;
+  return extension()->backing_store();
+}
+
+size_t JSArrayBuffer::GetByteLength() const {
+  if V8_UNLIKELY (is_shared() && is_resizable()) {
+    // Invariant: byte_length for GSAB is 0 (it needs to be read from the
+    // BackingStore).
+    DCHECK_EQ(0, byte_length());
+
+    return GetBackingStore()->byte_length(std::memory_order_seq_cst);
+  }
+  return byte_length();
+}
+
 uint32_t JSArrayBuffer::GetBackingStoreRefForDeserialization() const {
   return static_cast<uint32_t>(ReadField<Address>(kBackingStoreOffset));
 }
@@ -114,20 +130,6 @@ uint32_t* JSArrayBuffer::extension_hi() const {
 }
 #endif
 
-size_t JSArrayBuffer::allocation_length() const {
-  if (backing_store() == nullptr) {
-    return 0;
-  }
-  return byte_length();
-}
-
-void* JSArrayBuffer::allocation_base() const {
-  if (backing_store() == nullptr) {
-    return nullptr;
-  }
-  return backing_store();
-}
-
 void JSArrayBuffer::clear_padding() {
   if (FIELD_SIZE(kOptionalPaddingOffset) != 0) {
     DCHECK_EQ(4, FIELD_SIZE(kOptionalPaddingOffset));
@@ -158,6 +160,13 @@ BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, is_shared,
 BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, is_resizable,
                     JSArrayBuffer::IsResizableBit)
 
+bool JSArrayBuffer::IsEmpty() const {
+  auto backing_store = GetBackingStore();
+  bool is_empty = !backing_store || backing_store->IsEmpty();
+  DCHECK_IMPLIES(is_empty, byte_length() == 0);
+  return is_empty;
+}
+
 size_t JSArrayBufferView::byte_offset() const {
   return ReadField<size_t>(kByteOffsetOffset);
 }

src/objects/js-array-buffer.cc

@@ -125,22 +125,6 @@ void JSArrayBuffer::Detach(bool force_for_wasm_memory) {
   set_was_detached(true);
 }
 
-std::shared_ptr<BackingStore> JSArrayBuffer::GetBackingStore() const {
-  if (!extension()) return nullptr;
-  return extension()->backing_store();
-}
-
-size_t JSArrayBuffer::GetByteLength() const {
-  if V8_UNLIKELY (is_shared() && is_resizable()) {
-    // Invariant: byte_length for GSAB is 0 (it needs to be read from the
-    // BackingStore).
-    DCHECK_EQ(0, byte_length());
-
-    return GetBackingStore()->byte_length(std::memory_order_seq_cst);
-  }
-  return byte_length();
-}
-
 size_t JSArrayBuffer::GsabByteLength(Isolate* isolate,
                                      Address raw_array_buffer) {
   // TODO(v8:11111): Cache the last seen length in JSArrayBuffer and use it
@@ -207,7 +191,7 @@ Handle<JSArrayBuffer> JSTypedArray::GetBuffer() {
   DCHECK(!array_buffer->is_resizable());
 
   // The existing array buffer should be empty.
-  DCHECK_NULL(array_buffer->backing_store());
+  DCHECK(array_buffer->IsEmpty());
 
   // Allocate a new backing store and attach it to the existing array buffer.
   size_t byte_length = self->byte_length();

src/objects/js-array-buffer.h

@@ -37,17 +37,13 @@ class JSArrayBuffer
   DECL_PRIMITIVE_ACCESSORS(byte_length, size_t)
 
   // [backing_store]: backing memory for this array
+  // It should not be assumed that this will be nullptr for empty ArrayBuffers.
   DECL_GETTER(backing_store, void*)
   inline void set_backing_store(void* value);
 
   // [extension]: extension object used for GC
   DECL_PRIMITIVE_ACCESSORS(extension, ArrayBufferExtension*)
 
-  // For non-wasm, allocation_length and allocation_base are byte_length and
-  // backing_store, respectively.
-  inline size_t allocation_length() const;
-  inline void* allocation_base() const;
-
   // [bit_field]: boolean flags
   DECL_PRIMITIVE_ACCESSORS(bit_field, uint32_t)
 
@@ -80,6 +76,13 @@ class JSArrayBuffer
   // GrowableSharedArrayBuffer.
   DECL_BOOLEAN_ACCESSORS(is_resizable)
 
+  // An ArrayBuffer is empty if its BackingStore is empty or if there is none.
+  // An empty ArrayBuffer will have a byte_length of zero but not necessarily a
+  // nullptr backing_store. An ArrayBuffer with a byte_length of zero may not
+  // necessarily be empty though, as it may be a GrowableSharedArrayBuffer.
+  // An ArrayBuffer with a size greater than zero is never empty.
+  DECL_GETTER(IsEmpty, bool)
+
   // Initializes the fields of the ArrayBuffer. The provided backing_store can
   // be nullptr. If it is not nullptr, then the function registers it with
   // src/heap/array-buffer-tracker.h.
@@ -104,9 +107,9 @@ class JSArrayBuffer
   // Get a reference to backing store of this array buffer, if there is a
   // backing store. Returns nullptr if there is no backing store (e.g. detached
   // or a zero-length array buffer).
-  std::shared_ptr<BackingStore> GetBackingStore() const;
+  inline std::shared_ptr<BackingStore> GetBackingStore() const;
 
-  size_t GetByteLength() const;
+  inline size_t GetByteLength() const;
 
   static size_t GsabByteLength(Isolate* isolate, Address raw_array_buffer);
 

src/objects/js-array-buffer.tq

@@ -78,6 +78,12 @@ extern class JSTypedArray extends JSArrayBufferView {
   base_pointer: ByteArray|Smi;
 }
 
+@export
+macro IsOnHeapTypedArray(array: JSTypedArray): bool {
+  // See JSTypedArray::is_on_heap()
+  return TaggedNotEqual(array.base_pointer, SmiConstant(0));
+}
+
 extern class JSDataView extends JSArrayBufferView {
   data_pointer: ExternalPointer;
 }

src/snapshot/deserializer.cc

@@ -249,9 +249,9 @@ Deserializer<IsolateT>::Deserializer(IsolateT* isolate,
   isolate->RegisterDeserializerStarted();
 
   // We start the indices here at 1, so that we can distinguish between an
-  // actual index and a nullptr (serialized as kNullRefSentinel) in a
-  // deserialized object requiring fix-up.
-  STATIC_ASSERT(kNullRefSentinel == 0);
+  // actual index and an empty backing store (serialized as
+  // kEmptyBackingStoreRefSentinel) in a deserialized object requiring fix-up.
+  STATIC_ASSERT(kEmptyBackingStoreRefSentinel == 0);
   backing_stores_.push_back({});
 
 #ifdef DEBUG
@@ -488,7 +488,7 @@ void Deserializer<IsolateT>::PostProcessNewObject(Handle<Map> map,
     JSArrayBuffer buffer = JSArrayBuffer::cast(data_view->buffer());
     void* backing_store = nullptr;
     uint32_t store_index = buffer.GetBackingStoreRefForDeserialization();
-    if (store_index != kNullRefSentinel) {
+    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.
@@ -519,7 +519,8 @@ void Deserializer<IsolateT>::PostProcessNewObject(Handle<Map> map,
   } else if (InstanceTypeChecker::IsJSArrayBuffer(instance_type)) {
     Handle<JSArrayBuffer> buffer = Handle<JSArrayBuffer>::cast(obj);
     // Postpone allocation of backing store to avoid triggering the GC.
-    if (buffer->GetBackingStoreRefForDeserialization() != kNullRefSentinel) {
+    if (buffer->GetBackingStoreRefForDeserialization() !=
+        kEmptyBackingStoreRefSentinel) {
       new_off_heap_array_buffers_.push_back(buffer);
     } else {
       buffer->set_backing_store(nullptr);

src/snapshot/serializer-deserializer.h

@@ -259,9 +259,9 @@ class SerializerDeserializer : public RootVisitor {
                            RootIndex>;
   using HotObject = BytecodeValueEncoder<kHotObject, 0, kHotObjectCount - 1>;
 
-  // This backing store reference value represents nullptr values during
+  // This backing store reference value represents empty backing stores during
   // serialization/deserialization.
-  static const uint32_t kNullRefSentinel = 0;
+  static const uint32_t kEmptyBackingStoreRefSentinel = 0;
 };
 
 }  // namespace internal

src/snapshot/serializer.cc

@@ -528,16 +528,16 @@ void Serializer::ObjectSerializer::SerializeJSArrayBuffer() {
   int32_t byte_length = static_cast<int32_t>(buffer->byte_length());
   ArrayBufferExtension* extension = buffer->extension();
 
-  // The embedder-allocated backing store only exists for the off-heap case.
-  if (backing_store != nullptr) {
+  // Only serialize non-empty backing stores.
+  if (buffer->IsEmpty()) {
+    buffer->SetBackingStoreRefForSerialization(kEmptyBackingStoreRefSentinel);
+  } else {
     uint32_t ref = SerializeBackingStore(backing_store, byte_length);
     buffer->SetBackingStoreRefForSerialization(ref);
 
     // Ensure deterministic output by setting extension to null during
     // serialization.
     buffer->set_extension(nullptr);
-  } else {
-    buffer->SetBackingStoreRefForSerialization(kNullRefSentinel);
   }
 
   SerializeObject();

test/cctest/test-serialize.cc

@@ -1217,7 +1217,7 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobOnOrOffHeapTypedArray) {
       i::Handle<i::JSArrayBuffer> buffer =
           GetBufferFromTypedArray(CompileRun("x"));
       // The resulting buffer should be on-heap.
-      CHECK_NULL(buffer->backing_store());
+      CHECK(buffer->IsEmpty());
       creator.SetDefaultContext(
           context, v8::SerializeInternalFieldsCallback(
                        SerializeInternalFields, reinterpret_cast<void*>(2016)));
@@ -1244,14 +1244,14 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobOnOrOffHeapTypedArray) {
     i::Handle<i::JSArrayBuffer> buffer =
         GetBufferFromTypedArray(CompileRun("x"));
     // The resulting buffer should be on-heap.
-    CHECK_NULL(buffer->backing_store());
+    CHECK(buffer->IsEmpty());
 
     buffer = GetBufferFromTypedArray(CompileRun("y"));
-    CHECK_NULL(buffer->backing_store());
+    CHECK(buffer->IsEmpty());
 
     buffer = GetBufferFromTypedArray(CompileRun("z"));
     // The resulting buffer should be off-heap.
-    CHECK_NOT_NULL(buffer->backing_store());
+    CHECK(!buffer->IsEmpty());
   }
   isolate->Dispose();
   delete[] blob.data;  // We can dispose of the snapshot blob now.