Revision: 19691
Author:   [email protected]
Date:     Thu Mar  6 13:07:51 2014 UTC
Log:      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
[email protected]

Review URL: https://codereview.chromium.org/188643002
http://code.google.com/p/v8/source/detail?r=19691

Added:
 /branches/bleeding_edge/test/mjsunit/regress/regress-349885.js
Modified:
 /branches/bleeding_edge/src/a64/lithium-codegen-a64.cc
 /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
 /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
 /branches/bleeding_edge/src/ic.cc
 /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/objects.h
 /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc

=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-349885.js Thu Mar 6 13:07:51 2014 UTC
@@ -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));
=======================================
--- /branches/bleeding_edge/src/a64/lithium-codegen-a64.cc Wed Mar 5 12:45:46 2014 UTC +++ /branches/bleeding_edge/src/a64/lithium-codegen-a64.cc Thu Mar 6 13:07:51 2014 UTC
@@ -1489,11 +1489,7 @@

   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);
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Wed Mar 5 12:45:46 2014 UTC +++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Thu Mar 6 13:07:51 2014 UTC
@@ -5262,11 +5262,7 @@

   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,
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Wed Mar 5 12:45:46 2014 UTC +++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Thu Mar 6 13:07:51 2014 UTC
@@ -5789,11 +5789,7 @@

   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);
=======================================
--- /branches/bleeding_edge/src/ic.cc   Thu Mar  6 12:19:06 2014 UTC
+++ /branches/bleeding_edge/src/ic.cc   Thu Mar  6 13:07:51 2014 UTC
@@ -1701,7 +1701,12 @@
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);
+            }
           }
         }
       }
=======================================
--- /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Wed Mar 5 12:45:46 2014 UTC +++ /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Thu Mar 6 13:07:51 2014 UTC
@@ -5217,11 +5217,7 @@
   }
   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,
=======================================
--- /branches/bleeding_edge/src/objects.cc      Wed Mar  5 10:07:07 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc      Thu Mar  6 13:07:51 2014 UTC
@@ -13111,6 +13111,21 @@
     }
   }
 }
+
+
+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) {
=======================================
--- /branches/bleeding_edge/src/objects.h       Mon Mar  3 11:11:39 2014 UTC
+++ /branches/bleeding_edge/src/objects.h       Thu Mar  6 13:07:51 2014 UTC
@@ -2412,6 +2412,9 @@
       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);
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Wed Mar 5 12:45:46 2014 UTC +++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Thu Mar 6 13:07:51 2014 UTC
@@ -5085,11 +5085,7 @@

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

--
--
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.

Reply via email to