Revision: 21920
Author: [email protected]
Date: Mon Jun 23 09:09:05 2014 UTC
Log: Re-land "Clusterfuzz identified overflow check needed in
dehoisting."
BUG=380092
LOG=N
[email protected]
Review URL: https://codereview.chromium.org/335063005
http://code.google.com/p/v8/source/detail?r=21920
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-380092.js
Modified:
/branches/bleeding_edge/src/hydrogen-dehoist.cc
/branches/bleeding_edge/src/hydrogen-instructions.cc
/branches/bleeding_edge/src/hydrogen-instructions.h
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-380092.js Mon Jun
23 09:09:05 2014 UTC
@@ -0,0 +1,22 @@
+// 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
+
+function many_hoist(o, index) {
+ return o[index + 33554427];
+}
+
+var obj = {};
+many_hoist(obj, 0);
+%OptimizeFunctionOnNextCall(many_hoist);
+many_hoist(obj, 5);
+
+function constant_too_large(o, index) {
+ return o[index + 1033554433];
+}
+
+constant_too_large(obj, 0);
+%OptimizeFunctionOnNextCall(constant_too_large);
+constant_too_large(obj, 5);
=======================================
--- /branches/bleeding_edge/src/hydrogen-dehoist.cc Fri Jun 6 13:16:24
2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-dehoist.cc Mon Jun 23 09:09:05
2014 UTC
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "src/hydrogen-dehoist.h"
+#include "src/base/safe_math.h"
namespace v8 {
namespace internal {
@@ -28,15 +29,25 @@
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;
+
+ // Multiply value by elements size, bailing out on overflow.
+ int32_t elements_kind_size =
+ 1 << ElementsKindToShiftSize(array_operation->elements_kind());
+ v8::base::internal::CheckedNumeric<int32_t> multiply_result = value;
+ multiply_result = multiply_result * elements_kind_size;
+ if (!multiply_result.IsValid()) return;
+ value = multiply_result.ValueOrDie();
+
+ // Ensure that the array operation can add value to existing base offset
+ // without overflowing.
+ if (!array_operation->TryIncreaseBaseOffset(value)) 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->SetDehoisted(true);
}
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Fri Jun 20
11:26:17 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Mon Jun 23
09:09:05 2014 UTC
@@ -25,6 +25,8 @@
#error Unsupported target architecture.
#endif
+#include "src/base/safe_math.h"
+
namespace v8 {
namespace internal {
@@ -3482,6 +3484,22 @@
stream->Add(" check_hole");
}
}
+
+
+bool HLoadKeyed::TryIncreaseBaseOffset(uint32_t increase_by_value) {
+ // The base offset is usually simply the size of the array header, except
+ // with dehoisting adds an addition offset due to a array index key
+ // manipulation, in which case it becomes (array header size +
+ // constant-offset-from-key * kPointerSize)
+ uint32_t base_offset = BaseOffsetField::decode(bit_field_);
+ v8::base::internal::CheckedNumeric<uint32_t> addition_result =
base_offset;
+ addition_result += increase_by_value;
+ if (!addition_result.IsValid()) return false;
+ base_offset = addition_result.ValueOrDie();
+ if (!BaseOffsetField::is_valid(base_offset)) return false;
+ bit_field_ = BaseOffsetField::update(bit_field_, base_offset);
+ return true;
+}
bool HLoadKeyed::UsesMustHandleHole() const {
@@ -4060,6 +4078,19 @@
if (MustPrefillWithFiller()) stream->Add("F");
stream->Add(")");
}
+
+
+bool HStoreKeyed::TryIncreaseBaseOffset(uint32_t increase_by_value) {
+ // The base offset is usually simply the size of the array header, except
+ // with dehoisting adds an addition offset due to a array index key
+ // manipulation, in which case it becomes (array header size +
+ // constant-offset-from-key * kPointerSize)
+ v8::base::internal::CheckedNumeric<uint32_t> addition_result =
base_offset_;
+ addition_result += increase_by_value;
+ if (!addition_result.IsValid()) return false;
+ base_offset_ = addition_result.ValueOrDie();
+ return true;
+}
bool HStoreKeyed::NeedsCanonicalization() {
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Mon Jun 23 07:10:25
2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-instructions.h Mon Jun 23 09:09:05
2014 UTC
@@ -6451,8 +6451,8 @@
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;
+ // TryIncreaseBaseOffset returns false if overflow would result.
+ virtual bool TryIncreaseBaseOffset(uint32_t increase_by_value) = 0;
virtual bool IsDehoisted() = 0;
virtual void SetDehoisted(bool is_dehoisted) = 0;
virtual ~ArrayInstructionInterface() { }
@@ -6499,17 +6499,7 @@
}
bool HasDependency() const { return OperandAt(0) != OperandAt(2); }
uint32_t base_offset() { return BaseOffsetField::decode(bit_field_); }
- void IncreaseBaseOffset(uint32_t base_offset) {
- // The base offset is usually simply the size of the array header,
except
- // with dehoisting adds an addition offset due to a array index key
- // manipulation, in which case it becomes (array header size +
- // constant-offset-from-key * kPointerSize)
- base_offset += BaseOffsetField::decode(bit_field_);
- bit_field_ = BaseOffsetField::update(bit_field_, base_offset);
- }
- virtual int MaxBaseOffsetBits() {
- return kBitsForBaseOffset;
- }
+ bool TryIncreaseBaseOffset(uint32_t increase_by_value);
HValue* GetKey() { return key(); }
void SetKey(HValue* key) { SetOperandAt(1, key); }
bool IsDehoisted() { return IsDehoistedField::decode(bit_field_); }
@@ -6973,16 +6963,7 @@
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) {
- // The base offset is usually simply the size of the array header,
except
- // with dehoisting adds an addition offset due to a array index key
- // manipulation, in which case it becomes (array header size +
- // constant-offset-from-key * kPointerSize)
- base_offset_ += base_offset;
- }
- virtual int MaxBaseOffsetBits() {
- return 31 - ElementsKindToShiftSize(elements_kind_);
- }
+ bool TryIncreaseBaseOffset(uint32_t increase_by_value);
HValue* GetKey() { return key(); }
void SetKey(HValue* key) { SetOperandAt(1, key); }
bool IsDehoisted() { return is_dehoisted_; }
--
--
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/d/optout.
