Reviewers: Toon Verwaest,
Message:
Hi Toon,
Here is what we discussed, PTAL,
--Michael
Description:
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]
Please review this at https://codereview.chromium.org/188643002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+37, -30 lines):
M src/a64/lithium-codegen-a64.cc
M src/arm/lithium-codegen-arm.cc
M src/ia32/lithium-codegen-ia32.cc
M src/ic.cc
M src/mips/lithium-codegen-mips.cc
M src/objects.h
M src/objects.cc
M src/x64/lithium-codegen-x64.cc
A + test/mjsunit/regress/regress-349885.js
Index: src/a64/lithium-codegen-a64.cc
diff --git a/src/a64/lithium-codegen-a64.cc b/src/a64/lithium-codegen-a64.cc
index
0148349a14c7c5838028b733dc575739fd242484..7a93754857cc472707ffbda55f8fb74bab1ceb85
100644
--- 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);
Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index
dd012af579194dafb2aaa5c733c25dccc6ed7718..2edf4fcefb955ce3025963d5859d8aff8e5573aa
100644
--- 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,
Index: src/ia32/lithium-codegen-ia32.cc
diff --git a/src/ia32/lithium-codegen-ia32.cc
b/src/ia32/lithium-codegen-ia32.cc
index
3e8224b24f95f0bf1179e67c82fd82ac2b867b68..54bc3382e967bf0c468faf36237d812cbbe4a371
100644
--- 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);
Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index
2607bb3b68d53405be03865fe60538dea4e8b9e0..61408186ac73cd409a1b633eecd6ce258819f4c7
100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1699,7 +1699,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);
+ }
}
}
}
Index: src/mips/lithium-codegen-mips.cc
diff --git a/src/mips/lithium-codegen-mips.cc
b/src/mips/lithium-codegen-mips.cc
index
b9607dea7d212ae4b5b15cb4c2009430d36ff597..72f3296f54cdeca539acccdb6c158bb6ef6e03cb
100644
--- 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,
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
2a4cfe30a6a82077faff6a364332911e6f25bb44..d06f916d2053aecb2c223f0e312f72f455e06728
100644
--- 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);
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
f90662fc6c03e84109486bc83c794ae9ddb2493a..17a60fb4e2e81a9a88cca22f5388c9e3530de461
100644
--- 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);
Index: src/x64/lithium-codegen-x64.cc
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index
36ebcad53c08acd971bc46ef879b15df5787ef18..383cf3783f25892de0f195232babf6d1f697cd3c
100644
--- 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);
Index: test/mjsunit/regress/regress-349885.js
diff --git a/test/mjsunit/regress/regress-347542.js
b/test/mjsunit/regress/regress-349885.js
similarity index 64%
copy from test/mjsunit/regress/regress-347542.js
copy to test/mjsunit/regress/regress-349885.js
index
901d798fb7fbea45f0d9f3d8ba6c7a9846bf6dd6..dd3e79526041b0362434608b81429d83e18ae8ef
100644
--- a/test/mjsunit/regress/regress-347542.js
+++ b/test/mjsunit/regress/regress-349885.js
@@ -4,8 +4,12 @@
// Flags: --allow-natives-syntax
-function foo() {}
-foo();
+// The bug 349885
+
+function foo(a) {
+ a[292755462] = new Object();
+}
+foo(new Array(5));
+foo(new Array(5));
%OptimizeFunctionOnNextCall(foo);
-foo();
-%NeverOptimizeFunction(foo);
+foo(new Array(10));
--
--
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.
