Reviewers: danno,
Message:
Hi Danno, here is the fix we discussed. Perhaps the overflow check
is "overkill"
in the HStoreKeyed case? PTAL,
--Michael
Description:
Clusterfuzz identified overflow check needed in dehoisting.
BUG=380092
R=da...@chromium.org
LOG=N
Please review this at https://codereview.chromium.org/315593002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+28, -25 lines):
M src/hydrogen-dehoist.cc
M src/hydrogen-instructions.h
A + test/mjsunit/regress/regress-380092.js
Index: src/hydrogen-dehoist.cc
diff --git a/src/hydrogen-dehoist.cc b/src/hydrogen-dehoist.cc
index
e8008231ad3f22feaba64fbe9220bcd6c656a0ae..859b9eca8f39052be325de2467c90c8573786aa1
100644
--- a/src/hydrogen-dehoist.cc
+++ b/src/hydrogen-dehoist.cc
@@ -28,15 +28,17 @@ static void
DehoistArrayIndex(ArrayInstructionInterface* array_operation) {
if (!constant->HasInteger32Value()) return;
int32_t sign = binary_operation->IsSub() ? -1 : 1;
int32_t value = constant->Integer32Value() * sign;
- // We limit offset values to 30 bits because we want to avoid the risk of
- // overflows when the offset is added to the object header size.
- if (value >= 1 << array_operation->MaxBaseOffsetBits() || value < 0)
return;
+ if (value < 0) return;
+ uint32_t uvalue = static_cast<uint32_t>(value) <<
+ ElementsKindToShiftSize(array_operation->elements_kind());
+ // Protect against overflows when the offset is added to the object
header
+ // size.
+ if (!array_operation->CanIncreaseBaseOffset(uvalue)) return;
array_operation->SetKey(subexpression);
if (binary_operation->HasNoUses()) {
binary_operation->DeleteAndReplaceWith(NULL);
}
- value <<= ElementsKindToShiftSize(array_operation->elements_kind());
- array_operation->IncreaseBaseOffset(static_cast<uint32_t>(value));
+ array_operation->IncreaseBaseOffset(uvalue);
array_operation->SetDehoisted(true);
}
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index
05b31162af5730e4b83bb5844ed1aad40615f823..d106084a86c3f939267f02f4595ce26ca21f51f4
100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -6426,8 +6426,8 @@ class ArrayInstructionInterface {
virtual HValue* GetKey() = 0;
virtual void SetKey(HValue* key) = 0;
virtual ElementsKind elements_kind() const = 0;
- virtual void IncreaseBaseOffset(uint32_t base_offset) = 0;
- virtual int MaxBaseOffsetBits() = 0;
+ virtual bool CanIncreaseBaseOffset(uint32_t increase_by_value) = 0;
+ virtual void IncreaseBaseOffset(uint32_t increase_by_value) = 0;
virtual bool IsDehoisted() = 0;
virtual void SetDehoisted(bool is_dehoisted) = 0;
virtual ~ArrayInstructionInterface() { }
@@ -6474,12 +6474,13 @@ class HLoadKeyed V8_FINAL
}
bool HasDependency() const { return OperandAt(0) != OperandAt(2); }
uint32_t base_offset() { return BaseOffsetField::decode(bit_field_); }
- void IncreaseBaseOffset(uint32_t base_offset) {
- base_offset += BaseOffsetField::decode(bit_field_);
- bit_field_ = BaseOffsetField::update(bit_field_, base_offset);
+ bool CanIncreaseBaseOffset(uint32_t increase_by_value) {
+ increase_by_value += BaseOffsetField::decode(bit_field_);
+ return BaseOffsetField::is_valid(increase_by_value);
}
- virtual int MaxBaseOffsetBits() {
- return kBitsForBaseOffset;
+ void IncreaseBaseOffset(uint32_t increase_by_value) {
+ increase_by_value += BaseOffsetField::decode(bit_field_);
+ bit_field_ = BaseOffsetField::update(bit_field_, increase_by_value);
}
HValue* GetKey() { return key(); }
void SetKey(HValue* key) { SetOperandAt(1, key); }
@@ -6935,11 +6936,13 @@ class HStoreKeyed V8_FINAL
StoreFieldOrKeyedMode store_mode() const { return store_mode_; }
ElementsKind elements_kind() const { return elements_kind_; }
uint32_t base_offset() { return base_offset_; }
- void IncreaseBaseOffset(uint32_t base_offset) {
- base_offset_ += base_offset;
+ bool CanIncreaseBaseOffset(uint32_t increase_by_value) {
+ // unsigned integer addition "wraps around."
+ uint32_t result = increase_by_value + base_offset_;
+ return result >= base_offset_ && result >= increase_by_value;
}
- virtual int MaxBaseOffsetBits() {
- return 31 - ElementsKindToShiftSize(elements_kind_);
+ void IncreaseBaseOffset(uint32_t increase_by_value) {
+ base_offset_ += increase_by_value;
}
HValue* GetKey() { return key(); }
void SetKey(HValue* key) { SetOperandAt(1, key); }
Index: test/mjsunit/regress/regress-380092.js
diff --git a/test/mjsunit/regress/regress-alloc-smi-check.js
b/test/mjsunit/regress/regress-380092.js
similarity index 56%
copy from test/mjsunit/regress/regress-alloc-smi-check.js
copy to test/mjsunit/regress/regress-380092.js
index
295048a13ef862ceb21939de104e7968dd7772da..a7634355ca5d0db9a867cb691b05129b984911f8
100644
--- a/test/mjsunit/regress/regress-alloc-smi-check.js
+++ b/test/mjsunit/regress/regress-380092.js
@@ -1,16 +1,14 @@
// 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
-var x = {};
+// Flags: --allow-natives-syntax
-function f(a) {
- a[200000000] = x;
+function many_hoist(o, index) {
+ return o[index + 33554427];
}
-f(new Array(100000));
-f([]);
-%OptimizeFunctionOnNextCall(f);
-f([]);
+var obj = {};
+many_hoist(obj, 0);
+%OptimizeFunctionOnNextCall(many_hoist);
+many_hoist(obj, 5);
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
