Fix for 435073: CHECK failure in CHECK(p->IsSmi()) failed.

The bug was an error when copying arrays in crankshaft. If it's a holey smi
array, the copy must be done as FAST_HOLEY_ELEMENTS to prevent representation
changes from being inserted that deopt on encountering the hole.

Also, prevent inlining array pop() and shift() if the length is read-only.

BUG=435073
LOG=N
R=verwaest@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#25455}

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 46a3810..2573a60 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -8367,6 +8367,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
       if (receiver_map.is_null()) return false;
       if (receiver_map->instance_type() != JS_ARRAY_TYPE) return false;
       ElementsKind elements_kind = receiver_map->elements_kind();
+      if (JSArray::IsReadOnlyLengthDescriptor(receiver_map)) return false;
       if (!IsFastElementsKind(elements_kind)) return false;
       if (receiver_map->is_observed()) return false;
       if (!receiver_map->is_extensible()) return false;
@@ -8484,6 +8485,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
       if (receiver_map.is_null()) return false;
       if (receiver_map->instance_type() != JS_ARRAY_TYPE) return false;
       ElementsKind kind = receiver_map->elements_kind();
+      if (JSArray::IsReadOnlyLengthDescriptor(receiver_map)) return false;
       if (!IsFastElementsKind(kind)) return false;
       if (receiver_map->is_observed()) return false;
       if (!receiver_map->is_extensible()) return false;
@@ -8556,10 +8558,12 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
                   graph()->GetConstant0(), new_length, Token::LT);
               HValue* key = AddUncasted<HAdd>(new_key, graph()->GetConstant1());
               key->ClearFlag(HValue::kCanOverflow);
+              ElementsKind copy_kind =
+                  kind == FAST_HOLEY_SMI_ELEMENTS ? FAST_HOLEY_ELEMENTS : kind;
               HValue* element = AddUncasted<HLoadKeyed>(
-                  elements, key, lengthiszero, kind, ALLOW_RETURN_HOLE);
-              HStoreKeyed* store = Add<HStoreKeyed>(
-                  elements, new_key, element, kind);
+                  elements, key, lengthiszero, copy_kind, ALLOW_RETURN_HOLE);
+              HStoreKeyed* store =
+                  Add<HStoreKeyed>(elements, new_key, element, copy_kind);
               store->SetFlag(HValue::kAllowUndefinedAsNaN);
             }
             loop.EndBody();
@@ -11407,11 +11411,13 @@ void HOptimizedGraphBuilder::BuildEmitFixedArray(
       site_context->ExitScope(current_site, value_object);
       Add<HStoreKeyed>(object_elements, key_constant, result, kind);
     } else {
-      HInstruction* value_instruction =
-          Add<HLoadKeyed>(boilerplate_elements, key_constant,
-                          static_cast<HValue*>(NULL), kind,
-                          ALLOW_RETURN_HOLE);
-      Add<HStoreKeyed>(object_elements, key_constant, value_instruction, kind);
+      ElementsKind copy_kind =
+          kind == FAST_HOLEY_SMI_ELEMENTS ? FAST_HOLEY_ELEMENTS : kind;
+      HInstruction* value_instruction = Add<HLoadKeyed>(
+          boilerplate_elements, key_constant, static_cast<HValue*>(NULL),
+          copy_kind, ALLOW_RETURN_HOLE);
+      Add<HStoreKeyed>(object_elements, key_constant, value_instruction,
+                       copy_kind);
     }
   }
 }

diff --git a/test/mjsunit/array-methods-read-only-length.js b/test/mjsunit/array-methods-read-only-length.js
index 925ef1f..2943b16 100644 (file)
--- a/test/mjsunit/array-methods-read-only-length.js
+++ b/test/mjsunit/array-methods-read-only-length.js
@@ -51,8 +51,7 @@ testAdd("fast properties");
 
 testAdd("normalized");
 
-function testRemove(mode) {
-  var a = [1, 2, 3];
+function testRemove(a, mode) {
   Object.defineProperty(a, "length", { writable : false});
 
   function check(f) {
@@ -91,11 +90,22 @@ function testRemove(mode) {
   check(splice);
   %OptimizeFunctionOnNextCall(splice);
   check(splice);
-}
 
-testRemove("fast properties");
+  %ClearFunctionTypeFeedback(pop);
+  %ClearFunctionTypeFeedback(shift);
+  %ClearFunctionTypeFeedback(splice);
+}
 
-testRemove("normalized");
+for (var i = 0; i < 3; i++) {
+  var a = [1, 2, 3];
+  if (i == 1) {
+    a = [1, 2, 3.5];
+  } else if (i == 2) {
+    a = [1, 2, "string"];
+  }
+  testRemove(a, "fast properties");
+  testRemove(a, "normalized");
+}
 
 var b = [];
 Object.defineProperty(b.__proto__, "0", {

diff --git a/test/mjsunit/array-shift4.js b/test/mjsunit/array-shift4.js
new file mode 100644 (file)
index 0000000..669b11a
--- /dev/null
+++ b/test/mjsunit/array-shift4.js
@@ -0,0 +1,24 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+// Inlining shift with holey smi arrays shouldn't deopt just because it
+// encounters the hole on the copy step.
+function doShift(a) {
+  var x = a.shift();
+  return x;
+}
+
+function makeArray() {
+  var a = [1, 2,, 3];
+  a[0] = 2;
+  return a;
+}
+
+doShift(makeArray());
+doShift(makeArray());
+%OptimizeFunctionOnNextCall(doShift);
+doShift(makeArray());
+assertOptimized(doShift);

diff --git a/test/mjsunit/regress/regress-435073.js b/test/mjsunit/regress/regress-435073.js
new file mode 100644 (file)
index 0000000..dbaa612
--- /dev/null
+++ b/test/mjsunit/regress/regress-435073.js
@@ -0,0 +1,12 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --verify-heap
+
+function test(x) { [x,,]; }
+
+test(0);
+test(0);
+%OptimizeFunctionOnNextCall(test);
+test(0);