Fix Array.prototype.push and Array.prototype.unshift for read-only length.

src/builtins.cc

Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index d0c1a446a8beb9e80e5ddce6fab385d74468363c..cbbb447951303b488fd157b5cf4199c1bb7bfc11 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -382,15 +382,19 @@ BUILTIN(ArrayPush) {
   }
 
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
+  int len = Smi::cast(array->length())->value();
+  int to_add = args.length() - 1;
+  if (to_add > 0 && JSArray::WouldChangeReadOnlyLength(array, len + to_add)) {
+    RETURN_FAILURE_ON_EXCEPTION(
+        isolate,
+        JSArray::ReadOnlyLengthError(array));

[Michael Starzinger, 2014/05/14 08:30:48]

As discussed offline: Instead of handling this corner-case here in the optimized
version, could we instead call out to the JS builtin reference implementation
instead? This would make the optimized path simpler.

[ulan, 2014/05/15 11:15:54]

Replaced RETURN_FAILURE_ON_EXCEPTION(..) with call to JS builtin. I thought
about simplifying the condition as well (only checking for read-only length),
but left it unchanged because it allows to avoid length lookup when the index is
within bounds.

+  }
   ASSERT(!array->map()->is_observed());
 
   ElementsKind kind = array->GetElementsKind();
 
   if (IsFastSmiOrObjectElementsKind(kind)) {
     Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
-
-    int len = Smi::cast(array->length())->value();
-    int to_add = args.length() - 1;
     if (to_add == 0) {
       return Smi::FromInt(len);
     }
@@ -429,10 +433,7 @@ BUILTIN(ArrayPush) {
     array->set_length(Smi::FromInt(new_length));
     return Smi::FromInt(new_length);
   } else {
-    int len = Smi::cast(array->length())->value();
     int elms_len = elms_obj->length();
-
-    int to_add = args.length() - 1;
     if (to_add == 0) {
       return Smi::FromInt(len);
     }
@@ -587,6 +588,12 @@ BUILTIN(ArrayUnshift) {
   // we should never hit this case.
   ASSERT(to_add <= (Smi::kMaxValue - len));
 
+  if (to_add > 0 && JSArray::WouldChangeReadOnlyLength(array, len + to_add)) {
+    RETURN_FAILURE_ON_EXCEPTION(
+        isolate,
+        JSArray::ReadOnlyLengthError(array));

[Michael Starzinger, 2014/05/14 08:30:48]

Likewise.

+  }
+
   JSObject::EnsureCanContainElements(array, &args, 1, to_add,
                                      DONT_ALLOW_DOUBLE_ELEMENTS);