Revision: 18164
Author: [email protected]
Date: Fri Nov 29 15:22:16 2013 UTC
Log: Array builtins need to be prevented from changing frozen objects,
and changing structure on sealed objects.
BUG=299979
LOG=Y
[email protected]
Review URL: https://codereview.chromium.org/80623002
http://code.google.com/p/v8/source/detail?r=18164
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-299979.js
Modified:
/branches/bleeding_edge/src/arm/stub-cache-arm.cc
/branches/bleeding_edge/src/array.js
/branches/bleeding_edge/src/builtins.cc
/branches/bleeding_edge/src/ia32/stub-cache-ia32.cc
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/src/mips/stub-cache-mips.cc
/branches/bleeding_edge/src/objects-printer.cc
/branches/bleeding_edge/src/x64/stub-cache-x64.cc
/branches/bleeding_edge/test/mjsunit/object-freeze.js
/branches/bleeding_edge/test/mjsunit/object-seal.js
/branches/bleeding_edge/test/mjsunit/regress/regress-2711.js
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-299979.js Fri Nov
29 15:22:16 2013 UTC
@@ -0,0 +1,34 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+(function(){
+ "use strict";
+ var list = Object.freeze([1, 2, 3]);
+ assertThrows(function() { list.unshift(4); }, TypeError);
+ assertThrows(function() { list.shift(); }, TypeError);
+})();
=======================================
--- /branches/bleeding_edge/src/arm/stub-cache-arm.cc Fri Nov 29 12:57:47
2013 UTC
+++ /branches/bleeding_edge/src/arm/stub-cache-arm.cc Fri Nov 29 15:22:16
2013 UTC
@@ -1611,10 +1611,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
@@ -1854,10 +1856,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
=======================================
--- /branches/bleeding_edge/src/array.js Fri Oct 25 11:55:56 2013 UTC
+++ /branches/bleeding_edge/src/array.js Fri Nov 29 15:22:16 2013 UTC
@@ -424,6 +424,11 @@
this.length = n;
return;
}
+
+ if ($Object.isSealed(this)) {
+ throw MakeTypeError("array_functions_change_sealed",
+ ["Array.prototype.pop"]);
+ }
if (%IsObserved(this))
return ObservedArrayPop.call(this, n);
@@ -461,12 +466,17 @@
throw MakeTypeError("called_on_null_or_undefined",
["Array.prototype.push"]);
}
+
+ var n = TO_UINT32(this.length);
+ var m = %_ArgumentsLength();
+ if (m > 0 && $Object.isSealed(this)) {
+ throw MakeTypeError("array_functions_change_sealed",
+ ["Array.prototype.push"]);
+ }
if (%IsObserved(this))
return ObservedArrayPush.apply(this, arguments);
- var n = TO_UINT32(this.length);
- var m = %_ArgumentsLength();
for (var i = 0; i < m; i++) {
this[i+n] = %_Arguments(i);
}
@@ -603,6 +613,11 @@
this.length = 0;
return;
}
+
+ if ($Object.isSealed(this)) {
+ throw MakeTypeError("array_functions_change_sealed",
+ ["Array.prototype.shift"]);
+ }
if (%IsObserved(this))
return ObservedArrayShift.call(this, len);
@@ -644,16 +659,33 @@
throw MakeTypeError("called_on_null_or_undefined",
["Array.prototype.unshift"]);
}
-
- if (%IsObserved(this))
- return ObservedArrayUnshift.apply(this, arguments);
var len = TO_UINT32(this.length);
var num_arguments = %_ArgumentsLength();
+ var is_sealed = $Object.isSealed(this);
- if (IS_ARRAY(this)) {
+ if (num_arguments > 0 && is_sealed) {
+ throw MakeTypeError("array_functions_change_sealed",
+ ["Array.prototype.unshift"]);
+ }
+
+ if (%IsObserved(this))
+ return ObservedArrayUnshift.apply(this, arguments);
+
+ if (IS_ARRAY(this) && !is_sealed) {
SmartMove(this, 0, 0, len, num_arguments);
} else {
+ if (num_arguments == 0 && $Object.isFrozen(this)) {
+ // In the zero argument case, values from the prototype come into the
+ // object. This can't be allowed on frozen arrays.
+ for (var i = 0; i < len; i++) {
+ if (!this.hasOwnProperty(i) && !IS_UNDEFINED(this[i])) {
+ throw MakeTypeError("array_functions_on_frozen",
+ ["Array.prototype.shift"]);
+ }
+ }
+ }
+
SimpleMove(this, 0, 0, len, num_arguments);
}
@@ -663,7 +695,7 @@
this.length = len + num_arguments;
- return len + num_arguments;
+ return this.length;
}
@@ -802,6 +834,14 @@
deleted_elements.length = del_count;
var num_elements_to_add = num_arguments > 2 ? num_arguments - 2 : 0;
+ if (del_count != num_elements_to_add && $Object.isSealed(this)) {
+ throw MakeTypeError("array_functions_change_sealed",
+ ["Array.prototype.splice"]);
+ } else if (del_count > 0 && $Object.isFrozen(this)) {
+ throw MakeTypeError("array_functions_on_frozen",
+ ["Array.prototype.splice"]);
+ }
+
var use_simple_splice = true;
if (IS_ARRAY(this) &&
num_elements_to_add !== del_count) {
=======================================
--- /branches/bleeding_edge/src/builtins.cc Fri Nov 29 09:54:38 2013 UTC
+++ /branches/bleeding_edge/src/builtins.cc Fri Nov 29 15:22:16 2013 UTC
@@ -307,6 +307,7 @@
if (!receiver->IsJSArray()) return NULL;
JSArray* array = JSArray::cast(receiver);
if (array->map()->is_observed()) return NULL;
+ if (!array->map()->is_extensible()) return NULL;
HeapObject* elms = array->elements();
Map* map = elms->map();
if (map == heap->fixed_array_map()) {
=======================================
--- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Fri Nov 29 12:57:47
2013 UTC
+++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Fri Nov 29 15:22:16
2013 UTC
@@ -1707,10 +1707,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
@@ -1950,10 +1952,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
=======================================
--- /branches/bleeding_edge/src/messages.js Thu Nov 28 08:59:45 2013 UTC
+++ /branches/bleeding_edge/src/messages.js Fri Nov 29 15:22:16 2013 UTC
@@ -111,6 +111,8 @@
constructor_not_function: ["Constructor ", "%0", " requires 'new'"],
not_a_promise: ["%0", "is not a promise"],
promise_cyclic: ["Chaining cycle detected for
promise", "%0"],
+ array_functions_on_frozen: ["Cannot modify frozen array elements"],
+ array_functions_change_sealed: ["Cannot add/remove sealed array
elements"],
// RangeError
invalid_array_length: ["Invalid array length"],
invalid_array_buffer_length: ["Invalid array buffer length"],
=======================================
--- /branches/bleeding_edge/src/mips/stub-cache-mips.cc Fri Nov 29 12:57:47
2013 UTC
+++ /branches/bleeding_edge/src/mips/stub-cache-mips.cc Fri Nov 29 15:22:16
2013 UTC
@@ -1597,10 +1597,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
@@ -1839,10 +1841,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
=======================================
--- /branches/bleeding_edge/src/objects-printer.cc Mon Nov 25 12:19:02 2013
UTC
+++ /branches/bleeding_edge/src/objects-printer.cc Fri Nov 29 15:22:16 2013
UTC
@@ -556,6 +556,11 @@
if (is_access_check_needed()) {
PrintF(out, " - access_check_needed\n");
}
+ if (is_frozen()) {
+ PrintF(out, " - frozen\n");
+ } else if (!is_extensible()) {
+ PrintF(out, " - sealed\n");
+ }
PrintF(out, " - back pointer: ");
GetBackPointer()->ShortPrint(out);
PrintF(out, "\n - instance descriptors %s#%i: ",
=======================================
--- /branches/bleeding_edge/src/x64/stub-cache-x64.cc Fri Nov 29 12:57:47
2013 UTC
+++ /branches/bleeding_edge/src/x64/stub-cache-x64.cc Fri Nov 29 15:22:16
2013 UTC
@@ -1637,18 +1637,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // ----------- S t a t e -------------
- // -- rcx : name
- // -- rsp[0] : return address
- // -- rsp[(argc - n) * 8] : arg[n] (zero-based)
- // -- ...
- // -- rsp[(argc + 1) * 8] : receiver
- // -----------------------------------
-
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
@@ -1884,10 +1878,12 @@
Handle<JSFunction> function,
Handle<String> name,
Code::StubType type) {
- // If object is not an array or is observed, bail out to regular call.
+ // If object is not an array or is observed or sealed, bail out to
regular
+ // call.
if (!object->IsJSArray() ||
!cell.is_null() ||
- Handle<JSArray>::cast(object)->map()->is_observed()) {
+ Handle<JSArray>::cast(object)->map()->is_observed() ||
+ !Handle<JSArray>::cast(object)->map()->is_extensible()) {
return Handle<Code>::null();
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/object-freeze.js Mon May 27
10:56:27 2013 UTC
+++ /branches/bleeding_edge/test/mjsunit/object-freeze.js Fri Nov 29
15:22:16 2013 UTC
@@ -314,3 +314,26 @@
Object.freeze(obj);
assertTrue(%HasFastProperties(obj));
assertTrue(Object.isFrozen(obj));
+
+// Test array built-in functions with freeze.
+obj = [1,2,3];
+Object.freeze(obj);
+// if frozen implies sealed, then the tests in object-seal.js are mostly
+// sufficient.
+assertTrue(Object.isSealed(obj));
+
+assertDoesNotThrow(function() { obj.push(); });
+assertDoesNotThrow(function() { obj.unshift(); });
+assertDoesNotThrow(function() { obj.splice(0,0); });
+assertTrue(Object.isFrozen(obj));
+
+// Verify that an item can't be changed with splice.
+assertThrows(function() { obj.splice(0,1,1); }, TypeError);
+
+// Verify that unshift() with no arguments will fail if it reifies from
+// the prototype into the object.
+obj = [1,,3];
+obj.__proto__[1] = 1;
+assertEquals(1, obj[1]);
+Object.freeze(obj);
+assertThrows(function() { obj.unshift(); }, TypeError);
=======================================
--- /branches/bleeding_edge/test/mjsunit/object-seal.js Tue May 31 08:08:42
2011 UTC
+++ /branches/bleeding_edge/test/mjsunit/object-seal.js Fri Nov 29 15:22:16
2013 UTC
@@ -28,6 +28,7 @@
// Tests the Object.seal and Object.isSealed methods - ES 15.2.3.9 and
// ES 15.2.3.12
+// Flags: --allow-natives-syntax --noalways-opt
// Test that we throw an error if an object is not passed as argument.
var non_objects = new Array(undefined, null, 1, -1, 0, 42.43);
@@ -192,3 +193,78 @@
// Make sure that Object.seal returns the sealed object.
var obj4 = {};
assertTrue(obj4 === Object.seal(obj4));
+
+//
+// Test that built-in array functions can't modify a sealed array.
+//
+obj = [1, 2, 3];
+var objControl = [4, 5, 6];
+
+// Allow these functions to set up monomorphic calls, using custom
built-ins.
+var push_call = function(a) { a.push(10); return a; }
+var pop_call = function(a) { return a.pop(); }
+for (var i = 0; i < 3; i++) {
+ push_call(obj);
+ pop_call(obj);
+}
+
+Object.seal(obj);
+assertThrows(function() { push_call(obj); }, TypeError);
+assertThrows(function() { pop_call(obj); }, TypeError);
+
+// But the control object is fine at these sites.
+assertDoesNotThrow(function() { push_call(objControl); });
+assertDoesNotThrow(function() { pop_call(objControl); });
+
+assertDoesNotThrow(function() { obj.push(); });
+assertThrows(function() { obj.push(3); }, TypeError);
+assertThrows(function() { obj.pop(); }, TypeError);
+assertThrows(function() { obj.shift(3); }, TypeError);
+assertDoesNotThrow(function() { obj.unshift(); });
+assertThrows(function() { obj.unshift(1); }, TypeError);
+assertThrows(function() { obj.splice(0, 0, 100, 101, 102); }, TypeError);
+assertDoesNotThrow(function() { obj.splice(0,0); });
+
+assertDoesNotThrow(function() { objControl.push(3); });
+assertDoesNotThrow(function() { objControl.pop(); });
+assertDoesNotThrow(function() { objControl.shift(3); });
+assertDoesNotThrow(function() { objControl.unshift(); });
+assertDoesNotThrow(function() { objControl.splice(0, 0, 100, 101, 102); });
+
+// Verify that crankshaft still does the right thing.
+obj = [1, 2, 3];
+
+push_call = function(a) { a.push(1000); return a; }
+// Include a call site that doesn't have a custom built-in.
+var shift_call = function(a) { a.shift(1000); return a; }
+for (var i = 0; i < 3; i++) {
+ push_call(obj);
+ shift_call(obj);
+}
+
+%OptimizeFunctionOnNextCall(push_call);
+%OptimizeFunctionOnNextCall(shift_call);
+push_call(obj);
+shift_call(obj);
+assertOptimized(push_call);
+assertOptimized(shift_call);
+Object.seal(obj);
+assertThrows(function() { push_call(obj); }, TypeError);
+assertThrows(function() { shift_call(obj); }, TypeError);
+assertOptimized(push_call);
+// shift() doesn't have a custom call generator, so deopt will occur.
+assertUnoptimized(shift_call);
+assertDoesNotThrow(function() { push_call(objControl); });
+assertDoesNotThrow(function() { shift_call(objControl); });
+
+// Verify special behavior of splice on sealed objects.
+obj = [1,2,3];
+Object.seal(obj);
+assertDoesNotThrow(function() { obj.splice(0,1,100); });
+assertEquals(100, obj[0]);
+assertDoesNotThrow(function() { obj.splice(0,2,1,2); });
+assertDoesNotThrow(function() { obj.splice(1,2,1,2); });
+// Count of items to delete is clamped by length.
+assertDoesNotThrow(function() { obj.splice(1,2000,1,2); });
+assertThrows(function() { obj.splice(0,0,1); }, TypeError);
+assertThrows(function() { obj.splice(1,2000,1,2,3); }, TypeError);
=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-2711.js Sun Jul 14
22:03:46 2013 UTC
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-2711.js Fri Nov 29
15:22:16 2013 UTC
@@ -27,7 +27,7 @@
// Test that frozen arrays don't let their length change
var a = Object.freeze([1]);
-a.push(2);
+assertThrows(function() { a.push(2); }, TypeError);
assertEquals(1, a.length);
-a.push(2);
+assertThrows(function() { a.push(2); }, TypeError);
assertEquals(1, a.length);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
