[heap] Filter out stale left-trimmed handles for scavenges

mlippautz · Commit bot · commit 7a88ff3cc096 · 2016-06-22T12:22:46.000Z
The missing part from https://codereview.chromium.org/2078403002/ R=jochen@chromium.org BUG=chromium:621869 LOG=N Review-Url: https://codereview.chromium.org/2077353004 Cr-Commit-Position: refs/heads/master@{#37184}
diff --git a/src/heap/heap-inl.h b/src/heap/heap-inl.h
@@ -476,6 +476,31 @@ void Heap::CopyBlock(Address dst, Address src, int byte_size) {
             static_cast<size_t>(byte_size / kPointerSize));
 }
 
+bool Heap::PurgeLeftTrimmedObject(Object** object) {
+  HeapObject* current = reinterpret_cast<HeapObject*>(*object);
+  const MapWord map_word = current->map_word();
+  if (current->IsFiller() && !map_word.IsForwardingAddress()) {
+#ifdef DEBUG
+    // We need to find a FixedArrayBase map after walking the fillers.
+    while (current->IsFiller()) {
+      Address next = reinterpret_cast<Address>(current);
+      if (current->map() == one_pointer_filler_map()) {
+        next += kPointerSize;
+      } else if (current->map() == two_pointer_filler_map()) {
+        next += 2 * kPointerSize;
+      } else {
+        next += current->Size();
+      }
+      current = reinterpret_cast<HeapObject*>(next);
+    }
+    DCHECK(current->IsFixedArrayBase());
+#endif  // DEBUG
+    *object = nullptr;
+    return true;
+  }
+  return false;
+}
+
 template <Heap::FindMementoMode mode>
 AllocationMemento* Heap::FindAllocationMemento(HeapObject* object) {
   // Check if there is potentially a memento behind the object. If
diff --git a/src/heap/heap.h b/src/heap/heap.h
@@ -626,6 +626,12 @@ class Heap {
   // stored on the map to facilitate fast dispatch for {StaticVisitorBase}.
   static int GetStaticVisitorIdForMap(Map* map);
 
+  // We cannot avoid stale handles to left-trimmed objects, but can only make
+  // sure all handles still needed are updated. Filter out a stale pointer
+  // and clear the slot to allow post processing of handles (needed because
+  // the sweeper might actually free the underlying page).
+  inline bool PurgeLeftTrimmedObject(Object** object);
+
   // Notifies the heap that is ok to start marking or other activities that
   // should not happen during deserialization.
   void NotifyDeserializationComplete();
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
@@ -1435,31 +1435,7 @@ class RootMarkingVisitor : public ObjectVisitor {
 
     HeapObject* object = HeapObject::cast(*p);
 
-    // We cannot avoid stale handles to left-trimmed objects, but can only make
-    // sure all handles still needed are updated. Filter out any stale pointers
-    // and clear the slot to allow post processing of handles (needed because
-    // the sweeper might actually free the underlying page).
-    if (object->IsFiller()) {
-#ifdef DEBUG
-      // We need to find a FixedArrayBase map after walking the fillers.
-      Heap* heap = collector_->heap();
-      HeapObject* current = object;
-      while (current->IsFiller()) {
-        Address next = reinterpret_cast<Address>(current);
-        if (current->map() == heap->one_pointer_filler_map()) {
-          next += kPointerSize;
-        } else if (current->map() == heap->two_pointer_filler_map()) {
-          next += 2 * kPointerSize;
-        } else {
-          next += current->Size();
-        }
-        current = reinterpret_cast<HeapObject*>(next);
-      }
-      DCHECK(current->IsFixedArrayBase());
-#endif  // DEBUG
-      *p = nullptr;
-      return;
-    }
+    if (collector_->heap()->PurgeLeftTrimmedObject(p)) return;
 
     MarkBit mark_bit = Marking::MarkBitFrom(object);
     if (Marking::IsBlackOrGrey(mark_bit)) return;
diff --git a/src/heap/scavenger.cc b/src/heap/scavenger.cc
@@ -444,6 +444,9 @@ void ScavengeVisitor::VisitPointers(Object** start, Object** end) {
 void ScavengeVisitor::ScavengePointer(Object** p) {
   Object* object = *p;
   if (!heap_->InNewSpace(object)) return;
+
+  if (heap_->PurgeLeftTrimmedObject(p)) return;
+
   Scavenger::ScavengeObject(reinterpret_cast<HeapObject**>(p),
                             reinterpret_cast<HeapObject*>(object));
 }
diff --git a/src/objects-inl.h b/src/objects-inl.h
@@ -1274,8 +1274,7 @@ Map* MapWord::ToMap() {
   return reinterpret_cast<Map*>(value_);
 }
 
-
-bool MapWord::IsForwardingAddress() {
+bool MapWord::IsForwardingAddress() const {
   return HAS_SMI_TAG(reinterpret_cast<Object*>(value_));
 }
 
diff --git a/src/objects.h b/src/objects.h
@@ -1490,7 +1490,7 @@ class MapWord BASE_EMBEDDED {
   // True if this map word is a forwarding address for a scavenge
   // collection.  Only valid during a scavenge collection (specifically,
   // when all map words are heap object pointers, i.e. not during a full GC).
-  inline bool IsForwardingAddress();
+  inline bool IsForwardingAddress() const;
 
   // Create a map word from a forwarding address.
   static inline MapWord FromForwardingAddress(HeapObject* object);
diff --git a/test/mjsunit/regress/regress-621869.js b/test/mjsunit/regress/regress-621869.js
@@ -0,0 +1,18 @@
+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-gc
+
+var o0 = [];
+var o1 = [];
+var cnt = 0;
+var only_scavenge = true;
+o1.__defineGetter__(0, function() {
+  if (cnt++ > 2) return;
+  o0.shift();
+  gc(only_scavenge);
+  o0.push((64));
+  o0.concat(o1);
+});
+o1[0];

Original file line number	Diff line number	Diff line change
`@@ -1274,8 +1274,7 @@ Map* MapWord::ToMap() {`
`1274`	`1274`	`return reinterpret_cast<Map*>(value_);`
`1275`	`1275`	`}`
`1276`	`1276`
`1277`		`-`
`1278`		`-bool MapWord::IsForwardingAddress() {`
	`1277`	`+bool MapWord::IsForwardingAddress() const {`
`1279`	`1278`	`return HAS_SMI_TAG(reinterpret_cast<Object*>(value_));`
`1280`	`1279`	`}`
`1281`	`1280`