Properly process arrays with overridden prototype in various Array's functions.
authorantonm@chromium.org <antonm@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 12 May 2010 12:22:09 +0000 (12:22 +0000)
committerantonm@chromium.org <antonm@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 12 May 2010 12:22:09 +0000 (12:22 +0000)
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
test/mjsunit/array-concat.js
test/mjsunit/array-pop.js
test/mjsunit/array-shift.js
test/mjsunit/array-slice.js
test/mjsunit/array-splice.js
test/mjsunit/array-unshift.js

index 4971275..9a0fbd2 100644 (file)
@@ -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);
     }
 
index 2346c8d..db89f4d 100644 (file)
  * @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]));
index a8d131e..f193f09 100644 (file)
     }
     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.
index d985b31..3601cbb 100644 (file)
   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));
+})();
index 30e9f3e..8f9ce53 100644 (file)
 
 
 // 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;
   var array = new Array(len);
index 887097d..88c4876 100644 (file)
 
   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;
+
+  var at3 = "@3";
+  var at7 = "@7";
+
+  for (var i = 0; i < 7; i++) {
+    var array = new Array(len);
     Array.prototype[3] = at3;
     Array.prototype[7] = at7;
 
     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]);
 
     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)");
   }
 })();
 
index dbe245b..c4cc95c 100644 (file)
@@ -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;
   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++) {