From 0d017282d32ce634f364461aa79ee996108f8b9d Mon Sep 17 00:00:00 2001 From: fedor Date: Wed, 16 Sep 2015 10:27:40 -0700 Subject: [PATCH] [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} --- src/heap/mark-compact.cc | 28 ++++++++++++++++++++++++++++ src/heap/objects-visiting-inl.h | 11 +++-------- src/heap/objects-visiting.cc | 4 +++- src/heap/store-buffer.cc | 11 +++++++++++ src/objects-inl.h | 28 ++++++++++++++++++++++++++++ src/objects.h | 17 ++++++++++++++--- test/cctest/test-api.cc | 22 ++++++++++++++++++++++ 7 files changed, 109 insertions(+), 12 deletions(-) diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index d4c619bd1..ad2ef3601 100644 --- 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(slot - object->address()) == BytecodeArray::kConstantPoolOffset; + } else if (object->IsJSArrayBuffer()) { + int off = static_cast(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 8873f6379..29a5afc32 100644 --- a/src/heap/objects-visiting-inl.h +++ b/src/heap/objects-visiting-inl.h @@ -92,10 +92,8 @@ int StaticNewSpaceVisitor::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 >(heap, object); if (!JSArrayBuffer::cast(object)->is_external()) { heap->array_buffer_tracker()->MarkLive(JSArrayBuffer::cast(object)); } @@ -528,10 +526,7 @@ void StaticMarkingVisitor::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(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 6660d42e8..902a96a64 100644 --- 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(this) ->JSFunctionIterateBody(object_size, v); diff --git a/src/heap/store-buffer.cc b/src/heap/store-buffer.cc index 8a0ee5477..1c673dacc 100644 --- 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 bc866fd2d..22736dc4b 100644 --- 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 +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 9e773c036..4bd947fc1 100644 --- 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 + static inline void JSArrayBufferIterateBody(Heap* heap, HeapObject* obj); + + static inline void JSArrayBufferIterateBody(HeapObject* obj, + ObjectVisitor* v); + class IsExternal : public BitField {}; class IsNeuterable : public BitField {}; class WasNeutered : public BitField {}; diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index df502cb17..4ead519e2 100644 --- 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(i::kHeapObjectTag); + + // Create ArrayBuffer with pointer-that-cannot-be-visited in the backing store + Local 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