Reviewers: Yang,

Message:
PTAL

Description:
Fix PathTracer.

When tracing, we abuse the map for marking, thereby mutating it.
HeapObject::map() takes care of recovering unabused value.

Please review this at https://codereview.chromium.org/316533002/

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

Affected files (+28, -32 lines):
  M src/heap.h
  M src/heap.cc
  M src/objects-inl.h
  M test/cctest/test-heap.cc


Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index ff7a9ff12f20d6f7e37751f7d2bd91fb26939684..28bd81b3c4a8dd33c5bfcc06d1133d801d1812e5 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -5837,9 +5837,8 @@ void PathTracer::MarkRecursively(Object** p, MarkVisitor* mark_visitor) {

   HeapObject* obj = HeapObject::cast(*p);

-  Object* map = obj->map();
-
-  if (!map->IsHeapObject()) return;  // visited before
+  MapWord map_word = obj->map_word();
+  if (!map_word.ToMap()->IsHeapObject()) return;  // visited before

   if (found_target_in_trace_) return;  // stop if target found
   object_stack_.Add(obj);
@@ -5853,11 +5852,11 @@ void PathTracer::MarkRecursively(Object** p, MarkVisitor* mark_visitor) {
   bool is_native_context = SafeIsNativeContext(obj);

   // not visited yet
-  Map* map_p = reinterpret_cast<Map*>(HeapObject::cast(map));
-
-  Address map_addr = map_p->address();
+  Map* map = Map::cast(map_word.ToMap());

- obj->set_map_no_write_barrier(reinterpret_cast<Map*>(map_addr + kMarkTag));
+  MapWord marked_map_word =
+      MapWord::FromRawValue(obj->map_word().ToRawValue() + kMarkTag);
+  obj->set_map_word(marked_map_word);

   // Scan the object body.
   if (is_native_context && (visit_mode_ == VISIT_ONLY_STRONG)) {
@@ -5868,17 +5867,16 @@ void PathTracer::MarkRecursively(Object** p, MarkVisitor* mark_visitor) {
         Context::kHeaderSize + Context::FIRST_WEAK_SLOT * kPointerSize);
     mark_visitor->VisitPointers(start, end);
   } else {
-    obj->IterateBody(map_p->instance_type(),
-                     obj->SizeFromMap(map_p),
-                     mark_visitor);
+ obj->IterateBody(map->instance_type(), obj->SizeFromMap(map), mark_visitor);
   }

   // Scan the map after the body because the body is a lot more interesting
   // when doing leak detection.
-  MarkRecursively(&map, mark_visitor);
+  MarkRecursively(reinterpret_cast<Object**>(&map), mark_visitor);

-  if (!found_target_in_trace_)  // don't pop if found the target
+  if (!found_target_in_trace_) {  // don't pop if found the target
     object_stack_.RemoveLast();
+  }
 }


@@ -5887,25 +5885,18 @@ void PathTracer::UnmarkRecursively(Object** p, UnmarkVisitor* unmark_visitor) {

   HeapObject* obj = HeapObject::cast(*p);

-  Object* map = obj->map();
-
-  if (map->IsHeapObject()) return;  // unmarked already
-
-  Address map_addr = reinterpret_cast<Address>(map);
-
-  map_addr -= kMarkTag;
-
-  ASSERT_TAG_ALIGNED(map_addr);
+  MapWord map_word = obj->map_word();
+  if (map_word.ToMap()->IsHeapObject()) return;  // unmarked already

-  HeapObject* map_p = HeapObject::FromAddress(map_addr);
+  MapWord unmarked_map_word =
+      MapWord::FromRawValue(map_word.ToRawValue() - kMarkTag);
+  obj->set_map_word(unmarked_map_word);

-  obj->set_map_no_write_barrier(reinterpret_cast<Map*>(map_p));
+  Map* map = Map::cast(unmarked_map_word.ToMap());

-  UnmarkRecursively(reinterpret_cast<Object**>(&map_p), unmark_visitor);
+  UnmarkRecursively(reinterpret_cast<Object**>(&map), unmark_visitor);

-  obj->IterateBody(Map::cast(map_p)->instance_type(),
-                   obj->SizeFromMap(Map::cast(map_p)),
-                   unmark_visitor);
+ obj->IterateBody(map->instance_type(), obj->SizeFromMap(map), unmark_visitor);
 }


Index: src/heap.h
diff --git a/src/heap.h b/src/heap.h
index 8956e3c56b50f31af04c57a3caf9669d4c58f3f2..c7a7c82aa65c17252f376a681f9b318f40078493 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -2717,6 +2717,9 @@ class PathTracer : public ObjectVisitor {
     FIND_FIRST  // Will stop the search after first match.
   };

+  // Tags 0, 1, and 3 are used. Use 2 for marking visited HeapObject.
+  static const int kMarkTag = 2;
+
   // For the WhatToFind arg, if FIND_FIRST is specified, tracing will stop
   // after the first match.  If FIND_ALL is specified, then tracing will be
   // done for all matches.
@@ -2748,9 +2751,6 @@ class PathTracer : public ObjectVisitor {
   void UnmarkRecursively(Object** p, UnmarkVisitor* unmark_visitor);
   virtual void ProcessResults();

-  // Tags 0, 1, and 3 are used. Use 2 for marking visited HeapObject.
-  static const int kMarkTag = 2;
-
   Object* search_target_;
   bool found_target_;
   bool found_target_in_trace_;
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 42c46e0568f10f8b3289e3d555b02106c2234c56..7af165bd7efb0ec3e4cb6b93c12113ca784464ab 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1328,7 +1328,14 @@ Isolate* HeapObject::GetIsolate() {


 Map* HeapObject::map() {
+#ifdef DEBUG
+  // Clear mark potentially added by PathTracer.
+  uintptr_t raw_value =
+ map_word().ToRawValue() & ~static_cast<uintptr_t>(PathTracer::kMarkTag);
+  return MapWord::FromRawValue(raw_value).ToMap();
+#else
   return map_word().ToMap();
+#endif
 }


Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 6d4da76240052db2fadc1097e19a3513a19a3378..34d8b9e7a4e0841342454e41e7ccca9f11684f63 100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -4274,8 +4274,6 @@ TEST(ArrayShiftSweeping) {

 #ifdef DEBUG
 TEST(PathTracer) {
- // Type cast checks fail because the path tracer abuses the map for marking.
-  if (i::FLAG_enable_slow_asserts) return;
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());



--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to