Properly process arrays with overridden prototype in various Array's functions.

Bailout to JS Array builtins if array's prototype is different from
Array.prototype.  Otherwise there might be inherited elements coming
from this prototype.

Review URL: http://codereview.chromium.org/2037008

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

src/builtins.cc

@@ -330,22 +330,19 @@ static FixedArray* LeftTrimFixedArray(FixedArray* elms, int to_trim) {
 }
 
 
-static bool ArrayPrototypeHasNoElements() {
+static bool ArrayPrototypeHasNoElements(Context* global_context,
+                                        JSObject* array_proto) {
   // This method depends on non writability of Object and Array prototype
   // fields.
-  Context* global_context = Top::context()->global_context();
-  // Array.prototype
-  JSObject* proto =
-      JSObject::cast(global_context->array_function()->prototype());
-  if (proto->elements() != Heap::empty_fixed_array()) return false;
+  if (array_proto->elements() != Heap::empty_fixed_array()) return false;
   // Hidden prototype
-  proto = JSObject::cast(proto->GetPrototype());
-  ASSERT(proto->elements() == Heap::empty_fixed_array());
+  array_proto = JSObject::cast(array_proto->GetPrototype());
+  ASSERT(array_proto->elements() == Heap::empty_fixed_array());
   // Object.prototype
-  proto = JSObject::cast(proto->GetPrototype());
-  if (proto != global_context->initial_object_prototype()) return false;
-  if (proto->elements() != Heap::empty_fixed_array()) return false;
-  ASSERT(proto->GetPrototype()->IsNull());
+  array_proto = JSObject::cast(array_proto->GetPrototype());
+  if (array_proto != global_context->initial_object_prototype()) return false;
+  if (array_proto->elements() != Heap::empty_fixed_array()) return false;
+  ASSERT(array_proto->GetPrototype()->IsNull());
   return true;
 }
 
@@ -368,6 +365,18 @@ static bool IsJSArrayWithFastElements(Object* receiver,
 }
 
 
+static bool IsFastElementMovingAllowed(Object* receiver,
+                                       FixedArray** elements) {
+  if (!IsJSArrayWithFastElements(receiver, elements)) return false;
+
+  Context* global_context = Top::context()->global_context();
+  JSObject* array_proto =
+      JSObject::cast(global_context->array_function()->prototype());
+  if (JSArray::cast(receiver)->GetPrototype() != array_proto) return false;
+  return ArrayPrototypeHasNoElements(global_context, array_proto);
+}
+
+
 static Object* CallJsBuiltin(const char* name,
                              BuiltinArguments<NO_EXTRA_ARGUMENTS> args) {
   HandleScope handleScope;
@@ -465,11 +474,7 @@ BUILTIN(ArrayPop) {
     return top;
   }
 
-  // Remember to check the prototype chain.
-  JSFunction* array_function =
-      Top::context()->global_context()->array_function();
-  JSObject* prototype = JSObject::cast(array_function->prototype());
-  top = prototype->GetElement(len - 1);
+  top = array->GetPrototype()->GetElement(len - 1);
 
   return top;
 }
@@ -478,8 +483,7 @@ BUILTIN(ArrayPop) {
 BUILTIN(ArrayShift) {
   Object* receiver = *args.receiver();
   FixedArray* elms = NULL;
-  if (!IsJSArrayWithFastElements(receiver, &elms)
-      || !ArrayPrototypeHasNoElements()) {
+  if (!IsFastElementMovingAllowed(receiver, &elms)) {
     return CallJsBuiltin("ArrayShift", args);
   }
   JSArray* array = JSArray::cast(receiver);
@@ -515,8 +519,7 @@ BUILTIN(ArrayShift) {
 BUILTIN(ArrayUnshift) {
   Object* receiver = *args.receiver();
   FixedArray* elms = NULL;
-  if (!IsJSArrayWithFastElements(receiver, &elms)
-      || !ArrayPrototypeHasNoElements()) {
+  if (!IsFastElementMovingAllowed(receiver, &elms)) {
     return CallJsBuiltin("ArrayUnshift", args);
   }
   JSArray* array = JSArray::cast(receiver);
@@ -565,8 +568,7 @@ BUILTIN(ArrayUnshift) {
 BUILTIN(ArraySlice) {
   Object* receiver = *args.receiver();
   FixedArray* elms = NULL;
-  if (!IsJSArrayWithFastElements(receiver, &elms)
-      || !ArrayPrototypeHasNoElements()) {
+  if (!IsFastElementMovingAllowed(receiver, &elms)) {
     return CallJsBuiltin("ArraySlice", args);
   }
   JSArray* array = JSArray::cast(receiver);
@@ -635,8 +637,7 @@ BUILTIN(ArraySlice) {
 BUILTIN(ArraySplice) {
   Object* receiver = *args.receiver();
   FixedArray* elms = NULL;
-  if (!IsJSArrayWithFastElements(receiver, &elms)
-      || !ArrayPrototypeHasNoElements()) {
+  if (!IsFastElementMovingAllowed(receiver, &elms)) {
     return CallJsBuiltin("ArraySplice", args);
   }
   JSArray* array = JSArray::cast(receiver);
@@ -788,7 +789,10 @@ BUILTIN(ArraySplice) {
 
 
 BUILTIN(ArrayConcat) {
-  if (!ArrayPrototypeHasNoElements()) {
+  Context* global_context = Top::context()->global_context();
+  JSObject* array_proto =
+      JSObject::cast(global_context->array_function()->prototype());
+  if (!ArrayPrototypeHasNoElements(global_context, array_proto)) {
     return CallJsBuiltin("ArrayConcat", args);
   }
 
@@ -798,7 +802,8 @@ BUILTIN(ArrayConcat) {
   int result_len = 0;
   for (int i = 0; i < n_arguments; i++) {
     Object* arg = args[i];
-    if (!arg->IsJSArray() || !JSArray::cast(arg)->HasFastElements()) {
+    if (!arg->IsJSArray() || !JSArray::cast(arg)->HasFastElements()
+        || JSArray::cast(arg)->GetPrototype() != array_proto) {
       return CallJsBuiltin("ArrayConcat", args);
     }
 

test/mjsunit/array-concat.js

@@ -29,7 +29,82 @@
  * @fileoverview Test concat on small and large arrays
  */
 
-var poses = [140, 4000000000];
+var poses;
+
+poses = [140, 4000000000];
+while (pos = poses.shift()) {
+  var a = new Array(pos);
+  var array_proto = [];
+  a.__proto__ = array_proto;
+  assertEquals(pos, a.length);
+  a.push('foo');
+  assertEquals(pos + 1, a.length);
+  var b = ['bar'];
+  var c = a.concat(b);
+  assertEquals(pos + 2, c.length);
+  assertEquals("undefined", typeof(c[pos - 1]));
+  assertEquals("foo", c[pos]);
+  assertEquals("bar", c[pos + 1]);
+
+  // Can we fool the system by putting a number in a string?
+  var onetwofour = "124";
+  a[onetwofour] = 'doo';
+  assertEquals(a[124], 'doo');
+  c = a.concat(b);
+  assertEquals(c[124], 'doo');
+
+  // If we put a number in the prototype, then the spec says it should be
+  // copied on concat.
+  array_proto["123"] = 'baz';
+  assertEquals(a[123], 'baz');
+
+  c = a.concat(b);
+  assertEquals(pos + 2, c.length);
+  assertEquals("baz", c[123]);
+  assertEquals("undefined", typeof(c[pos - 1]));
+  assertEquals("foo", c[pos]);
+  assertEquals("bar", c[pos + 1]);
+
+  // When we take the number off the prototype it disappears from a, but
+  // the concat put it in c itself.
+  array_proto["123"] = undefined;
+  assertEquals("undefined", typeof(a[123]));
+  assertEquals("baz", c[123]);
+
+  // If the element of prototype is shadowed, the element on the instance
+  // should be copied, but not the one on the prototype.
+  array_proto[123] = 'baz';
+  a[123] = 'xyz';
+  assertEquals('xyz', a[123]);
+  c = a.concat(b);
+  assertEquals('xyz', c[123]);
+
+  // Non-numeric properties on the prototype or the array shouldn't get
+  // copied.
+  array_proto.moe = 'joe';
+  a.ben = 'jerry';
+  assertEquals(a["moe"], 'joe');
+  assertEquals(a["ben"], 'jerry');
+  c = a.concat(b);
+  // ben was not copied
+  assertEquals("undefined", typeof(c.ben));
+
+  // When we take moe off the prototype it disappears from all arrays.
+  array_proto.moe = undefined;
+  assertEquals("undefined", typeof(c.moe));
+
+  // Negative indices don't get concated.
+  a[-1] = 'minus1';
+  assertEquals("minus1", a[-1]);
+  assertEquals("undefined", typeof(a[0xffffffff]));
+  c = a.concat(b);
+  assertEquals("undefined", typeof(c[-1]));
+  assertEquals("undefined", typeof(c[0xffffffff]));
+  assertEquals(c.length, a.length + 1);
+
+}
+
+poses = [140, 4000000000];
 while (pos = poses.shift()) {
   var a = new Array(pos);
   assertEquals(pos, a.length);
@@ -91,7 +166,7 @@ while (pos = poses.shift()) {
   Array.prototype.moe = undefined;
   assertEquals("undefined", typeof(c.moe));
 
-  // Negative indeces don't get concated.
+  // Negative indices don't get concated.
   a[-1] = 'minus1';
   assertEquals("minus1", a[-1]);
   assertEquals("undefined", typeof(a[0xffffffff]));

test/mjsunit/array-pop.js

@@ -81,6 +81,34 @@
     }
     Array.prototype.length = 0;  // Clean-up.
   }
+
+  // Check that pop works on inherited properties for
+  // arrays with array prototype.
+  for (var i = 0; i < 10 ;i++) {  // Ensure ICs are stabilized.
+    var array_proto = [];
+    array_proto[1] = 1;
+    array_proto[3] = 3;
+    array_proto[5] = 5;
+    array_proto[7] = 7;
+    array_proto[9] = 9;
+    a = [0,1,2,,4,,6,7,8,,];
+    a.__proto__ = array_proto;
+    assertEquals(10, a.length, "array_proto-inherit-initial-length");
+    for (var j = 9; j >= 0; j--) {
+      assertEquals(j + 1, a.length, "array_proto-inherit-pre-length-" + j);
+      assertTrue(j in a, "array_proto-has property " + j);
+      var own = a.hasOwnProperty(j);
+      var inherited = array_proto.hasOwnProperty(j);
+      assertEquals(j, a.pop(), "array_proto-inherit-pop");
+      assertEquals(j, a.length, "array_proto-inherit-post-length");
+      assertFalse(a.hasOwnProperty(j), "array_proto-inherit-deleted-own-" + j);
+      assertEquals(inherited, array_proto.hasOwnProperty(j),
+                   "array_proto-inherit-not-deleted-inherited" + j);
+    }
+  }
+
+  // Check that pop works on inherited properties for
+  // arrays with array prototype.
 })();
 
 // Test the case of not JSArray receiver.

test/mjsunit/array-shift.js

@@ -69,3 +69,40 @@
   assertTrue(delete Array.prototype[5]);
   assertTrue(delete Array.prototype[7]);
 })();
+
+// Now check the case with array of holes and some elements on prototype
+// which is an array itself.
+(function() {
+  var len = 9;
+  var array = new Array(len);
+  var array_proto = new Array();
+  array_proto[3] = "@3";
+  array_proto[7] = "@7";
+  array.__proto__ = array_proto;
+
+  assertEquals(len, array.length);
+  for (var i = 0; i < array.length; i++) {
+    assertEquals(array[i], array_proto[i]);
+  }
+
+  array.shift();
+
+  assertEquals(len - 1, array.length);
+  // Note that shift copies values from prototype into the array.
+  assertEquals(array[2], array_proto[3]);
+  assertTrue(array.hasOwnProperty(2));
+
+  assertEquals(array[6], array_proto[7]);
+  assertTrue(array.hasOwnProperty(6));
+
+  // ... but keeps the rest as holes:
+  array_proto[5] = "@5";
+  assertEquals(array[5], array_proto[5]);
+  assertFalse(array.hasOwnProperty(5));
+
+  assertEquals(array[3], array_proto[3]);
+  assertFalse(array.hasOwnProperty(3));
+
+  assertEquals(array[7], array_proto[7]);
+  assertFalse(array.hasOwnProperty(7));
+})();

test/mjsunit/array-slice.js

@@ -126,6 +126,53 @@
 })();
 
 
+// Now check the case with array of holes and some elements on prototype.
+// Note: that is important that this test runs before the next one
+// as the next one tampers Array.prototype.
+(function() {
+  var len = 9;
+  var array = new Array(len);
+
+  var at3 = "@3";
+  var at7 = "@7";
+
+  for (var i = 0; i < 7; i++) {
+    var array_proto = [];
+    array_proto[3] = at3;
+    array_proto[7] = at7;
+    array.__proto__ = array_proto;
+
+    assertEquals(len, array.length);
+    for (var i = 0; i < array.length; i++) {
+      assertEquals(array[i], array_proto[i]);
+    }
+
+    var sliced = array.slice();
+
+    assertEquals(len, sliced.length);
+
+    assertTrue(delete array_proto[3]);
+    assertTrue(delete array_proto[7]);
+
+    // Note that slice copies values from prototype into the array.
+    assertEquals(array[3], undefined);
+    assertFalse(array.hasOwnProperty(3));
+    assertEquals(sliced[3], at3);
+    assertTrue(sliced.hasOwnProperty(3));
+
+    assertEquals(array[7], undefined);
+    assertFalse(array.hasOwnProperty(7));
+    assertEquals(sliced[7], at7);
+    assertTrue(sliced.hasOwnProperty(7));
+
+    // ... but keeps the rest as holes:
+    array_proto[5] = "@5";
+    assertEquals(array[5], array_proto[5]);
+    assertFalse(array.hasOwnProperty(5));
+  }
+})();
+
+
 // Now check the case with array of holes and some elements on prototype.
 (function() {
   var len = 9;

test/mjsunit/array-splice.js

@@ -246,6 +246,56 @@
 })();
 
 
+// Now check the case with array of holes and some elements on prototype.
+(function() {
+  var len = 9;
+
+  var at3 = "@3";
+  var at7 = "@7";
+
+  for (var i = 0; i < 7; i++) {
+    var array = new Array(len);
+    var array_proto = [];
+    array_proto[3] = at3;
+    array_proto[7] = at7;
+    array.__proto__ = array_proto;
+
+    var spliced = array.splice(2, 2, 'one', undefined, 'two');
+
+    // Second hole (at index 3) of array turns into
+    // value of Array.prototype[3] while copying.
+    assertEquals([, at3], spliced);
+    assertEquals([, , 'one', undefined, 'two', , , at7, at7, ,], array);
+
+    // ... but array[3] and array[7] is actually a hole:
+    assertTrue(delete array_proto[3]);
+    assertEquals(undefined, array[3]);
+    assertTrue(delete array_proto[7]);
+    assertEquals(undefined, array[7]);
+
+    // and now check hasOwnProperty
+    assertFalse(array.hasOwnProperty(0), "array.hasOwnProperty(0)");
+    assertFalse(array.hasOwnProperty(1), "array.hasOwnProperty(1)");
+    assertTrue(array.hasOwnProperty(2));
+    assertTrue(array.hasOwnProperty(3));
+    assertTrue(array.hasOwnProperty(4));
+    assertFalse(array.hasOwnProperty(5), "array.hasOwnProperty(5)");
+    assertFalse(array.hasOwnProperty(6), "array.hasOwnProperty(6)");
+    assertFalse(array.hasOwnProperty(7), "array.hasOwnProperty(7)");
+    assertTrue(array.hasOwnProperty(8));
+    assertFalse(array.hasOwnProperty(9), "array.hasOwnProperty(9)");
+
+    // and now check couple of indices above length.
+    assertFalse(array.hasOwnProperty(10), "array.hasOwnProperty(10)");
+    assertFalse(array.hasOwnProperty(15), "array.hasOwnProperty(15)");
+    assertFalse(array.hasOwnProperty(31), "array.hasOwnProperty(31)");
+    assertFalse(array.hasOwnProperty(63), "array.hasOwnProperty(63)");
+    assertFalse(array.hasOwnProperty(2 << 32 - 1),
+                "array.hasOwnProperty(2 << 31 - 1)");
+  }
+})();
+
+
 // Now check the case with array of holes and some elements on prototype.
 (function() {
   var len = 9;
@@ -265,7 +315,9 @@
     assertEquals([, at3], spliced);
     assertEquals([, , 'one', undefined, 'two', , , at7, at7, ,], array);
 
-    // ... but array[7] is actually a hole:
+    // ... but array[3] and array[7] is actually a hole:
+    assertTrue(delete Array.prototype[3]);
+    assertEquals(undefined, array[3]);
     assertTrue(delete Array.prototype[7]);
     assertEquals(undefined, array[7]);
 
@@ -286,7 +338,8 @@
     assertFalse(array.hasOwnProperty(15), "array.hasOwnProperty(15)");
     assertFalse(array.hasOwnProperty(31), "array.hasOwnProperty(31)");
     assertFalse(array.hasOwnProperty(63), "array.hasOwnProperty(63)");
-    assertFalse(array.hasOwnProperty(2 << 32 - 1), "array.hasOwnProperty(2 << 31 - 1)");
+    assertFalse(array.hasOwnProperty(2 << 32 - 1),
+                "array.hasOwnProperty(2 << 31 - 1)");
   }
 })();
 

test/mjsunit/array-unshift.js

@@ -37,8 +37,8 @@
 })();
 
 
-// Check that unshif with no args has a side-effect of
-// feeling the holes with elements from the prototype
+// Check that unshift with no args has a side-effect of
+// filling the holes with elements from the prototype
 // (if present, of course)
 (function() {
   var len = 3;
@@ -115,6 +115,81 @@
   assertTrue(delete Array.prototype[7]);
 })();
 
+// Check that unshift with no args has a side-effect of
+// filling the holes with elements from the prototype
+// (if present, of course)
+(function() {
+  var len = 3;
+  var array = new Array(len);
+
+  var at0 = '@0';
+  var at2 = '@2';
+
+  var array_proto = [];
+  array_proto[0] = at0;
+  array_proto[2] = at2;
+  array.__proto__ = array_proto;
+
+  // array owns nothing...
+  assertFalse(array.hasOwnProperty(0));
+  assertFalse(array.hasOwnProperty(1));
+  assertFalse(array.hasOwnProperty(2));
+
+  // ... but sees values from array_proto.
+  assertEquals(array[0], at0);
+  assertEquals(array[1], undefined);
+  assertEquals(array[2], at2);
+
+  assertEquals(len, array.unshift());
+
+  // unshift makes array own 0 and 2...
+  assertTrue(array.hasOwnProperty(0));
+  assertFalse(array.hasOwnProperty(1));
+  assertTrue(array.hasOwnProperty(2));
+
+  // ... so they are not affected be delete.
+  assertEquals(array[0], at0);
+  assertEquals(array[1], undefined);
+  assertEquals(array[2], at2);
+})();
+
+
+// Now check the case with array of holes and some elements on prototype.
+(function() {
+  var len = 9;
+  var array = new Array(len);
+  var array_proto = []
+  array_proto[3] = "@3";
+  array_proto[7] = "@7";
+  array.__proto__ = array_proto;
+
+  assertEquals(len, array.length);
+  for (var i = 0; i < array.length; i++) {
+    assertEquals(array[i], array_proto[i]);
+  }
+
+  assertEquals(len + 1, array.unshift('head'));
+
+  assertEquals(len + 1, array.length);
+  // Note that unshift copies values from prototype into the array.
+  assertEquals(array[4], array_proto[3]);
+  assertTrue(array.hasOwnProperty(4));
+
+  assertEquals(array[8], array_proto[7]);
+  assertTrue(array.hasOwnProperty(8));
+
+  // ... but keeps the rest as holes:
+  array_proto[5] = "@5";
+  assertEquals(array[5], array_proto[5]);
+  assertFalse(array.hasOwnProperty(5));
+
+  assertEquals(array[3], array_proto[3]);
+  assertFalse(array.hasOwnProperty(3));
+
+  assertEquals(array[7], array_proto[7]);
+  assertFalse(array.hasOwnProperty(7));
+})();
+
 // Check the behaviour when approaching maximal values for length.
 (function() {
   for (var i = 0; i < 7; i++) {