[typedarray] Fix GetBuffer for 0-length off-heap typed arrays.

Fixes a crash that happens when calling postMessage on an empty typed
array.

GetBuffer should only call MaterializeArrayBuffer for on-heap buffers,
but the on-heap check is slightly wrong. This CL moves the on-heap check
logic to the JSTypedArray class so that other parts of the codebase
don't need to worry about how that is determined.

Also add some dchecks to materialize itself. It should only receive
on-heap buffers and should always transform them to off-heap buffers.
There is also no reason for it to be static, so change that here too.

Bug: chromium:797588
Change-Id: Icd88a5b68e424d82c9f1f7889ca42a40a72a1bdc
Reviewed-on: https://chromium-review.googlesource.com/995898
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52388}

src/objects.cc

@@ -19253,14 +19253,13 @@ bool JSArrayBuffer::SetupAllocatingData(Handle<JSArrayBuffer> array_buffer,
   return true;
 }
 
-
 Handle<JSArrayBuffer> JSTypedArray::MaterializeArrayBuffer(
     Handle<JSTypedArray> typed_array) {
+  DCHECK(typed_array->is_on_heap());
 
-  Handle<Map> map(typed_array->map());
   Isolate* isolate = typed_array->GetIsolate();
 
-  DCHECK(IsFixedTypedArrayElementsKind(map->elements_kind()));
+  DCHECK(IsFixedTypedArrayElementsKind(typed_array->GetElementsKind()));
 
   Handle<FixedTypedArrayBase> fixed_typed_array(
       FixedTypedArrayBase::cast(typed_array->elements()));
@@ -19293,17 +19292,14 @@ Handle<JSArrayBuffer> JSTypedArray::MaterializeArrayBuffer(
           static_cast<uint8_t*>(buffer->backing_store()));
 
   typed_array->set_elements(*new_elements);
+  DCHECK(!typed_array->is_on_heap());
 
   return buffer;
 }
 
-
 Handle<JSArrayBuffer> JSTypedArray::GetBuffer() {
-  Handle<JSArrayBuffer> array_buffer(JSArrayBuffer::cast(buffer()),
-                                     GetIsolate());
-  if (array_buffer->was_neutered() ||
-      array_buffer->backing_store() != nullptr ||
-      array_buffer->is_wasm_memory()) {
+  if (!is_on_heap()) {
+    Handle<JSArrayBuffer> array_buffer(JSArrayBuffer::cast(buffer()));
     return array_buffer;
   }
   Handle<JSTypedArray> self(this);

src/objects/js-array-inl.h

@@ -215,6 +215,14 @@ void JSTypedArray::set_length(Object* value, WriteBarrierMode mode) {
   CONDITIONAL_WRITE_BARRIER(GetHeap(), this, kLengthOffset, value, mode);
 }
 
+bool JSTypedArray::is_on_heap() const {
+  DisallowHeapAllocation no_gc;
+  // Checking that buffer()->backing_store() is not nullptr is not sufficient;
+  // it will be nullptr when byte_length is 0 as well.
+  FixedTypedArrayBase* fta(FixedTypedArrayBase::cast(elements()));
+  return fta->base_pointer() == fta;
+}
+
 // static
 MaybeHandle<JSTypedArray> JSTypedArray::Validate(Isolate* isolate,
                                                  Handle<Object> receiver,

src/objects/js-array.h

@@ -293,6 +293,9 @@ class JSTypedArray : public JSArrayBufferView {
 
   Handle<JSArrayBuffer> GetBuffer();
 
+  // Whether the buffer's backing store is on-heap or off-heap.
+  inline bool is_on_heap() const;
+
   static inline MaybeHandle<JSTypedArray> Validate(Isolate* isolate,
                                                    Handle<Object> receiver,
                                                    const char* method_name);

src/snapshot/deserializer.cc

@@ -220,7 +220,7 @@ HeapObject* Deserializer<AllocatorT>::PostProcessNewObject(HeapObject* obj,
       FixedTypedArrayBase* elements =
           FixedTypedArrayBase::cast(typed_array->elements());
       // Must be off-heap layout.
-      DCHECK_NULL(elements->base_pointer());
+      DCHECK(!typed_array->is_on_heap());
 
       void* pointer_with_offset = reinterpret_cast<void*>(
           reinterpret_cast<intptr_t>(elements->external_pointer()) +

src/snapshot/serializer.cc

@@ -398,9 +398,7 @@ void Serializer<AllocatorT>::ObjectSerializer::SerializeJSTypedArray() {
       FixedTypedArrayBase::cast(typed_array->elements());
 
   if (!typed_array->WasNeutered()) {
-    bool off_heap = elements->base_pointer() == nullptr;
-
-    if (off_heap) {
+    if (!typed_array->is_on_heap()) {
       // Explicitly serialize the backing store now.
       JSArrayBuffer* buffer = JSArrayBuffer::cast(typed_array->buffer());
       CHECK(buffer->byte_length()->IsSmi());

test/cctest/test-api.cc

@@ -3940,6 +3940,28 @@ THREADED_TEST(ArrayBuffer_AllocationInformation) {
   CHECK_LE(data + contents.ByteLength(), alloc + contents.AllocationLength());
 }
 
+THREADED_TEST(ArrayBuffer_ExternalizeEmpty) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate, 0);
+  CheckInternalFieldsAreZero(ab);
+  CHECK_EQ(0, static_cast<int>(ab->ByteLength()));
+  CHECK(!ab->IsExternal());
+
+  // Externalize the buffer (taking ownership of the backing store memory).
+  ScopedArrayBufferContents ab_contents(ab->Externalize());
+
+  Local<v8::Uint8Array> u8a = v8::Uint8Array::New(ab, 0, 0);
+  // Calling Buffer() will materialize the ArrayBuffer (transitioning it from
+  // on-heap to off-heap if need be). This should not affect whether it is
+  // marked as is_external or not.
+  USE(u8a->Buffer());
+
+  CHECK(ab->IsExternal());
+}
+
 class ScopedSharedArrayBufferContents {
  public:
   explicit ScopedSharedArrayBufferContents(