From 972a0c851586ddde30f948d1ea8510f74e141f7c Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 16 Sep 2015 10:57:30 -0700 Subject: [PATCH] deps: backport 0d01728 from v8's upstream Original commit message: [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} Ref: https://github.com/nodejs/node/issues/2791 Ref: https://github.com/nodejs/node/pull/2912 PR-URL: https://github.com/nodejs/node/pull/3351 Reviewed-By: indutny - Fedor Indutny Reviewed-By: bnoordhuis - Ben Noordhuis --- deps/v8/src/heap/mark-compact.cc | 28 +++++++++++++++++++++++++ deps/v8/src/heap/objects-visiting-inl.h | 11 +++------- deps/v8/src/heap/objects-visiting.cc | 4 +++- deps/v8/src/heap/store-buffer.cc | 11 ++++++++++ deps/v8/src/objects-inl.h | 28 +++++++++++++++++++++++++ deps/v8/src/objects.h | 17 ++++++++++++--- deps/v8/test/cctest/test-api.cc | 22 +++++++++++++++++++ 7 files changed, 109 insertions(+), 12 deletions(-) diff --git a/deps/v8/src/heap/mark-compact.cc b/deps/v8/src/heap/mark-compact.cc index 32c3a99af2ffd4..ee5b4a0ad46b1c 100644 --- a/deps/v8/src/heap/mark-compact.cc +++ b/deps/v8/src/heap/mark-compact.cc @@ -2755,6 +2755,28 @@ void MarkCompactCollector::MigrateObjectMixed(HeapObject* dst, HeapObject* src, Address base_pointer_slot = dst->address() + FixedTypedArrayBase::kBasePointerOffset; RecordMigratedSlot(Memory::Object_at(base_pointer_slot), base_pointer_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(); @@ -3178,6 +3200,12 @@ bool MarkCompactCollector::IsSlotInLiveObject(Address slot) { if (object->IsFixedTypedArrayBase()) { return static_cast(slot - object->address()) == FixedTypedArrayBase::kBasePointerOffset; + } 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/deps/v8/src/heap/objects-visiting-inl.h b/deps/v8/src/heap/objects-visiting-inl.h index 21b770d8c5a174..9a2e2ee147538b 100644 --- a/deps/v8/src/heap/objects-visiting-inl.h +++ b/deps/v8/src/heap/objects-visiting-inl.h @@ -91,10 +91,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->RegisterLiveArrayBuffer(true, JSArrayBuffer::cast(object)->backing_store()); @@ -517,10 +515,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->RegisterLiveArrayBuffer(false, JSArrayBuffer::cast(object)->backing_store()); diff --git a/deps/v8/src/heap/objects-visiting.cc b/deps/v8/src/heap/objects-visiting.cc index d29b99eb0572f5..602d37b805a7f0 100644 --- a/deps/v8/src/heap/objects-visiting.cc +++ b/deps/v8/src/heap/objects-visiting.cc @@ -219,7 +219,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: @@ -235,6 +234,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/deps/v8/src/heap/store-buffer.cc b/deps/v8/src/heap/store-buffer.cc index 95fb83c3c8c919..900b8fe1b0997c 100644 --- a/deps/v8/src/heap/store-buffer.cc +++ b/deps/v8/src/heap/store-buffer.cc @@ -492,6 +492,17 @@ void StoreBuffer::IteratePointersToNewSpace(ObjectSlotCallback slot_callback) { obj_address + FixedTypedArrayBase::kBasePointerOffset, obj_address + FixedTypedArrayBase::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/deps/v8/src/objects-inl.h b/deps/v8/src/objects-inl.h index b3713b644c5dfe..b7aba56f5a4bf5 100644 --- a/deps/v8/src/objects-inl.h +++ b/deps/v8/src/objects-inl.h @@ -1483,6 +1483,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; @@ -6516,6 +6518,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/deps/v8/src/objects.h b/deps/v8/src/objects.h index 3a9e7e0c767db3..9e6a068c4ceecc 100644 --- a/deps/v8/src/objects.h +++ b/deps/v8/src/objects.h @@ -9451,9 +9451,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 @@ -9464,6 +9469,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/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 3d5915e5a0c4b5..d465f2f87a96ee 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -14220,6 +14220,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