Revision: 4649
Author: [email protected]
Date: Wed May 12 05:22:09 2010
Log: 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
http://code.google.com/p/v8/source/detail?r=4649
Modified:
/branches/bleeding_edge/src/builtins.cc
/branches/bleeding_edge/test/mjsunit/array-concat.js
/branches/bleeding_edge/test/mjsunit/array-pop.js
/branches/bleeding_edge/test/mjsunit/array-shift.js
/branches/bleeding_edge/test/mjsunit/array-slice.js
/branches/bleeding_edge/test/mjsunit/array-splice.js
/branches/bleeding_edge/test/mjsunit/array-unshift.js
=======================================
--- /branches/bleeding_edge/src/builtins.cc Wed May 5 05:25:58 2010
+++ /branches/bleeding_edge/src/builtins.cc Wed May 12 05:22:09 2010
@@ -330,22 +330,19 @@
}
-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;
}
@@ -366,6 +363,18 @@
*elements = FixedArray::cast(elms);
return true;
}
+
+
+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,
@@ -465,11 +474,7 @@
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(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(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(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(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(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 @@
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);
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-concat.js Wed Oct 29
03:02:09 2008
+++ /branches/bleeding_edge/test/mjsunit/array-concat.js Wed May 12
05:22:09 2010
@@ -29,9 +29,13 @@
* @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);
@@ -42,6 +46,77 @@
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);
+ 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';
@@ -91,7 +166,7 @@
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]));
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-pop.js Tue May 4 06:23:58
2010
+++ /branches/bleeding_edge/test/mjsunit/array-pop.js Wed May 12 05:22:09
2010
@@ -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.
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-shift.js Mon Feb 15 04:01:46
2010
+++ /branches/bleeding_edge/test/mjsunit/array-shift.js Wed May 12 05:22:09
2010
@@ -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));
+})();
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-slice.js Thu Mar 4 13:29:33
2010
+++ /branches/bleeding_edge/test/mjsunit/array-slice.js Wed May 12 05:22:09
2010
@@ -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;
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-splice.js Tue Mar 23
04:40:38 2010
+++ /branches/bleeding_edge/test/mjsunit/array-splice.js Wed May 12
05:22:09 2010
@@ -253,6 +253,56 @@
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;
+
+ var at3 = "@3";
+ var at7 = "@7";
+
for (var i = 0; i < 7; i++) {
var array = new Array(len);
Array.prototype[3] = at3;
@@ -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)");
}
})();
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-unshift.js Mon Mar 1
07:33:30 2010
+++ /branches/bleeding_edge/test/mjsunit/array-unshift.js Wed May 12
05:22:09 2010
@@ -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++) {
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
