Fix WeakMap processing for evacuation candidates.

This fixes processing of WeakMaps so that value entries on an evacuation
candidate are correctly recorded in the slots buffer. We didn't pass the
correct slot into the backing hashtable while visiting values.

Also the live bytes counter for large object space pages was not reset
correctly when incremental marking is aborted.

R=vegorov@chromium.org
BUG=v8:2060
TEST=cctest/test-weakmaps/Regress2060

Review URL: https://chromiumcodereview.appspot.com/10034010

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11264 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index b4f488b..fb5a7e7 100644 (file)
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -337,6 +337,7 @@ void MarkCompactCollector::VerifyMarkbitsAreClean() {
   for (HeapObject* obj = it.Next(); obj != NULL; obj = it.Next()) {
     MarkBit mark_bit = Marking::MarkBitFrom(obj);
     ASSERT(Marking::IsWhite(mark_bit));
+    ASSERT_EQ(0, Page::FromAddress(obj->address())->LiveBytes());
   }
 }
 #endif
@@ -373,6 +374,7 @@ void MarkCompactCollector::ClearMarkbits() {
     MarkBit mark_bit = Marking::MarkBitFrom(obj);
     mark_bit.Clear();
     mark_bit.Next().Clear();
+    Page::FromAddress(obj->address())->ResetLiveBytes();
   }
 }
 
@@ -2592,12 +2594,10 @@ void MarkCompactCollector::ProcessWeakMaps() {
     ObjectHashTable* table = ObjectHashTable::cast(weak_map->table());
     for (int i = 0; i < table->Capacity(); i++) {
       if (MarkCompactCollector::IsMarked(HeapObject::cast(table->KeyAt(i)))) {
-        Object* value = table->get(table->EntryToValueIndex(i));
-        StaticMarkingVisitor::VisitPointer(heap(), &value);
-        table->set_unchecked(heap(),
-                             table->EntryToValueIndex(i),
-                             value,
-                             UPDATE_WRITE_BARRIER);
+        int idx = ObjectHashTable::EntryToValueIndex(i);
+        Object** slot =
+            HeapObject::RawField(table, FixedArray::OffsetOfElementAt(idx));
+        StaticMarkingVisitor::VisitPointer(heap(), slot);
       }
     }
     weak_map_obj = weak_map->next();

diff --git a/test/cctest/test-weakmaps.cc b/test/cctest/test-weakmaps.cc
index 56d5936..d12b9c7 100644 (file)
--- a/test/cctest/test-weakmaps.cc
+++ b/test/cctest/test-weakmaps.cc
@@ -48,11 +48,11 @@ static Handle<JSWeakMap> AllocateJSWeakMap() {
 
 static void PutIntoWeakMap(Handle<JSWeakMap> weakmap,
                            Handle<JSObject> key,
-                           int value) {
+                           Handle<Object> value) {
   Handle<ObjectHashTable> table = PutIntoObjectHashTable(
       Handle<ObjectHashTable>(ObjectHashTable::cast(weakmap->table())),
       Handle<JSObject>(JSObject::cast(*key)),
-      Handle<Smi>(Smi::FromInt(value)));
+      value);
   weakmap->set_table(*table);
 }
 
@@ -83,7 +83,9 @@ TEST(Weakness) {
   // Put entry into weak map.
   {
     v8::HandleScope scope;
-    PutIntoWeakMap(weakmap, Handle<JSObject>(JSObject::cast(*key)), 23);
+    PutIntoWeakMap(weakmap,
+                   Handle<JSObject>(JSObject::cast(*key)),
+                   Handle<Smi>(Smi::FromInt(23)));
   }
   CHECK_EQ(1, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
 
@@ -133,7 +135,7 @@ TEST(Shrinking) {
     Handle<Map> map = FACTORY->NewMap(JS_OBJECT_TYPE, JSObject::kHeaderSize);
     for (int i = 0; i < 32; i++) {
       Handle<JSObject> object = FACTORY->NewJSObjectFromMap(map);
-      PutIntoWeakMap(weakmap, object, i);
+      PutIntoWeakMap(weakmap, object, Handle<Smi>(Smi::FromInt(i)));
     }
   }
 
@@ -152,3 +154,35 @@ TEST(Shrinking) {
   // Check shrunk capacity.
   CHECK_EQ(32, ObjectHashTable::cast(weakmap->table())->Capacity());
 }
+
+
+// Test that weak map values on an evacuation candidate which are not reachable
+// by other paths are correctly recorded in the slots buffer.
+TEST(Regress2060) {
+  FLAG_always_compact = true;
+  LocalContext context;
+  v8::HandleScope scope;
+  Handle<JSFunction> function =
+      FACTORY->NewFunction(FACTORY->function_symbol(), FACTORY->null_value());
+  Handle<JSObject> key = FACTORY->NewJSObject(function);
+  Handle<JSWeakMap> weakmap = AllocateJSWeakMap();
+
+  // Start second old-space page so that values land on evacuation candidate.
+  Page* first_page = HEAP->old_pointer_space()->anchor()->next_page();
+  FACTORY->NewFixedArray(900 * KB / kPointerSize, TENURED);
+
+  // Fill up weak map with values on an evacuation candidate.
+  {
+    v8::HandleScope scope;
+    for (int i = 0; i < 32; i++) {
+      Handle<JSObject> object = FACTORY->NewJSObject(function, TENURED);
+      CHECK(!HEAP->InNewSpace(object->address()));
+      CHECK(!first_page->Contains(object->address()));
+      PutIntoWeakMap(weakmap, key, object);
+    }
+  }
+
+  // Force compacting garbage collection.
+  CHECK(FLAG_always_compact);
+  HEAP->CollectAllGarbage(Heap::kNoGCFlags);
+}