[objects] do not visit ArrayBuffer's backing store

ArrayBuffer's backing store is a pointer to external heap, and can't be
treated as a heap object. Doing so will result in crashes, when the
backing store is unaligned.

See: https://github.com/nodejs/node/issues/2791

BUG=chromium:530531
R=mlippautz@chromium.org
LOG=N

Review URL: https://codereview.chromium.org/1327403002

Cr-Commit-Position: refs/heads/master@{#30771}

diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index d4c619bd1953b6450999f6c57be5b71754152505..ad2ef3601caa5a39ff095b1a6c4d35e6a65bddfd 100644 (file)
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -2768,6 +2768,28 @@ void MarkCompactCollector::MigrateObjectMixed(HeapObject* dst, HeapObject* src,
         dst->address() + BytecodeArray::kConstantPoolOffset;
     RecordMigratedSlot(Memory::Object_at(constant_pool_slot),
                        constant_pool_slot);
+  } else if (src->IsJSArrayBuffer()) {
+    heap()->MoveBlock(dst->address(), src->address(), size);
+
+    // Visit inherited JSObject properties and byte length of ArrayBuffer
+    Address regular_slot =
+        dst->address() + JSArrayBuffer::BodyDescriptor::kStartOffset;
+    Address regular_slots_end =
+        dst->address() + JSArrayBuffer::kByteLengthOffset + kPointerSize;
+    while (regular_slot < regular_slots_end) {
+      RecordMigratedSlot(Memory::Object_at(regular_slot), regular_slot);
+      regular_slot += kPointerSize;
+    }
+
+    // Skip backing store and visit just internal fields
+    Address internal_field_slot = dst->address() + JSArrayBuffer::kSize;
+    Address internal_fields_end =
+        dst->address() + JSArrayBuffer::kSizeWithInternalFields;
+    while (internal_field_slot < internal_fields_end) {
+      RecordMigratedSlot(Memory::Object_at(internal_field_slot),
+                         internal_field_slot);
+      internal_field_slot += kPointerSize;
+    }
   } else if (FLAG_unbox_double_fields) {
     Address dst_addr = dst->address();
     Address src_addr = src->address();
@@ -3263,6 +3285,12 @@ bool MarkCompactCollector::IsSlotInLiveObject(Address slot) {
         } else if (object->IsBytecodeArray()) {
           return static_cast<int>(slot - object->address()) ==
                  BytecodeArray::kConstantPoolOffset;
+        } else if (object->IsJSArrayBuffer()) {
+          int off = static_cast<int>(slot - object->address());
+          return (off >= JSArrayBuffer::BodyDescriptor::kStartOffset &&
+                  off <= JSArrayBuffer::kByteLengthOffset) ||
+                 (off >= JSArrayBuffer::kSize &&
+                  off < JSArrayBuffer::kSizeWithInternalFields);
         } else if (FLAG_unbox_double_fields) {
           // Filter out slots that happen to point to unboxed double fields.
           LayoutDescriptorHelper helper(object->map());

diff --git a/src/heap/objects-visiting-inl.h b/src/heap/objects-visiting-inl.h
index 8873f637929853044ec4e531a5a428e6cfb387eb..29a5afc320a370d4e9ee2802865353eb8054fe80 100644 (file)
--- a/src/heap/objects-visiting-inl.h
+++ b/src/heap/objects-visiting-inl.h
@@ -92,10 +92,8 @@ int StaticNewSpaceVisitor<StaticVisitor>::VisitJSArrayBuffer(
     Map* map, HeapObject* object) {
   Heap* heap = map->GetHeap();
 
-  VisitPointers(
-      heap, object,
-      HeapObject::RawField(object, JSArrayBuffer::BodyDescriptor::kStartOffset),
-      HeapObject::RawField(object, JSArrayBuffer::kSizeWithInternalFields));
+  JSArrayBuffer::JSArrayBufferIterateBody<
+      StaticNewSpaceVisitor<StaticVisitor> >(heap, object);
   if (!JSArrayBuffer::cast(object)->is_external()) {
     heap->array_buffer_tracker()->MarkLive(JSArrayBuffer::cast(object));
   }
@@ -528,10 +526,7 @@ void StaticMarkingVisitor<StaticVisitor>::VisitJSArrayBuffer(
     Map* map, HeapObject* object) {
   Heap* heap = map->GetHeap();
 
-  StaticVisitor::VisitPointers(
-      heap, object,
-      HeapObject::RawField(object, JSArrayBuffer::BodyDescriptor::kStartOffset),
-      HeapObject::RawField(object, JSArrayBuffer::kSizeWithInternalFields));
+  JSArrayBuffer::JSArrayBufferIterateBody<StaticVisitor>(heap, object);
   if (!JSArrayBuffer::cast(object)->is_external() &&
       !heap->InNewSpace(object)) {
     heap->array_buffer_tracker()->MarkLive(JSArrayBuffer::cast(object));

diff --git a/src/heap/objects-visiting.cc b/src/heap/objects-visiting.cc
index 6660d42e810105d627a77bc8c91c7d36f227bed8..902a96a644cb054d99d09c6fc4df1adccc8d1744 100644 (file)
--- a/src/heap/objects-visiting.cc
+++ b/src/heap/objects-visiting.cc
@@ -220,7 +220,6 @@ void HeapObject::IterateBody(InstanceType type, int object_size,
     case JS_VALUE_TYPE:
     case JS_DATE_TYPE:
     case JS_ARRAY_TYPE:
-    case JS_ARRAY_BUFFER_TYPE:
     case JS_TYPED_ARRAY_TYPE:
     case JS_DATA_VIEW_TYPE:
     case JS_SET_TYPE:
@@ -237,6 +236,9 @@ void HeapObject::IterateBody(InstanceType type, int object_size,
     case JS_MESSAGE_OBJECT_TYPE:
       JSObject::BodyDescriptor::IterateBody(this, object_size, v);
       break;
+    case JS_ARRAY_BUFFER_TYPE:
+      JSArrayBuffer::JSArrayBufferIterateBody(this, v);
+      break;
     case JS_FUNCTION_TYPE:
       reinterpret_cast<JSFunction*>(this)
           ->JSFunctionIterateBody(object_size, v);

diff --git a/src/heap/store-buffer.cc b/src/heap/store-buffer.cc
index 8a0ee5477918a5683cc83c3dc5eab4be660acbfc..1c673dacc5d735be404fe4e0d196e2f10fad0327 100644 (file)
--- a/src/heap/store-buffer.cc
+++ b/src/heap/store-buffer.cc
@@ -499,6 +499,17 @@ void StoreBuffer::IteratePointersToNewSpace(ObjectSlotCallback slot_callback) {
                         obj_address + BytecodeArray::kConstantPoolOffset,
                         obj_address + BytecodeArray::kHeaderSize,
                         slot_callback);
+                  } else if (heap_object->IsJSArrayBuffer()) {
+                    FindPointersToNewSpaceInRegion(
+                        obj_address +
+                            JSArrayBuffer::BodyDescriptor::kStartOffset,
+                        obj_address + JSArrayBuffer::kByteLengthOffset +
+                            kPointerSize,
+                        slot_callback);
+                    FindPointersToNewSpaceInRegion(
+                        obj_address + JSArrayBuffer::kSize,
+                        obj_address + JSArrayBuffer::kSizeWithInternalFields,
+                        slot_callback);
                   } else if (FLAG_unbox_double_fields) {
                     LayoutDescriptorHelper helper(heap_object->map());
                     DCHECK(!helper.all_fields_tagged());

diff --git a/src/objects-inl.h b/src/objects-inl.h
index bc866fd2d94b6ae1422a123d64d6512202c464d9..22736dc4b7f03567bcbc094fe075f7949498f90a 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1505,6 +1505,8 @@ HeapObjectContents HeapObject::ContentType() {
   } else if (type >= FIRST_FIXED_TYPED_ARRAY_TYPE &&
              type <= LAST_FIXED_TYPED_ARRAY_TYPE) {
     return HeapObjectContents::kMixedValues;
+  } else if (type == JS_ARRAY_BUFFER_TYPE) {
+    return HeapObjectContents::kMixedValues;
   } else if (type <= LAST_DATA_TYPE) {
     // TODO(jochen): Why do we claim that Code and Map contain only raw values?
     return HeapObjectContents::kRawValues;
@@ -6596,6 +6598,32 @@ void JSArrayBuffer::set_is_shared(bool value) {
 }
 
 
+// static
+template <typename StaticVisitor>
+void JSArrayBuffer::JSArrayBufferIterateBody(Heap* heap, HeapObject* obj) {
+  StaticVisitor::VisitPointers(
+      heap, obj,
+      HeapObject::RawField(obj, JSArrayBuffer::BodyDescriptor::kStartOffset),
+      HeapObject::RawField(obj,
+                           JSArrayBuffer::kByteLengthOffset + kPointerSize));
+  StaticVisitor::VisitPointers(
+      heap, obj, HeapObject::RawField(obj, JSArrayBuffer::kSize),
+      HeapObject::RawField(obj, JSArrayBuffer::kSizeWithInternalFields));
+}
+
+
+void JSArrayBuffer::JSArrayBufferIterateBody(HeapObject* obj,
+                                             ObjectVisitor* v) {
+  v->VisitPointers(
+      HeapObject::RawField(obj, JSArrayBuffer::BodyDescriptor::kStartOffset),
+      HeapObject::RawField(obj,
+                           JSArrayBuffer::kByteLengthOffset + kPointerSize));
+  v->VisitPointers(
+      HeapObject::RawField(obj, JSArrayBuffer::kSize),
+      HeapObject::RawField(obj, JSArrayBuffer::kSizeWithInternalFields));
+}
+
+
 Object* JSArrayBufferView::byte_offset() const {
   if (WasNeutered()) return Smi::FromInt(0);
   return Object::cast(READ_FIELD(this, kByteOffsetOffset));

diff --git a/src/objects.h b/src/objects.h
index 9e773c036265bffde6d27c446160bf98ebfc3413..4bd947fc182b6d6489cd1b1837a772491f7d15f7 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -9670,9 +9670,14 @@ class JSArrayBuffer: public JSObject {
   DECLARE_PRINTER(JSArrayBuffer)
   DECLARE_VERIFIER(JSArrayBuffer)
 
-  static const int kBackingStoreOffset = JSObject::kHeaderSize;
-  static const int kByteLengthOffset = kBackingStoreOffset + kPointerSize;
-  static const int kBitFieldSlot = kByteLengthOffset + kPointerSize;
+  static const int kByteLengthOffset = JSObject::kHeaderSize;
+
+  // NOTE: GC will visit objects fields:
+  // 1. From JSObject::BodyDescriptor::kStartOffset to kByteLengthOffset +
+  //    kPointerSize
+  // 2. From start of the internal fields and up to the end of them
+  static const int kBackingStoreOffset = kByteLengthOffset + kPointerSize;
+  static const int kBitFieldSlot = kBackingStoreOffset + kPointerSize;
 #if V8_TARGET_LITTLE_ENDIAN || !V8_HOST_ARCH_64_BIT
   static const int kBitFieldOffset = kBitFieldSlot;
 #else
@@ -9683,6 +9688,12 @@ class JSArrayBuffer: public JSObject {
   static const int kSizeWithInternalFields =
       kSize + v8::ArrayBuffer::kInternalFieldCount * kPointerSize;
 
+  template <typename StaticVisitor>
+  static inline void JSArrayBufferIterateBody(Heap* heap, HeapObject* obj);
+
+  static inline void JSArrayBufferIterateBody(HeapObject* obj,
+                                              ObjectVisitor* v);
+
   class IsExternal : public BitField<bool, 1, 1> {};
   class IsNeuterable : public BitField<bool, 2, 1> {};
   class WasNeutered : public BitField<bool, 3, 1> {};

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index df502cb177e6d8ee103668a80c57c6c689f01bd3..4ead519e26910689178842347a76fcc5869776d2 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -14223,6 +14223,28 @@ THREADED_TEST(DataView) {
 }
 
 
+THREADED_TEST(SkipArrayBufferBackingStoreDuringGC) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  // Make sure the pointer looks like a heap object
+  uint8_t* store_ptr = reinterpret_cast<uint8_t*>(i::kHeapObjectTag);
+
+  // Create ArrayBuffer with pointer-that-cannot-be-visited in the backing store
+  Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate, store_ptr, 8);
+
+  // Should not crash
+  CcTest::heap()->CollectGarbage(i::NEW_SPACE);  // in survivor space now
+  CcTest::heap()->CollectGarbage(i::NEW_SPACE);  // in old gen now
+  CcTest::heap()->CollectAllGarbage();
+  CcTest::heap()->CollectAllGarbage();
+
+  // Should not move the pointer
+  CHECK_EQ(ab->GetContents().Data(), store_ptr);
+}
+
+
 THREADED_TEST(SharedUint8Array) {
   i::FLAG_harmony_sharedarraybuffer = true;
   TypedArrayTestHelper<uint8_t, v8::Uint8Array, i::FixedUint8Array,