diff --git a/src/hydrogen-dehoist.cc b/src/hydrogen-dehoist.cc index fe0ae764ad..b377498cac 100644 --- a/src/hydrogen-dehoist.cc +++ b/src/hydrogen-dehoist.cc @@ -28,15 +28,25 @@ 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; + + // Check for overflow. + // TODO(mvstanton): replace with safe_math.h operations when that code is + // integrated. + int32_t shift_amount = + 1 << ElementsKindToShiftSize(array_operation->elements_kind()); + int32_t multiplication_result = value * shift_amount; + if ((multiplication_result >> shift_amount) != value) return; + value = multiplication_result; + + // Ensure that the array operation can add value to existing base offset + // without overflowing. + if (!array_operation->CanIncreaseBaseOffset(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(value)); + array_operation->IncreaseBaseOffset(value); array_operation->SetDehoisted(true); } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index d1f2372db6..1fc800e6ff 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -6404,8 +6404,9 @@ 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; + // increase_by_value should be non-negative + virtual bool CanIncreaseBaseOffset(int32_t increase_by_value) = 0; + virtual void IncreaseBaseOffset(int32_t increase_by_value) = 0; virtual bool IsDehoisted() = 0; virtual void SetDehoisted(bool is_dehoisted) = 0; virtual ~ArrayInstructionInterface() { } @@ -6451,13 +6452,20 @@ class HLoadKeyed V8_FINAL return OperandAt(2); } 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); + uint32_t base_offset() { + int32_t base_offset_value = BaseOffsetField::decode(bit_field_); + ASSERT(base_offset_value >= 0); + return static_cast(base_offset_value); } - virtual int MaxBaseOffsetBits() { - return kBitsForBaseOffset; + bool CanIncreaseBaseOffset(int32_t increase_by_value) { + ASSERT(increase_by_value >= 0); + int32_t new_value = BaseOffsetField::decode(bit_field_) + increase_by_value; + return (new_value >= 0 && BaseOffsetField::is_valid(new_value)); + } + void IncreaseBaseOffset(int32_t increase_by_value) { + ASSERT(increase_by_value >= 0); + 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); } @@ -6607,7 +6615,7 @@ class HLoadKeyed V8_FINAL public BitField {}; // NOLINT class BaseOffsetField: - public BitField + public BitField {}; // NOLINT class IsDehoistedField: public BitField @@ -6921,12 +6929,18 @@ 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; + uint32_t base_offset() { + ASSERT(base_offset_ >= 0); + return static_cast(base_offset_); } - virtual int MaxBaseOffsetBits() { - return 31 - ElementsKindToShiftSize(elements_kind_); + bool CanIncreaseBaseOffset(int32_t increase_by_value) { + ASSERT(increase_by_value >= 0); + // Guard against overflow + return (increase_by_value + base_offset_) >= 0; + } + void IncreaseBaseOffset(int32_t increase_by_value) { + ASSERT(increase_by_value >= 0); + base_offset_ += increase_by_value; } HValue* GetKey() { return key(); } void SetKey(HValue* key) { SetOperandAt(1, key); } @@ -7017,7 +7031,7 @@ class HStoreKeyed V8_FINAL } ElementsKind elements_kind_; - uint32_t base_offset_; + int32_t base_offset_; bool is_dehoisted_ : 1; bool is_uninitialized_ : 1; StoreFieldOrKeyedMode store_mode_: 1; diff --git a/test/mjsunit/regress/regress-380092.js b/test/mjsunit/regress/regress-380092.js new file mode 100644 index 0000000000..a7634355ca --- /dev/null +++ b/test/mjsunit/regress/regress-380092.js @@ -0,0 +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 + +function many_hoist(o, index) { + return o[index + 33554427]; +} + +var obj = {}; +many_hoist(obj, 0); +%OptimizeFunctionOnNextCall(many_hoist); +many_hoist(obj, 5);