Clear SMI and non-evacuation candidate entries when filtering the slots buffer.

BUG=

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

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

src/heap/mark-compact.cc

@@ -4524,9 +4524,18 @@ void SlotsBuffer::RemoveInvalidSlots(Heap* heap, SlotsBuffer* buffer) {
       ObjectSlot slot = slots[slot_idx];
       if (!IsTypedSlot(slot)) {
         Object* object = *slot;
-        if ((object->IsHeapObject() && heap->InNewSpace(object)) ||
+        // Slots are invalid when they currently:
+        // - do not point to a heap object (SMI)
+        // - point to a heap object in new space
+        // - are not within a live heap object on a valid pointer slot
+        // - point to a heap object not on an evacuation candidate
+        if (!object->IsHeapObject() || heap->InNewSpace(object) ||
             !heap->mark_compact_collector()->IsSlotInLiveObject(
-                reinterpret_cast<Address>(slot))) {
+                reinterpret_cast<Address>(slot)) ||
+            !Page::FromAddress(reinterpret_cast<Address>(object))
+                 ->IsEvacuationCandidate()) {
+          // TODO(hpayer): Instead of replacing slots with kRemovedEntry we
+          // could shrink the slots buffer in-place.
           slots[slot_idx] = kRemovedEntry;
         }
       } else {
@@ -4558,6 +4567,8 @@ void SlotsBuffer::RemoveObjectSlots(Heap* heap, SlotsBuffer* buffer,
       if (!IsTypedSlot(slot)) {
         Address slot_address = reinterpret_cast<Address>(slot);
         if (slot_address >= start_slot && slot_address < end_slot) {
+          // TODO(hpayer): Instead of replacing slots with kRemovedEntry we
+          // could shrink the slots buffer in-place.
           slots[slot_idx] = kRemovedEntry;
           if (is_typed_slot) {
             slots[slot_idx - 1] = kRemovedEntry;

test/cctest/test-heap.cc

@@ -6502,6 +6502,81 @@ TEST(SlotsBufferObjectSlotsRemoval) {
 }
 
 
+TEST(FilterInvalidSlotsBufferEntries) {
+  FLAG_manual_evacuation_candidates_selection = true;
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+  Factory* factory = isolate->factory();
+  SlotsBuffer* buffer = new SlotsBuffer(NULL);
+
+  // Set up a fake black object that will contain a recorded SMI, a recorded
+  // pointer to a new space object, and a recorded pointer to a non-evacuation
+  // candidate object. These object should be filtered out. Additionally,
+  // we point to an evacuation candidate object which should not be filtered
+  // out.
+
+  // Create fake object and mark it black.
+  Handle<FixedArray> fake_object = factory->NewFixedArray(23, TENURED);
+  MarkBit mark_bit = Marking::MarkBitFrom(*fake_object);
+  Marking::MarkBlack(mark_bit);
+
+  // Write a SMI into field one and record its address;
+  Object** field_smi = fake_object->RawFieldOfElementAt(0);
+  *field_smi = Smi::FromInt(100);
+  buffer->Add(field_smi);
+
+  // Write a new space reference into field 2 and record its address;
+  Handle<FixedArray> new_space_object = factory->NewFixedArray(23);
+  mark_bit = Marking::MarkBitFrom(*new_space_object);
+  Marking::MarkBlack(mark_bit);
+  Object** field_new_space = fake_object->RawFieldOfElementAt(1);
+  *field_new_space = *new_space_object;
+  buffer->Add(field_new_space);
+
+  // Write an old space reference into field 3 which points to an object not on
+  // an evacuation candidate.
+  Handle<FixedArray> old_space_object_non_evacuation =
+      factory->NewFixedArray(23, TENURED);
+  mark_bit = Marking::MarkBitFrom(*old_space_object_non_evacuation);
+  Marking::MarkBlack(mark_bit);
+  Object** field_old_space_object_non_evacuation =
+      fake_object->RawFieldOfElementAt(2);
+  *field_old_space_object_non_evacuation = *old_space_object_non_evacuation;
+  buffer->Add(field_old_space_object_non_evacuation);
+
+  // Write an old space reference into field 4 which points to an object on an
+  // evacuation candidate.
+  SimulateFullSpace(heap->old_space());
+  Handle<FixedArray> valid_object =
+      isolate->factory()->NewFixedArray(23, TENURED);
+  Page* page = Page::FromAddress(valid_object->address());
+  page->SetFlag(MemoryChunk::EVACUATION_CANDIDATE);
+  Object** valid_field = fake_object->RawFieldOfElementAt(3);
+  *valid_field = *valid_object;
+  buffer->Add(valid_field);
+
+  SlotsBuffer::RemoveInvalidSlots(heap, buffer);
+  Object** kRemovedEntry = HeapObject::RawField(heap->empty_fixed_array(),
+                                                FixedArrayBase::kLengthOffset);
+  CHECK_EQ(buffer->Get(0), kRemovedEntry);
+  CHECK_EQ(buffer->Get(1), kRemovedEntry);
+  CHECK_EQ(buffer->Get(2), kRemovedEntry);
+  CHECK_EQ(buffer->Get(3), valid_field);
+
+  // Clean-up to make verify heap happy.
+  mark_bit = Marking::MarkBitFrom(*fake_object);
+  Marking::MarkWhite(mark_bit);
+  mark_bit = Marking::MarkBitFrom(*new_space_object);
+  Marking::MarkWhite(mark_bit);
+  mark_bit = Marking::MarkBitFrom(*old_space_object_non_evacuation);
+  Marking::MarkWhite(mark_bit);
+
+  delete buffer;
+}
+
+
 TEST(ContextMeasure) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());