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

@@ -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;
 }
 

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<WeakCell*>(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<WeakCell*>(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));
 }

src/heap/objects-visiting-inl.h

@@ -329,11 +329,10 @@ void StaticMarkingVisitor<StaticVisitor>::VisitWeakCell(Map* map,
                                                         HeapObject* object) {
   Heap* heap = map->GetHeap();
   WeakCell* weak_cell = reinterpret_cast<WeakCell*>(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);

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

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)

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);
 }

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<String> source = isolate->factory()->NewStringFromAsciiChecked(
+      "function f() {} function g() {}");
+  ScriptData* script_data = NULL;
+  Handle<SharedFunctionInfo> shared = Compiler::CompileScript(
+      source, Handle<String>(), 0, 0, v8::ScriptOriginOptions(),
+      Handle<Object>(), Handle<Context>(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;