Reviewers: Vyacheslav Egorov,

Description:
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.

[email protected]
BUG=v8:2060
TEST=cctest/test-weakmaps/Regress2060


Please review this at https://chromiumcodereview.appspot.com/10034010/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/mark-compact.cc
  M test/cctest/test-weakmaps.cc


Index: src/mark-compact.cc
diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index b4f488bd9570e98191da12d61dc9ee25b3dfa084..fb5a7e7caef741e1c2fd1bd51c4a95662ba32417 100644
--- 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();
Index: test/cctest/test-weakmaps.cc
diff --git a/test/cctest/test-weakmaps.cc b/test/cctest/test-weakmaps.cc
index 56d593628af6a6fe67eef6cc465bcfe81b10e801..4c37145119489d56af47978411c12c244c7e5ef1 100644
--- 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,36 @@ 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 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);
+      printf("o%d = %p (!= %p)\n", i, (void*)*object, (void*)first_page);
+    }
+  }
+
+  // Force compacting garbage collection.
+  CHECK(FLAG_always_compact);
+  HEAP->CollectAllGarbage(Heap::kNoGCFlags);
+}


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to