From f1982eb49083d0063214ffa0f9f92c0609ae31cc Mon Sep 17 00:00:00 2001 From: yangguo Date: Wed, 24 Jun 2015 07:26:31 -0700 Subject: [PATCH] Serializer: clear next link in weak cells. If we do not clear next links during serialization, the serializer would simply follow those links and serialize arbitrary objects held by weak cells. This breaks the invariant in the code serializer, which crashes if it sees context-dependent objects. R=ulan@chromium.org BUG=chromium:503552 LOG=Y Review URL: https://codereview.chromium.org/1203973002 Cr-Commit-Position: refs/heads/master@{#29255} --- src/heap/heap.cc | 2 +- src/heap/mark-compact.cc | 6 ++---- src/heap/objects-visiting-inl.h | 3 +-- src/objects-inl.h | 8 ++++++++ src/objects.h | 4 ++++ src/snapshot/serialize.cc | 26 ++++++++++++++++++++++++++ test/cctest/test-serialize.cc | 25 +++++++++++++++++++++++++ 7 files changed, 67 insertions(+), 7 deletions(-) diff --git a/src/heap/heap.cc b/src/heap/heap.cc index df1ba3129f..bad4528ff4 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -3126,7 +3126,7 @@ AllocationResult Heap::AllocateWeakCell(HeapObject* value) { } result->set_map_no_write_barrier(weak_cell_map()); WeakCell::cast(result)->initialize(value); - WeakCell::cast(result)->set_next(the_hole_value(), SKIP_WRITE_BARRIER); + WeakCell::cast(result)->clear_next(this); return result; } diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index 2c98b7adc4..257af3739f 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -2647,7 +2647,6 @@ void MarkCompactCollector::AbortWeakCollections() { void MarkCompactCollector::ProcessAndClearWeakCells() { - HeapObject* the_hole = heap()->the_hole_value(); Object* weak_cell_obj = heap()->encountered_weak_cells(); while (weak_cell_obj != Smi::FromInt(0)) { WeakCell* weak_cell = reinterpret_cast(weak_cell_obj); @@ -2682,19 +2681,18 @@ void MarkCompactCollector::ProcessAndClearWeakCells() { RecordSlot(slot, slot, *slot); } weak_cell_obj = weak_cell->next(); - weak_cell->set_next(the_hole, SKIP_WRITE_BARRIER); + weak_cell->clear_next(heap()); } heap()->set_encountered_weak_cells(Smi::FromInt(0)); } void MarkCompactCollector::AbortWeakCells() { - Object* the_hole = heap()->the_hole_value(); Object* weak_cell_obj = heap()->encountered_weak_cells(); while (weak_cell_obj != Smi::FromInt(0)) { WeakCell* weak_cell = reinterpret_cast(weak_cell_obj); weak_cell_obj = weak_cell->next(); - weak_cell->set_next(the_hole, SKIP_WRITE_BARRIER); + weak_cell->clear_next(heap()); } heap()->set_encountered_weak_cells(Smi::FromInt(0)); } diff --git a/src/heap/objects-visiting-inl.h b/src/heap/objects-visiting-inl.h index fa90be5c40..1d5b39dd4e 100644 --- a/src/heap/objects-visiting-inl.h +++ b/src/heap/objects-visiting-inl.h @@ -329,11 +329,10 @@ void StaticMarkingVisitor::VisitWeakCell(Map* map, HeapObject* object) { Heap* heap = map->GetHeap(); WeakCell* weak_cell = reinterpret_cast(object); - Object* the_hole = heap->the_hole_value(); // Enqueue weak cell in linked list of encountered weak collections. // We can ignore weak cells with cleared values because they will always // contain smi zero. - if (weak_cell->next() == the_hole && !weak_cell->cleared()) { + if (weak_cell->next_cleared() && !weak_cell->cleared()) { weak_cell->set_next(heap->encountered_weak_cells(), UPDATE_WEAK_WRITE_BARRIER); heap->set_encountered_weak_cells(weak_cell); diff --git a/src/objects-inl.h b/src/objects-inl.h index c508dd28db..819cc46e3e 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1925,6 +1925,14 @@ void WeakCell::set_next(Object* val, WriteBarrierMode mode) { } +void WeakCell::clear_next(Heap* heap) { + set_next(heap->the_hole_value(), SKIP_WRITE_BARRIER); +} + + +bool WeakCell::next_cleared() { return next()->IsTheHole(); } + + int JSObject::GetHeaderSize() { InstanceType type = map()->instance_type(); // Check for the most common kind of JavaScript object before diff --git a/src/objects.h b/src/objects.h index 785fa987b7..6695f3f0fb 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9610,6 +9610,10 @@ class WeakCell : public HeapObject { DECL_ACCESSORS(next, Object) + inline void clear_next(Heap* heap); + + inline bool next_cleared(); + DECLARE_CAST(WeakCell) DECLARE_PRINTER(WeakCell) diff --git a/src/snapshot/serialize.cc b/src/snapshot/serialize.cc index 7410c3b393..66da378617 100644 --- a/src/snapshot/serialize.cc +++ b/src/snapshot/serialize.cc @@ -1851,6 +1851,28 @@ void Serializer::ObjectSerializer::SerializeExternalString() { } +// Clear and later restore the next link in the weak cell, if the object is one. +class UnlinkWeakCellScope { + public: + explicit UnlinkWeakCellScope(HeapObject* object) : weak_cell_(NULL) { + if (object->IsWeakCell()) { + weak_cell_ = WeakCell::cast(object); + next_ = weak_cell_->next(); + weak_cell_->clear_next(object->GetHeap()); + } + } + + ~UnlinkWeakCellScope() { + if (weak_cell_) weak_cell_->set_next(next_, UPDATE_WEAK_WRITE_BARRIER); + } + + private: + WeakCell* weak_cell_; + Object* next_; + DisallowHeapAllocation no_gc_; +}; + + void Serializer::ObjectSerializer::Serialize() { if (FLAG_trace_serializer) { PrintF(" Encoding heap object: "); @@ -1910,6 +1932,8 @@ void Serializer::ObjectSerializer::Serialize() { return; } + UnlinkWeakCellScope unlink_weak_cell(object_); + object_->IterateBody(map->instance_type(), size, this); OutputRawData(object_->address() + size); } @@ -1934,6 +1958,8 @@ void Serializer::ObjectSerializer::SerializeDeferred() { serializer_->PutBackReference(object_, reference); sink_->PutInt(size >> kPointerSizeLog2, "deferred object size"); + UnlinkWeakCellScope unlink_weak_cell(object_); + object_->IterateBody(map->instance_type(), size, this); OutputRawData(object_->address() + size); } diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc index 6089e43db7..0bae94e219 100644 --- a/test/cctest/test-serialize.cc +++ b/test/cctest/test-serialize.cc @@ -1658,6 +1658,31 @@ TEST(SerializeInternalReference) { } +TEST(Regress503552) { + // Test that the code serializer can deal with weak cells that form a linked + // list during incremental marking. + + CcTest::InitializeVM(); + Isolate* isolate = CcTest::i_isolate(); + + HandleScope scope(isolate); + Handle source = isolate->factory()->NewStringFromAsciiChecked( + "function f() {} function g() {}"); + ScriptData* script_data = NULL; + Handle shared = Compiler::CompileScript( + source, Handle(), 0, 0, v8::ScriptOriginOptions(), + Handle(), Handle(isolate->native_context()), NULL, + &script_data, v8::ScriptCompiler::kProduceCodeCache, NOT_NATIVES_CODE, + false); + delete script_data; + + SimulateIncrementalMarking(isolate->heap()); + + script_data = CodeSerializer::Serialize(isolate, shared, source); + delete script_data; +} + + TEST(SerializationMemoryStats) { FLAG_profile_deserialization = true; FLAG_always_opt = false;