Never let the hole escape...

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

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

diff --git a/src/objects.cc b/src/objects.cc
index 940120541e4ca51be3861fa72dca83c97b44fa80..bb2f2d3d3bb52a3270f896bd39bda9f0484a2767 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4694,7 +4694,7 @@ Object* JSObject::LookupAccessor(String* name, AccessorComponent component) {
           Object* element = dictionary->ValueAt(entry);
           if (dictionary->DetailsAt(entry).type() == CALLBACKS &&
               element->IsAccessorPair()) {
-            return AccessorPair::cast(element)->get(component);
+            return AccessorPair::cast(element)->SafeGet(component);
           }
         }
       }
@@ -4710,7 +4710,7 @@ Object* JSObject::LookupAccessor(String* name, AccessorComponent component) {
         if (result.type() == CALLBACKS) {
           Object* obj = result.GetCallbackObject();
           if (obj->IsAccessorPair()) {
-            return AccessorPair::cast(obj)->get(component);
+            return AccessorPair::cast(obj)->SafeGet(component);
           }
         }
       }
@@ -5947,6 +5947,12 @@ MaybeObject* AccessorPair::CopyWithoutTransitions() {
 }
 
 
+Object* AccessorPair::SafeGet(AccessorComponent component) {
+    Object* accessor = get(component);
+    return accessor->IsTheHole() ? GetHeap()->undefined_value() : accessor;
+}
+
+
 MaybeObject* DeoptimizationInputData::Allocate(int deopt_entry_count,
                                                PretenureFlag pretenure) {
   ASSERT(deopt_entry_count > 0);

diff --git a/src/objects.h b/src/objects.h
index 1b5ce957bf16699f147b934fe8883c3a5ced2379..76958c199b6c20947bc07a0dbb861b737c804386 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -7944,6 +7944,9 @@ class AccessorPair: public Struct {
     }
   }
 
+  // Same as get, but returns undefined instead of the hole.
+  Object* SafeGet(AccessorComponent component);
+
   bool ContainsAccessor() {
     return IsJSAccessor(getter()) || IsJSAccessor(setter());
   }

diff --git a/src/runtime.cc b/src/runtime.cc
index 34a47c5d00574cbf8eaf8d4c5a92edef4379f778..1da4f630380c69361029dccea9f69facf1ef3cf9 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -1073,10 +1073,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetOwnProperty) {
                 AccessorPair::cast(dictionary->ValueAt(entry));
             elms->set(IS_ACCESSOR_INDEX, heap->true_value());
             if (CheckElementAccess(*obj, index, v8::ACCESS_GET)) {
-              elms->set(GETTER_INDEX, accessors->getter());
+              elms->set(GETTER_INDEX, accessors->SafeGet(ACCESSOR_GETTER));
             }
             if (CheckElementAccess(*obj, index, v8::ACCESS_SET)) {
-              elms->set(SETTER_INDEX, accessors->setter());
+              elms->set(SETTER_INDEX, accessors->SafeGet(ACCESSOR_SETTER));
             }
             break;
           }
@@ -1123,10 +1123,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetOwnProperty) {
 
     AccessorPair* accessors = AccessorPair::cast(result.GetCallbackObject());
     if (CheckAccess(*obj, *name, &result, v8::ACCESS_GET)) {
-      elms->set(GETTER_INDEX, accessors->getter());
+      elms->set(GETTER_INDEX, accessors->SafeGet(ACCESSOR_GETTER));
     }
     if (CheckAccess(*obj, *name, &result, v8::ACCESS_SET)) {
-      elms->set(SETTER_INDEX, accessors->setter());
+      elms->set(SETTER_INDEX, accessors->SafeGet(ACCESSOR_SETTER));
     }
   } else {
     elms->set(IS_ACCESSOR_INDEX, heap->false_value());

diff --git a/test/mjsunit/object-define-property.js b/test/mjsunit/object-define-property.js
index 432fbdf7fc3c325bccd11cd81e8e0f9d41a75d0b..9384a357bae1da251e9b44c0123bf037ff40ea4a 100644 (file)
--- a/test/mjsunit/object-define-property.js
+++ b/test/mjsunit/object-define-property.js
@@ -1075,3 +1075,13 @@ assertEquals(999, o[999]);
   assertEquals(2, arg0);
   assertEquals(3, arguments[0]);
 })(0);
+
+
+// Regression test: We should never observe the hole value.
+var objectWithGetter = {};
+objectWithGetter.__defineGetter__('foo', function() {});
+assertEquals(undefined, objectWithGetter.__lookupSetter__('foo'));
+
+var objectWithSetter = {};
+objectWithSetter.__defineSetter__('foo', function(x) {});
+assertEquals(undefined, objectWithSetter.__lookupGetter__('foo'));