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

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);

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());
   }

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());

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'));