Fix missing fast property accessors in heap snapshots.

Implementation for this case

var x = {};
x.__defineGetter__("y", function Y() { return 42; });

BUG=v8:1818
TEST=cctest/test-heap-profiler/FastCaseGetter

Review URL: http://codereview.chromium.org/8491041
Patch from Ilya Tikhonovsky <loislo@chromium.org>.

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

diff --git a/src/objects.cc b/src/objects.cc
index ca8714a..88efc3c 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -103,11 +103,6 @@ void PrintElementsKind(FILE* out, ElementsKind kind) {
 }
 
 
-// Getters and setters are stored in a fixed array property.  These are
-// constants for their indices.
-const int kGetterIndex = 0;
-const int kSetterIndex = 1;
-
 MUST_USE_RESULT static MaybeObject* CreateJSValue(JSFunction* constructor,
                                                   Object* value) {
   Object* result;
@@ -210,6 +205,13 @@ MaybeObject* Object::GetPropertyWithReceiver(Object* receiver,
 }
 
 
+// This may seem strange but the standard requires inline static const
+// definition, and w/o these the code doesn't link when being built in debug
+// mode using gcc.
+const int JSObject::kGetterIndex;
+const int JSObject::kSetterIndex;
+
+
 MaybeObject* JSObject::GetPropertyWithCallback(Object* receiver,
                                                Object* structure,
                                                String* name) {

diff --git a/src/objects.h b/src/objects.h
index a7554d2..a592a33 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -1886,6 +1886,11 @@ class JSObject: public JSReceiver {
 #endif
   Object* SlowReverseLookup(Object* value);
 
+  // Getters and setters are stored in a fixed array property.
+  // These are constants for their indices.
+  static const int kGetterIndex = 0;
+  static const int kSetterIndex = 1;
+
   // Maximal number of fast properties for the JSObject. Used to
   // restrict the number of map transitions to avoid an explosion in
   // the number of maps for objects used as dictionaries.

diff --git a/src/profile-generator.cc b/src/profile-generator.cc
index 8a68f53..1a2d005 100644 (file)
--- a/src/profile-generator.cc
+++ b/src/profile-generator.cc
@@ -1918,6 +1918,7 @@ void V8HeapExplorer::ExtractReferences(HeapObject* obj) {
           SetPropertyReference(
               obj, entry,
               heap_->prototype_symbol(), proto_or_map,
+              NULL,
               JSFunction::kPrototypeOrInitialMapOffset);
         } else {
           SetPropertyReference(
@@ -2101,6 +2102,7 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj,
             SetPropertyReference(
                 js_obj, entry,
                 descs->GetKey(i), js_obj->InObjectPropertyAt(index),
+                NULL,
                 js_obj->GetInObjectPropertyOffset(index));
           } else {
             SetPropertyReference(
@@ -2114,6 +2116,21 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj,
               js_obj, entry,
               descs->GetKey(i), descs->GetConstantFunction(i));
           break;
+        case CALLBACKS: {
+          Object* callback_obj = descs->GetValue(i);
+          if (callback_obj->IsFixedArray()) {
+            FixedArray* accessors = FixedArray::cast(callback_obj);
+            if (Object* getter = accessors->get(JSObject::kGetterIndex)) {
+              SetPropertyReference(js_obj, entry, descs->GetKey(i),
+                                   getter, "get-%s");
+            }
+            if (Object* setter = accessors->get(JSObject::kSetterIndex)) {
+              SetPropertyReference(js_obj, entry, descs->GetKey(i),
+                                   setter, "set-%s");
+            }
+          }
+          break;
+        }
         case NORMAL:  // only in slow mode
         case HANDLER:  // only in lookup results, not in descriptors
         case INTERCEPTOR:  // only in lookup results, not in descriptors
@@ -2122,9 +2139,6 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj,
         case CONSTANT_TRANSITION:
         case NULL_DESCRIPTOR:  // ... and not about "holes"
           break;
-          // TODO(svenpanne): Should we really ignore accessors here?
-        case CALLBACKS:
-          break;
       }
     }
   } else {
@@ -2349,15 +2363,23 @@ void V8HeapExplorer::SetPropertyReference(HeapObject* parent_obj,
                                           HeapEntry* parent_entry,
                                           String* reference_name,
                                           Object* child_obj,
+                                          const char* name_format_string,
                                           int field_offset) {
   HeapEntry* child_entry = GetEntry(child_obj);
   if (child_entry != NULL) {
     HeapGraphEdge::Type type = reference_name->length() > 0 ?
         HeapGraphEdge::kProperty : HeapGraphEdge::kInternal;
+    const char* name = name_format_string  != NULL ?
+        collection_->names()->GetFormatted(
+            name_format_string,
+            *reference_name->ToCString(DISALLOW_NULLS,
+                                       ROBUST_STRING_TRAVERSAL)) :
+        collection_->names()->GetName(reference_name);
+
     filler_->SetNamedReference(type,
                                parent_obj,
                                parent_entry,
-                               collection_->names()->GetName(reference_name),
+                               name,
                                child_obj,
                                child_entry);
     IndexedReferencesExtractor::MarkVisitedField(parent_obj, field_offset);

diff --git a/src/profile-generator.h b/src/profile-generator.h
index 5747dbd..44be3db 100644 (file)
--- a/src/profile-generator.h
+++ b/src/profile-generator.h
@@ -973,6 +973,7 @@ class V8HeapExplorer : public HeapEntriesAllocator {
                             HeapEntry* parent,
                             String* reference_name,
                             Object* child,
+                            const char* name_format_string = NULL,
                             int field_offset = -1);
   void SetPropertyShortcutReference(HeapObject* parent_obj,
                                     HeapEntry* parent,

diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc
index 87e7a7d..f835a1e 100644 (file)
--- a/test/cctest/test-heap-profiler.cc
+++ b/test/cctest/test-heap-profiler.cc
@@ -1023,3 +1023,30 @@ TEST(GetConstructorName) {
   CHECK_EQ(0, StringCmp(
       "Object", i::V8HeapExplorer::GetConstructorName(*js_obj6)));
 }
+
+TEST(FastCaseGetter) {
+  v8::HandleScope scope;
+  LocalContext env;
+
+  CompileRun("var obj1 = {};\n"
+             "obj1.__defineGetter__('propWithGetter', function Y() {\n"
+             "  return 42;\n"
+             "});\n"
+             "obj1.__defineSetter__('propWithSetter', function Z(value) {\n"
+             "  return this.value_ = value;\n"
+             "});\n");
+  const v8::HeapSnapshot* snapshot =
+      v8::HeapProfiler::TakeSnapshot(v8_str("fastCaseGetter"));
+
+  const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
+  CHECK_NE(NULL, global);
+  const v8::HeapGraphNode* obj1 =
+      GetProperty(global, v8::HeapGraphEdge::kShortcut, "obj1");
+  CHECK_NE(NULL, obj1);
+  const v8::HeapGraphNode* getterFunction =
+      GetProperty(obj1, v8::HeapGraphEdge::kProperty, "get-propWithGetter");
+  CHECK_NE(NULL, getterFunction);
+  const v8::HeapGraphNode* setterFunction =
+      GetProperty(obj1, v8::HeapGraphEdge::kProperty, "set-propWithSetter");
+  CHECK_NE(NULL, setterFunction);
+}