Array builtins need to be prevented from changing frozen objects, and changing...

Array builtins need to be prevented from changing frozen objects, and changing structure on sealed objects.

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

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

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

src/arm/stub-cache-arm.cc

@@ -1611,10 +1611,12 @@ Handle<Code> CallStubCompiler::CompileArrayPushCall(
     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<Code> CallStubCompiler::CompileArrayPopCall(
     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();
   }
 

src/array.js

@@ -425,6 +425,11 @@ function ArrayPop() {
     return;
   }
 
+  if ($Object.isSealed(this)) {
+    throw MakeTypeError("array_functions_change_sealed",
+                        ["Array.prototype.pop"]);
+  }
+
   if (%IsObserved(this))
     return ObservedArrayPop.call(this, n);
 
@@ -462,11 +467,16 @@ function ArrayPush() {
                         ["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);
   }
@@ -604,6 +614,11 @@ function ArrayShift() {
     return;
   }
 
+  if ($Object.isSealed(this)) {
+    throw MakeTypeError("array_functions_change_sealed",
+                        ["Array.prototype.shift"]);
+  }
+
   if (%IsObserved(this))
     return ObservedArrayShift.call(this, len);
 
@@ -645,15 +660,32 @@ function ArrayUnshift(arg1) {  // length == 1
                         ["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 @@ function ArrayUnshift(arg1) {  // length == 1
 
   this.length = len + num_arguments;
 
-  return len + num_arguments;
+  return this.length;
 }
 
 
@@ -802,6 +834,14 @@ function ArraySplice(start, delete_count) {
   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) {

src/builtins.cc

@@ -307,6 +307,7 @@ static inline MaybeObject* EnsureJSArrayWithWritableFastElements(
   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()) {

src/ia32/stub-cache-ia32.cc

@@ -1707,10 +1707,12 @@ Handle<Code> CallStubCompiler::CompileArrayPushCall(
     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<Code> CallStubCompiler::CompileArrayPopCall(
     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();
   }
 

src/messages.js

@@ -111,6 +111,8 @@ var kMessages = {
   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"],

src/mips/stub-cache-mips.cc

@@ -1597,10 +1597,12 @@ Handle<Code> CallStubCompiler::CompileArrayPushCall(
     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<Code> CallStubCompiler::CompileArrayPopCall(
     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();
   }
 

src/objects-printer.cc

@@ -556,6 +556,11 @@ void Map::MapPrint(FILE* out) {
   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: ",

src/x64/stub-cache-x64.cc

@@ -1637,18 +1637,12 @@ Handle<Code> CallStubCompiler::CompileArrayPushCall(
     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<Code> CallStubCompiler::CompileArrayPopCall(
     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();
   }
 

test/mjsunit/object-freeze.js

@@ -314,3 +314,26 @@ assertTrue(%HasFastProperties(obj));
 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);

test/mjsunit/object-seal.js

@@ -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 @@ assertFalse(Object.isSealed(obj4));
 // 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);

test/mjsunit/regress/regress-2711.js

@@ -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);

test/mjsunit/regress/regress-299979.js

+// 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);
+})();