Skip to content

Commit dcfa6aa

Browse files
indutnyofrobots
authored andcommitted
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: nodejs#2791 BUG=chromium:530531 [email protected] LOG=N Review URL: https://codereview.chromium.org/1327403002 Cr-Commit-Position: refs/heads/master@{nodejs#30771} Ref: nodejs#2791 Ref: nodejs#2912 PR-URL: nodejs#3351 Reviewed-By: indutny - Fedor Indutny <[email protected]> Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
1 parent 6456ffa commit dcfa6aa

7 files changed

+109
-12
lines changed

deps/v8/src/heap/mark-compact.cc

+28
Original file line numberDiff line numberDiff line change
@@ -2755,6 +2755,28 @@ void MarkCompactCollector::MigrateObjectMixed(HeapObject* dst, HeapObject* src,
27552755
Address base_pointer_slot =
27562756
dst->address() + FixedTypedArrayBase::kBasePointerOffset;
27572757
RecordMigratedSlot(Memory::Object_at(base_pointer_slot), base_pointer_slot);
2758+
} else if (src->IsJSArrayBuffer()) {
2759+
heap()->MoveBlock(dst->address(), src->address(), size);
2760+
2761+
// Visit inherited JSObject properties and byte length of ArrayBuffer
2762+
Address regular_slot =
2763+
dst->address() + JSArrayBuffer::BodyDescriptor::kStartOffset;
2764+
Address regular_slots_end =
2765+
dst->address() + JSArrayBuffer::kByteLengthOffset + kPointerSize;
2766+
while (regular_slot < regular_slots_end) {
2767+
RecordMigratedSlot(Memory::Object_at(regular_slot), regular_slot);
2768+
regular_slot += kPointerSize;
2769+
}
2770+
2771+
// Skip backing store and visit just internal fields
2772+
Address internal_field_slot = dst->address() + JSArrayBuffer::kSize;
2773+
Address internal_fields_end =
2774+
dst->address() + JSArrayBuffer::kSizeWithInternalFields;
2775+
while (internal_field_slot < internal_fields_end) {
2776+
RecordMigratedSlot(Memory::Object_at(internal_field_slot),
2777+
internal_field_slot);
2778+
internal_field_slot += kPointerSize;
2779+
}
27582780
} else if (FLAG_unbox_double_fields) {
27592781
Address dst_addr = dst->address();
27602782
Address src_addr = src->address();
@@ -3178,6 +3200,12 @@ bool MarkCompactCollector::IsSlotInLiveObject(Address slot) {
31783200
if (object->IsFixedTypedArrayBase()) {
31793201
return static_cast<int>(slot - object->address()) ==
31803202
FixedTypedArrayBase::kBasePointerOffset;
3203+
} else if (object->IsJSArrayBuffer()) {
3204+
int off = static_cast<int>(slot - object->address());
3205+
return (off >= JSArrayBuffer::BodyDescriptor::kStartOffset &&
3206+
off <= JSArrayBuffer::kByteLengthOffset) ||
3207+
(off >= JSArrayBuffer::kSize &&
3208+
off < JSArrayBuffer::kSizeWithInternalFields);
31813209
} else if (FLAG_unbox_double_fields) {
31823210
// Filter out slots that happen to point to unboxed double fields.
31833211
LayoutDescriptorHelper helper(object->map());

deps/v8/src/heap/objects-visiting-inl.h

+3-8
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,8 @@ int StaticNewSpaceVisitor<StaticVisitor>::VisitJSArrayBuffer(
9191
Map* map, HeapObject* object) {
9292
Heap* heap = map->GetHeap();
9393

94-
VisitPointers(
95-
heap, object,
96-
HeapObject::RawField(object, JSArrayBuffer::BodyDescriptor::kStartOffset),
97-
HeapObject::RawField(object, JSArrayBuffer::kSizeWithInternalFields));
94+
JSArrayBuffer::JSArrayBufferIterateBody<
95+
StaticNewSpaceVisitor<StaticVisitor> >(heap, object);
9896
if (!JSArrayBuffer::cast(object)->is_external()) {
9997
heap->RegisterLiveArrayBuffer(true,
10098
JSArrayBuffer::cast(object)->backing_store());
@@ -517,10 +515,7 @@ void StaticMarkingVisitor<StaticVisitor>::VisitJSArrayBuffer(
517515
Map* map, HeapObject* object) {
518516
Heap* heap = map->GetHeap();
519517

520-
StaticVisitor::VisitPointers(
521-
heap, object,
522-
HeapObject::RawField(object, JSArrayBuffer::BodyDescriptor::kStartOffset),
523-
HeapObject::RawField(object, JSArrayBuffer::kSizeWithInternalFields));
518+
JSArrayBuffer::JSArrayBufferIterateBody<StaticVisitor>(heap, object);
524519
if (!JSArrayBuffer::cast(object)->is_external()) {
525520
heap->RegisterLiveArrayBuffer(false,
526521
JSArrayBuffer::cast(object)->backing_store());

deps/v8/src/heap/objects-visiting.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ void HeapObject::IterateBody(InstanceType type, int object_size,
219219
case JS_VALUE_TYPE:
220220
case JS_DATE_TYPE:
221221
case JS_ARRAY_TYPE:
222-
case JS_ARRAY_BUFFER_TYPE:
223222
case JS_TYPED_ARRAY_TYPE:
224223
case JS_DATA_VIEW_TYPE:
225224
case JS_SET_TYPE:
@@ -235,6 +234,9 @@ void HeapObject::IterateBody(InstanceType type, int object_size,
235234
case JS_MESSAGE_OBJECT_TYPE:
236235
JSObject::BodyDescriptor::IterateBody(this, object_size, v);
237236
break;
237+
case JS_ARRAY_BUFFER_TYPE:
238+
JSArrayBuffer::JSArrayBufferIterateBody(this, v);
239+
break;
238240
case JS_FUNCTION_TYPE:
239241
reinterpret_cast<JSFunction*>(this)
240242
->JSFunctionIterateBody(object_size, v);

deps/v8/src/heap/store-buffer.cc

+11
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,17 @@ void StoreBuffer::IteratePointersToNewSpace(ObjectSlotCallback slot_callback) {
492492
obj_address + FixedTypedArrayBase::kBasePointerOffset,
493493
obj_address + FixedTypedArrayBase::kHeaderSize,
494494
slot_callback);
495+
} else if (heap_object->IsJSArrayBuffer()) {
496+
FindPointersToNewSpaceInRegion(
497+
obj_address +
498+
JSArrayBuffer::BodyDescriptor::kStartOffset,
499+
obj_address + JSArrayBuffer::kByteLengthOffset +
500+
kPointerSize,
501+
slot_callback);
502+
FindPointersToNewSpaceInRegion(
503+
obj_address + JSArrayBuffer::kSize,
504+
obj_address + JSArrayBuffer::kSizeWithInternalFields,
505+
slot_callback);
495506
} else if (FLAG_unbox_double_fields) {
496507
LayoutDescriptorHelper helper(heap_object->map());
497508
DCHECK(!helper.all_fields_tagged());

deps/v8/src/objects-inl.h

+28
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,8 @@ HeapObjectContents HeapObject::ContentType() {
14831483
} else if (type >= FIRST_FIXED_TYPED_ARRAY_TYPE &&
14841484
type <= LAST_FIXED_TYPED_ARRAY_TYPE) {
14851485
return HeapObjectContents::kMixedValues;
1486+
} else if (type == JS_ARRAY_BUFFER_TYPE) {
1487+
return HeapObjectContents::kMixedValues;
14861488
} else if (type <= LAST_DATA_TYPE) {
14871489
// TODO(jochen): Why do we claim that Code and Map contain only raw values?
14881490
return HeapObjectContents::kRawValues;
@@ -6516,6 +6518,32 @@ void JSArrayBuffer::set_is_shared(bool value) {
65166518
}
65176519

65186520

6521+
// static
6522+
template <typename StaticVisitor>
6523+
void JSArrayBuffer::JSArrayBufferIterateBody(Heap* heap, HeapObject* obj) {
6524+
StaticVisitor::VisitPointers(
6525+
heap, obj,
6526+
HeapObject::RawField(obj, JSArrayBuffer::BodyDescriptor::kStartOffset),
6527+
HeapObject::RawField(obj,
6528+
JSArrayBuffer::kByteLengthOffset + kPointerSize));
6529+
StaticVisitor::VisitPointers(
6530+
heap, obj, HeapObject::RawField(obj, JSArrayBuffer::kSize),
6531+
HeapObject::RawField(obj, JSArrayBuffer::kSizeWithInternalFields));
6532+
}
6533+
6534+
6535+
void JSArrayBuffer::JSArrayBufferIterateBody(HeapObject* obj,
6536+
ObjectVisitor* v) {
6537+
v->VisitPointers(
6538+
HeapObject::RawField(obj, JSArrayBuffer::BodyDescriptor::kStartOffset),
6539+
HeapObject::RawField(obj,
6540+
JSArrayBuffer::kByteLengthOffset + kPointerSize));
6541+
v->VisitPointers(
6542+
HeapObject::RawField(obj, JSArrayBuffer::kSize),
6543+
HeapObject::RawField(obj, JSArrayBuffer::kSizeWithInternalFields));
6544+
}
6545+
6546+
65196547
Object* JSArrayBufferView::byte_offset() const {
65206548
if (WasNeutered()) return Smi::FromInt(0);
65216549
return Object::cast(READ_FIELD(this, kByteOffsetOffset));

deps/v8/src/objects.h

+14-3
Original file line numberDiff line numberDiff line change
@@ -9451,9 +9451,14 @@ class JSArrayBuffer: public JSObject {
94519451
DECLARE_PRINTER(JSArrayBuffer)
94529452
DECLARE_VERIFIER(JSArrayBuffer)
94539453

9454-
static const int kBackingStoreOffset = JSObject::kHeaderSize;
9455-
static const int kByteLengthOffset = kBackingStoreOffset + kPointerSize;
9456-
static const int kBitFieldSlot = kByteLengthOffset + kPointerSize;
9454+
static const int kByteLengthOffset = JSObject::kHeaderSize;
9455+
9456+
// NOTE: GC will visit objects fields:
9457+
// 1. From JSObject::BodyDescriptor::kStartOffset to kByteLengthOffset +
9458+
// kPointerSize
9459+
// 2. From start of the internal fields and up to the end of them
9460+
static const int kBackingStoreOffset = kByteLengthOffset + kPointerSize;
9461+
static const int kBitFieldSlot = kBackingStoreOffset + kPointerSize;
94579462
#if V8_TARGET_LITTLE_ENDIAN || !V8_HOST_ARCH_64_BIT
94589463
static const int kBitFieldOffset = kBitFieldSlot;
94599464
#else
@@ -9464,6 +9469,12 @@ class JSArrayBuffer: public JSObject {
94649469
static const int kSizeWithInternalFields =
94659470
kSize + v8::ArrayBuffer::kInternalFieldCount * kPointerSize;
94669471

9472+
template <typename StaticVisitor>
9473+
static inline void JSArrayBufferIterateBody(Heap* heap, HeapObject* obj);
9474+
9475+
static inline void JSArrayBufferIterateBody(HeapObject* obj,
9476+
ObjectVisitor* v);
9477+
94679478
class IsExternal : public BitField<bool, 1, 1> {};
94689479
class IsNeuterable : public BitField<bool, 2, 1> {};
94699480
class WasNeutered : public BitField<bool, 3, 1> {};

deps/v8/test/cctest/test-api.cc

+22
Original file line numberDiff line numberDiff line change
@@ -14220,6 +14220,28 @@ THREADED_TEST(DataView) {
1422014220
}
1422114221

1422214222

14223+
THREADED_TEST(SkipArrayBufferBackingStoreDuringGC) {
14224+
LocalContext env;
14225+
v8::Isolate* isolate = env->GetIsolate();
14226+
v8::HandleScope handle_scope(isolate);
14227+
14228+
// Make sure the pointer looks like a heap object
14229+
uint8_t* store_ptr = reinterpret_cast<uint8_t*>(i::kHeapObjectTag);
14230+
14231+
// Create ArrayBuffer with pointer-that-cannot-be-visited in the backing store
14232+
Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate, store_ptr, 8);
14233+
14234+
// Should not crash
14235+
CcTest::heap()->CollectGarbage(i::NEW_SPACE); // in survivor space now
14236+
CcTest::heap()->CollectGarbage(i::NEW_SPACE); // in old gen now
14237+
CcTest::heap()->CollectAllGarbage();
14238+
CcTest::heap()->CollectAllGarbage();
14239+
14240+
// Should not move the pointer
14241+
CHECK_EQ(ab->GetContents().Data(), store_ptr);
14242+
}
14243+
14244+
1422314245
THREADED_TEST(SharedUint8Array) {
1422414246
i::FLAG_harmony_sharedarraybuffer = true;
1422514247
TypedArrayTestHelper<uint8_t, v8::Uint8Array, i::FixedUint8Array,

0 commit comments

Comments
 (0)