Skip to content

Commit

Permalink
deps: backport 0d01728 from v8's upstream
Browse files Browse the repository at this point in the history
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: nodejs#2791

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

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

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

Fix: nodejs#2791
PR-URL: nodejs#2912
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
indutny authored and targos committed Oct 6, 2015
1 parent dac8828 commit bdfe354
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 12 deletions.
28 changes: 28 additions & 0 deletions deps/v8/src/heap/mark-compact.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -3178,6 +3200,12 @@ bool MarkCompactCollector::IsSlotInLiveObject(Address slot) {
if (object->IsFixedTypedArrayBase()) {
return static_cast<int>(slot - object->address()) ==
FixedTypedArrayBase::kBasePointerOffset;
} 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());
Expand Down
11 changes: 3 additions & 8 deletions deps/v8/src/heap/objects-visiting-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,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->RegisterLiveArrayBuffer(true,
JSArrayBuffer::cast(object)->backing_store());
Expand Down Expand Up @@ -517,10 +515,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->RegisterLiveArrayBuffer(false,
JSArrayBuffer::cast(object)->backing_store());
Expand Down
4 changes: 3 additions & 1 deletion deps/v8/src/heap/objects-visiting.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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<JSFunction*>(this)
->JSFunctionIterateBody(object_size, v);
Expand Down
11 changes: 11 additions & 0 deletions deps/v8/src/heap/store-buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
28 changes: 28 additions & 0 deletions deps/v8/src/objects-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -6516,6 +6518,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));
Expand Down
17 changes: 14 additions & 3 deletions deps/v8/src/objects.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -9464,6 +9469,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> {};
Expand Down
22 changes: 22 additions & 0 deletions deps/v8/test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<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,
Expand Down

0 comments on commit bdfe354

Please sign in to comment.