Revert "Fix invalidation of old-to-old slots after object trimming."

This reverts commit 719d23c.

Reason for revert: TSAN failures

Original change's description:
> Fix invalidation of old-to-old slots after object trimming.
> 
> A recorded old-to-old slot may be overwritten with a pointer to a new
> space object. If the object containing the slot is trimmed later on,
> then the mark-compactor may crash on a stale pointer to new space.
> 
> This patch ensures that:
> 1) On trimming of an object we add it to the invalidated_slots sets.
> 2) The InvalidatedSlotsFilter::IsValid returns false for slots outside
>    the invalidated object unless the page was already swept.
> 
> Array left-trimming is handled as a special case because object start
> moves and cannot be added to the invalidated set. Instead, we clear
> the freed memory so that the recorded slots contain Smi values.
> 
> Bug: chromium:870226,chromium:816426
> Change-Id: Iffc05a58fcf52ece45fdb085b5d1fd4b3acb5d53
> Reviewed-on: https://chromium-review.googlesource.com/1163784
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Hannes Payer <hpayer@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#54953}

TBR=ulan@chromium.org,hpayer@chromium.org,mlippautz@chromium.org

Change-Id: I2e1ff83c2db7902488951a8f597d38133aeb3b04
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:870226, chromium:816426
Reviewed-on: https://chromium-review.googlesource.com/1165862
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54954}

src/heap/heap.cc

@@ -2882,17 +2882,6 @@ class LeftTrimmerVerifierRootVisitor : public RootVisitor {
 }  // namespace
 #endif  // ENABLE_SLOW_DCHECKS
 
-namespace {
-bool MayContainRecordedSlots(HeapObject* object) {
-  // New space object do not have recorded slots.
-  if (MemoryChunk::FromHeapObject(object)->InNewSpace()) return false;
-  // Whitelist objects that definitely do not have pointers.
-  if (object->IsByteArray() || object->IsFixedDoubleArray()) return false;
-  // Conservatively return true for other objects.
-  return true;
-}
-}  // namespace
-
 FixedArrayBase* Heap::LeftTrimFixedArray(FixedArrayBase* object,
                                          int elements_to_trim) {
   CHECK_NOT_NULL(object);
@@ -2929,17 +2918,7 @@ FixedArrayBase* Heap::LeftTrimFixedArray(FixedArrayBase* object,
   // Technically in new space this write might be omitted (except for
   // debug mode which iterates through the heap), but to play safer
   // we still do it.
-  // For arrays that can have recorded slots we clear the freed memory to
-  // avoid invalid old-to-old remembered set slots. Note that we cannot
-  // register invalidated slot because the start of the object can move if the
-  // array is left-trimmed later on.
-  DCHECK(HAS_SMI_TAG(static_cast<intptr_t>(kClearedFreeMemoryValue)));
-  ClearFreedMemoryMode clear_memory_mode =
-      MayContainRecordedSlots(object)
-          ? ClearFreedMemoryMode::kClearFreedMemory
-          : ClearFreedMemoryMode::kDontClearFreedMemory;
-  CreateFillerObjectAt(old_start, bytes_to_trim, ClearRecordedSlots::kYes,
-                       clear_memory_mode);
+  CreateFillerObjectAt(old_start, bytes_to_trim, ClearRecordedSlots::kYes);
 
   // Initialize header of the trimmed array. Since left trimming is only
   // performed on pages which are not concurrently swept creating a filler
@@ -3020,29 +2999,18 @@ void Heap::CreateFillerForArray(T* object, int elements_to_trim,
   }
 
   // Calculate location of new array end.
-  int old_size = object->Size();
-  Address old_end = object->address() + old_size;
+  Address old_end = object->address() + object->Size();
   Address new_end = old_end - bytes_to_trim;
 
-  // For arrays that can have recorded slots we clear the freed memory to
-  // avoid invalid old-to-old remembered set slots. Note that we cannot
-  // register invalidated slot because the start of the object can move if the
-  // array is left-trimmed later on.
-  ClearFreedMemoryMode clear_memory_mode =
-      MayContainRecordedSlots(object)
-          ? ClearFreedMemoryMode::kClearFreedMemory
-          : ClearFreedMemoryMode::kDontClearFreedMemory;
-  DCHECK(HAS_SMI_TAG(static_cast<intptr_t>(kClearedFreeMemoryValue)));
-
   // Technically in new space this write might be omitted (except for
   // debug mode which iterates through the heap), but to play safer
   // we still do it.
   // We do not create a filler for objects in large object space.
   // TODO(hpayer): We should shrink the large object page if the size
   // of the object changed significantly.
   if (!lo_space()->Contains(object)) {
-    HeapObject* filler = CreateFillerObjectAt(
-        new_end, bytes_to_trim, ClearRecordedSlots::kYes, clear_memory_mode);
+    HeapObject* filler =
+        CreateFillerObjectAt(new_end, bytes_to_trim, ClearRecordedSlots::kYes);
     DCHECK_NOT_NULL(filler);
     // Clear the mark bits of the black area that belongs now to the filler.
     // This is an optimization. The sweeper will release black fillers anyway.
@@ -3053,11 +3021,6 @@ void Heap::CreateFillerForArray(T* object, int elements_to_trim,
           page->AddressToMarkbitIndex(new_end),
           page->AddressToMarkbitIndex(new_end + bytes_to_trim));
     }
-  } else if (clear_memory_mode == ClearFreedMemoryMode::kClearFreedMemory) {
-    // Even though we do not create filler for large object space arrays, we
-    // have to clear the free memory if old-to-old slots may have been recorded.
-    memset(reinterpret_cast<void*>(new_end), kClearedFreeMemoryValue,
-           bytes_to_trim);
   }
 
   // Initialize header of the trimmed array. We are storing the new length
@@ -3327,12 +3290,20 @@ void Heap::RegisterDeserializedObjectsForBlackAllocation(
 
 void Heap::NotifyObjectLayoutChange(HeapObject* object, int size,
                                     const DisallowHeapAllocation&) {
-  if (incremental_marking()->IsMarking()) {
+  // In large object space, we only allow changing from non-pointers to
+  // pointers, so that there are no slots in the invalidated slots buffer.
+  // TODO(ulan) Make the IsString assertion stricter, we only want to allow
+  // strings that cannot have slots in them (seq-strings and such).
+  DCHECK(InOldSpace(object) || InNewSpace(object) ||
+         (lo_space()->Contains(object) &&
+          (object->IsString() || object->IsByteArray())));
+  if (FLAG_incremental_marking && incremental_marking()->IsMarking()) {
     incremental_marking()->MarkBlackAndPush(object);
-    if (incremental_marking()->IsCompacting() &&
-        MayContainRecordedSlots(object)) {
-      MemoryChunk::FromHeapObject(object)->RegisterObjectWithInvalidatedSlots(
-          object, size);
+    if (InOldSpace(object) && incremental_marking()->IsCompacting()) {
+      // The concurrent marker might have recorded slots for the object.
+      // Register this object as invalidated to filter out the slots.
+      MemoryChunk* chunk = MemoryChunk::FromAddress(object->address());
+      chunk->RegisterObjectWithInvalidatedSlots(object, size);
     }
   }
 #ifdef VERIFY_HEAP

src/heap/invalidated-slots-inl.h

@@ -56,7 +56,10 @@ bool InvalidatedSlotsFilter::IsValid(Address slot) {
             static_cast<int>(invalidated_end_ - invalidated_start_));
 
   if (offset >= invalidated_object_size_) {
-    return slots_in_free_space_are_valid_;
+    // A new object could have been allocated during evacuation in the free
+    // space outside the object. Since objects are not invalidated in GC pause
+    // we can return true here.
+    return true;
   }
   return invalidated_object_->IsValidSlot(invalidated_object_->map(), offset);
 }

src/heap/invalidated-slots.cc

@@ -9,12 +9,8 @@ namespace v8 {
 namespace internal {
 
 InvalidatedSlotsFilter::InvalidatedSlotsFilter(MemoryChunk* chunk) {
-  // Adjust slots_in_free_space_are_valid_ if more spaces are added.
-  DCHECK_IMPLIES(chunk->invalidated_slots() != nullptr, chunk->InOldSpace());
-  // The sweeper removes invalid slots and makes free space available for
-  // allocation. Slots for new objects can be recorded in new space.
-  slots_in_free_space_are_valid_ = chunk->SweepingDone();
-
+  DCHECK_IMPLIES(chunk->invalidated_slots() != nullptr,
+                 chunk->owner()->identity() == OLD_SPACE);
   InvalidatedSlots* invalidated_slots =
       chunk->invalidated_slots() ? chunk->invalidated_slots() : &empty_;
   iterator_ = invalidated_slots->begin();

src/heap/invalidated-slots.h

@@ -42,7 +42,6 @@ class InvalidatedSlotsFilter {
   Address invalidated_end_;
   HeapObject* invalidated_object_;
   int invalidated_object_size_;
-  bool slots_in_free_space_are_valid_;
   InvalidatedSlots empty_;
 #ifdef DEBUG
   Address last_slot_;

src/heap/mark-compact.cc

@@ -2195,8 +2195,6 @@ static inline SlotCallbackResult UpdateSlot(
     }
     DCHECK(!Heap::InFromSpace(target));
     DCHECK(!MarkCompactCollector::IsOnEvacuationCandidate(target));
-  } else {
-    DCHECK(heap_obj->map()->IsMap());
   }
   // OLD_TO_OLD slots are always removed after updating.
   return REMOVE_SLOT;

src/heap/spaces.cc

@@ -768,14 +768,6 @@ bool MemoryChunk::IsPagedSpace() const {
   return owner()->identity() != LO_SPACE;
 }
 
-bool MemoryChunk::InOldSpace() const {
-  return owner()->identity() == OLD_SPACE;
-}
-
-bool MemoryChunk::InLargeObjectSpace() const {
-  return owner()->identity() == LO_SPACE;
-}
-
 MemoryChunk* MemoryAllocator::AllocateChunk(size_t reserve_area_size,
                                             size_t commit_area_size,
                                             Executability executable,

src/heap/spaces.h

@@ -626,10 +626,6 @@ class MemoryChunk {
 
   bool InFromSpace() { return IsFlagSet(IN_FROM_SPACE); }
 
-  bool InOldSpace() const;
-
-  bool InLargeObjectSpace() const;
-
   Space* owner() const { return owner_; }
 
   void set_owner(Space* space) { owner_ = space; }

test/cctest/heap/heap-tester.h

@@ -21,10 +21,6 @@
   V(InvalidatedSlotsEvacuationCandidate)                  \
   V(InvalidatedSlotsNoInvalidatedRanges)                  \
   V(InvalidatedSlotsResetObjectRegression)                \
-  V(InvalidatedSlotsRightTrimFixedArray)                  \
-  V(InvalidatedSlotsRightTrimLargeFixedArray)             \
-  V(InvalidatedSlotsLeftTrimFixedArray)                   \
-  V(InvalidatedSlotsFastToSlow)                           \
   V(InvalidatedSlotsSomeInvalidatedRanges)                \
   V(TestNewSpaceRefsInCopiedCode)                         \
   V(GCFlags)                                              \