Bugfix for 349874: we incorrectly believe we saw a growing store

When we set an out of bounds array index, the index might be so large that
it causes the array to go to dictionary mode. It's better to avoid
"learning" that this was a growing store in that case.

This fix also partially reverts a fix for bug 347543, as this fix is
comprehensive and satisfies that repro case as well (partial revert of
v19591).

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

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

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

diff --git a/src/a64/lithium-codegen-a64.cc b/src/a64/lithium-codegen-a64.cc
index 0148349..7a93754 100644 (file)
--- a/src/a64/lithium-codegen-a64.cc
+++ b/src/a64/lithium-codegen-a64.cc
@@ -1489,11 +1489,7 @@ void LCodeGen::DoAllocate(LAllocate* instr) {
 
   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    if (size <= Page::kMaxRegularHeapObjectSize) {
-      __ Allocate(size, result, temp1, temp2, deferred->entry(), flags);
-    } else {
-      __ B(deferred->entry());
-    }
+    __ Allocate(size, result, temp1, temp2, deferred->entry(), flags);
   } else {
     Register size = ToRegister32(instr->size());
     __ Sxtw(size.X(), size);

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index dd012af..2edf4fc 100644 (file)
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -5262,11 +5262,7 @@ void LCodeGen::DoAllocate(LAllocate* instr) {
 
   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    if (size <= Page::kMaxRegularHeapObjectSize) {
-      __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
-    } else {
-      __ jmp(deferred->entry());
-    }
+    __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size,

diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 3e8224b..54bc338 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -5789,11 +5789,7 @@ void LCodeGen::DoAllocate(LAllocate* instr) {
 
   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    if (size <= Page::kMaxRegularHeapObjectSize) {
-      __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
-    } else {
-      __ jmp(deferred->entry());
-    }
+    __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);

diff --git a/src/ic.cc b/src/ic.cc
index 3c9f1b6..a5a7ba2 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1701,7 +1701,12 @@ MaybeObject* KeyedStoreIC::Store(Handle<Object> object,
           if (!(receiver->map()->DictionaryElementsInPrototypeChainOnly())) {
             KeyedAccessStoreMode store_mode =
                 GetStoreMode(receiver, key, value);
-            stub = StoreElementStub(receiver, store_mode);
+            // Use the generic stub if the store would send the receiver to
+            // dictionary mode.
+            if (!IsGrowStoreMode(store_mode) ||
+                !receiver->WouldConvertToSlowElements(key)) {
+              stub = StoreElementStub(receiver, store_mode);
+            }
           }
         }
       }

diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc
index b9607de..72f3296 100644 (file)
--- a/src/mips/lithium-codegen-mips.cc
+++ b/src/mips/lithium-codegen-mips.cc
@@ -5217,11 +5217,7 @@ void LCodeGen::DoAllocate(LAllocate* instr) {
   }
   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    if (size <= Page::kMaxRegularHeapObjectSize) {
-      __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
-    } else {
-      __ jmp(deferred->entry());
-    }
+    __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size,

diff --git a/src/objects.cc b/src/objects.cc
index 2a4cfe3..d06f916 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -13113,6 +13113,21 @@ void JSObject::GetElementsCapacityAndUsage(int* capacity, int* used) {
 }
 
 
+bool JSObject::WouldConvertToSlowElements(Handle<Object> key) {
+  uint32_t index;
+  if (HasFastElements() && key->ToArrayIndex(&index)) {
+    Handle<FixedArrayBase> backing_store(FixedArrayBase::cast(elements()));
+    uint32_t capacity = static_cast<uint32_t>(backing_store->length());
+    if (index >= capacity) {
+      if ((index - capacity) >= kMaxGap) return true;
+      uint32_t new_capacity = NewElementsCapacity(index + 1);
+      return ShouldConvertToSlowElements(new_capacity);
+    }
+  }
+  return false;
+}
+
+
 bool JSObject::ShouldConvertToSlowElements(int new_capacity) {
   STATIC_ASSERT(kMaxUncheckedOldFastElementsLength <=
                 kMaxUncheckedFastElementsLength);

diff --git a/src/objects.h b/src/objects.h
index f90662f..17a60fb 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -2412,6 +2412,9 @@ class JSObject: public JSReceiver {
       uint32_t arg_count,
       EnsureElementsMode mode);
 
+  // Would we convert a fast elements array to dictionary mode given
+  // an access at key?
+  bool WouldConvertToSlowElements(Handle<Object> key);
   // Do we want to keep the elements in fast case when increasing the
   // capacity?
   bool ShouldConvertToSlowElements(int new_capacity);

diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index 36ebcad..383cf37 100644 (file)
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -5085,11 +5085,7 @@ void LCodeGen::DoAllocate(LAllocate* instr) {
 
   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    if (size <= Page::kMaxRegularHeapObjectSize) {
-      __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
-    } else {
-      __ jmp(deferred->entry());
-    }
+    __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);

diff --git a/test/mjsunit/regress/regress-349885.js b/test/mjsunit/regress/regress-349885.js
new file mode 100644 (file)
index 0000000..dd3e795
--- /dev/null
+++ b/test/mjsunit/regress/regress-349885.js
@@ -0,0 +1,15 @@
+// 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
+
+// The bug 349885
+
+function foo(a) {
+  a[292755462] = new Object();
+}
+foo(new Array(5));
+foo(new Array(5));
+%OptimizeFunctionOnNextCall(foo);
+foo(new Array(10));