Fix issue with setters and their holders in accessors.cc

BUG=3462
LOG=Y
R=verwaest@chromium.org

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

Patch from Erik Arvidsson <arv@chromium.org>.

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

src/accessors.cc

@@ -136,6 +136,25 @@ bool Accessors::IsJSObjectFieldAccessor<HeapType>(Handle<HeapType> type,
                                                   int* object_offset);
 
 
+bool SetPropertyOnInstanceIfInherited(
+    Isolate* isolate, const v8::PropertyCallbackInfo<void>& info,
+    v8::Local<v8::String> name, Handle<Object> value) {
+  Handle<Object> holder = Utils::OpenHandle(*info.Holder());
+  Handle<Object> receiver = Utils::OpenHandle(*info.This());
+  if (*holder == *receiver) return false;
+  if (receiver->IsJSObject()) {
+    Handle<JSObject> object = Handle<JSObject>::cast(receiver);
+    // This behaves sloppy since we lost the actual strict-mode.
+    // TODO(verwaest): Fix by making ExecutableAccessorInfo behave like data
+    // properties.
+    if (!object->map()->is_extensible()) return true;
+    JSObject::SetOwnPropertyIgnoreAttributes(object, Utils::OpenHandle(*name),
+                                             value, NONE);
+  }
+  return true;
+}
+
+
 //
 // Accessors::ArrayLength
 //
@@ -176,17 +195,7 @@ void Accessors::ArrayLengthSetter(
   HandleScope scope(isolate);
   Handle<JSObject> object = Utils::OpenHandle(*info.This());
   Handle<Object> value = Utils::OpenHandle(*val);
-  // This means one of the object's prototypes is a JSArray and the
-  // object does not have a 'length' property.  Calling SetProperty
-  // causes an infinite loop.
-  if (!object->IsJSArray()) {
-    // This behaves sloppy since we lost the actual strict-mode.
-    // TODO(verwaest): Fix by making ExecutableAccessorInfo behave like data
-    // properties.
-    if (!object->map()->is_extensible()) return;
-    MaybeHandle<Object> maybe_result = JSObject::SetOwnPropertyIgnoreAttributes(
-        object, isolate->factory()->length_string(), value, NONE);
-    maybe_result.Check();
+  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) {
     return;
   }
 
@@ -838,25 +847,10 @@ static Handle<Object> GetFunctionPrototype(Isolate* isolate,
 
 
 static Handle<Object> SetFunctionPrototype(Isolate* isolate,
-                                           Handle<JSObject> receiver,
+                                           Handle<JSFunction> function,
                                            Handle<Object> value) {
-  Handle<JSFunction> function;
-  {
-    DisallowHeapAllocation no_allocation;
-    JSFunction* function_raw = FindInstanceOf<JSFunction>(isolate, *receiver);
-    if (function_raw == NULL) return isolate->factory()->undefined_value();
-    function = Handle<JSFunction>(function_raw, isolate);
-  }
-
-  if (!function->should_have_prototype()) {
-    // Since we hit this accessor, object will have no prototype property.
-    MaybeHandle<Object> maybe_result = JSObject::SetOwnPropertyIgnoreAttributes(
-        receiver, isolate->factory()->prototype_string(), value, NONE);
-    return maybe_result.ToHandleChecked();
-  }
-
   Handle<Object> old_value;
-  bool is_observed = *function == *receiver && function->map()->is_observed();
+  bool is_observed = function->map()->is_observed();
   if (is_observed) {
     if (function->has_prototype())
       old_value = handle(function->prototype(), isolate);
@@ -907,10 +901,12 @@ void Accessors::FunctionPrototypeSetter(
     const v8::PropertyCallbackInfo<void>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
-  Handle<JSObject> object =
-      Handle<JSObject>::cast(Utils::OpenHandle(*info.This()));
   Handle<Object> value = Utils::OpenHandle(*val);
-
+  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) {
+    return;
+  }
+  Handle<JSFunction> object =
+      Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
   SetFunctionPrototype(isolate, object, value);
 }
 
@@ -960,7 +956,8 @@ void Accessors::FunctionLengthSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
-  // Do nothing.
+  // Function length is non writable, non configurable.
+  UNREACHABLE();
 }
 
 
@@ -995,7 +992,8 @@ void Accessors::FunctionNameSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
-  // Do nothing.
+  // Function name is non writable, non configurable.
+  UNREACHABLE();
 }
 
 
@@ -1131,7 +1129,8 @@ void Accessors::FunctionArgumentsSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
-  // Do nothing.
+  // Function arguments is non writable, non configurable.
+  UNREACHABLE();
 }
 
 
@@ -1281,7 +1280,8 @@ void Accessors::FunctionCallerSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
-  // Do nothing.
+  // Function caller is non writable, non configurable.
+  UNREACHABLE();
 }
 
 

test/mjsunit/es7/object-observe.js

@@ -1685,8 +1685,10 @@ var obj = { __proto__: fun };
 Object.observe(obj, observer.callback);
 obj.prototype = 7;
 Object.deliverChangeRecords(observer.callback);
-observer.assertNotCalled();
-
+observer.assertRecordCount(1);
+observer.assertCallbackRecords([
+  { object: obj, name: 'prototype', type: 'add' },
+]);
 
 // Check that changes in observation status are detected in all IC states and
 // in optimized code, especially in cases usually using fast elements.

test/mjsunit/regress/regress-3462.js

+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+
+function TestFunctionPrototypeSetter() {
+  var f = function() {};
+  var o = {__proto__: f};
+  o.prototype = 42;
+  assertEquals(42, o.prototype);
+  assertTrue(o.hasOwnProperty('prototype'));
+}
+TestFunctionPrototypeSetter();
+
+
+function TestFunctionPrototypeSetterOnValue() {
+  var f = function() {};
+  var fp = f.prototype;
+  Number.prototype.__proto__ = f;
+  var n = 42;
+  var o = {};
+  n.prototype = o;
+  assertEquals(fp, n.prototype);
+  assertEquals(fp, f.prototype);
+  assertFalse(Number.prototype.hasOwnProperty('prototype'));
+}
+TestFunctionPrototypeSetterOnValue();
+
+
+function TestArrayLengthSetter() {
+  var a = [1];
+  var o = {__proto__: a};
+  o.length = 2;
+  assertEquals(2, o.length);
+  assertEquals(1, a.length);
+  assertTrue(o.hasOwnProperty('length'));
+}
+TestArrayLengthSetter();
+
+
+function TestArrayLengthSetterOnValue() {
+  Number.prototype.__proto__ = [1];
+  var n = 42;
+  n.length = 2;
+  assertEquals(1, n.length);
+  assertFalse(Number.prototype.hasOwnProperty('length'));
+}
+TestArrayLengthSetterOnValue();