Do not overwrite builtin code names in heap profiler

Make sure builtin code objects get their builtin tags
first. Otherwise a particular JSFunction object could set
its custom name to a generic builtin.

LOG=N
R=ulan@chromium.org, yurys@chromium.org

Review URL: https://codereview.chromium.org/145973006

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

src/heap-snapshot-generator.cc

@@ -1366,8 +1366,8 @@ void V8HeapExplorer::ExtractCodeCacheReferences(
 }
 
 
-void V8HeapExplorer::TagCodeObject(Code* code, const char* external_name) {
-  TagObject(code, names_->GetFormatted("(%s code)", external_name));
+void V8HeapExplorer::TagBuiltinCodeObject(Code* code, const char* name) {
+  TagObject(code, names_->GetFormatted("(%s builtin)", name));
 }
 
 
@@ -1663,24 +1663,20 @@ class RootsReferencesExtractor : public ObjectVisitor {
     }
     int strong_index = 0, all_index = 0, tags_index = 0, builtin_index = 0;
     while (all_index < all_references_.length()) {
-      if (strong_index < strong_references_.length() &&
-          strong_references_[strong_index] == all_references_[all_index]) {
+      bool is_strong = strong_index < strong_references_.length()
+          && strong_references_[strong_index] == all_references_[all_index];
       explorer->SetGcSubrootReference(reference_tags_[tags_index].tag,
-                                        false,
+                                      !is_strong,
                                       all_references_[all_index]);
-        ++strong_index;
-      } else {
-        explorer->SetGcSubrootReference(reference_tags_[tags_index].tag,
-                                        true,
-                                        all_references_[all_index]);
-      }
       if (reference_tags_[tags_index].tag ==
           VisitorSynchronization::kBuiltins) {
         ASSERT(all_references_[all_index]->IsCode());
-        explorer->TagCodeObject(Code::cast(all_references_[all_index]),
+        explorer->TagBuiltinCodeObject(
+            Code::cast(all_references_[all_index]),
             builtins->name(builtin_index++));
       }
       ++all_index;
+      if (is_strong) ++strong_index;
       if (reference_tags_[tags_index].index == all_index) ++tags_index;
     }
   }
@@ -1705,11 +1701,21 @@ class RootsReferencesExtractor : public ObjectVisitor {
 
 bool V8HeapExplorer::IterateAndExtractReferences(
     SnapshotFillerInterface* filler) {
-  HeapIterator iterator(heap_, HeapIterator::kFilterUnreachable);
-
   filler_ = filler;
-  bool interrupted = false;
 
+  // Make sure builtin code objects get their builtin tags
+  // first. Otherwise a particular JSFunction object could set
+  // its custom name to a generic builtin.
+  SetRootGcRootsReference();
+  RootsReferencesExtractor extractor(heap_);
+  heap_->IterateRoots(&extractor, VISIT_ONLY_STRONG);
+  extractor.SetCollectingAllReferences();
+  heap_->IterateRoots(&extractor, VISIT_ALL);
+  extractor.FillReferences(this);
+
+  // Now iterate the whole heap.
+  bool interrupted = false;
+  HeapIterator iterator(heap_, HeapIterator::kFilterUnreachable);
   // Heap iteration with filtering must be finished in any case.
   for (HeapObject* obj = iterator.next();
        obj != NULL;
@@ -1724,12 +1730,6 @@ bool V8HeapExplorer::IterateAndExtractReferences(
     return false;
   }
 
-  SetRootGcRootsReference();
-  RootsReferencesExtractor extractor(heap_);
-  heap_->IterateRoots(&extractor, VISIT_ONLY_STRONG);
-  extractor.SetCollectingAllReferences();
-  heap_->IterateRoots(&extractor, VISIT_ALL);
-  extractor.FillReferences(this);
   filler_ = NULL;
   return progress_->ProgressReport(true);
 }

src/heap-snapshot-generator.h

@@ -385,7 +385,7 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   bool IterateAndExtractReferences(SnapshotFillerInterface* filler);
   void TagGlobalObjects();
   void TagCodeObject(Code* code);
-  void TagCodeObject(Code* code, const char* external_name);
+  void TagBuiltinCodeObject(Code* code, const char* name);
 
   static String* GetConstructorName(JSObject* object);
 

test/cctest/test-heap-profiler.cc

@@ -2122,13 +2122,23 @@ TEST(CheckCodeNames) {
       stub_path, ARRAY_SIZE(stub_path));
   CHECK_NE(NULL, node);
 
-  const char* builtin_path[] = {
+  const char* builtin_path1[] = {
     "::(GC roots)",
     "::(Builtins)",
-    "::(KeyedLoadIC_Generic code)"
+    "::(KeyedLoadIC_Generic builtin)"
   };
-  node = GetNodeByPath(snapshot, builtin_path, ARRAY_SIZE(builtin_path));
+  node = GetNodeByPath(snapshot, builtin_path1, ARRAY_SIZE(builtin_path1));
   CHECK_NE(NULL, node);
+
+  const char* builtin_path2[] = {
+    "::(GC roots)",
+    "::(Builtins)",
+    "::(CompileUnoptimized builtin)"
+  };
+  node = GetNodeByPath(snapshot, builtin_path2, ARRAY_SIZE(builtin_path2));
+  CHECK_NE(NULL, node);
+  v8::String::Utf8Value node_name(node->GetName());
+  CHECK_EQ("(CompileUnoptimized builtin)", *node_name);
 }