From 2714fd2399cdc9d36de0f9df588221a0c9e55626 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Fri, 6 Jun 2014 13:16:24 +0000 Subject: [PATCH] Revert "Re-land Clusterfuzz identified overflow check needed in dehoisting." This reverts commit r21712 TBR=danno@chromium.org Review URL: https://codereview.chromium.org/315843005 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21715 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen-dehoist.cc | 20 ++++------------ src/hydrogen-instructions.h | 44 ++++++++++++---------------------- test/mjsunit/regress/regress-380092.js | 14 ----------- 3 files changed, 20 insertions(+), 58 deletions(-) delete mode 100644 test/mjsunit/regress/regress-380092.js diff --git a/src/hydrogen-dehoist.cc b/src/hydrogen-dehoist.cc index b377498..fe0ae76 100644 --- a/src/hydrogen-dehoist.cc +++ b/src/hydrogen-dehoist.cc @@ -28,25 +28,15 @@ static void DehoistArrayIndex(ArrayInstructionInterface* array_operation) { if (!constant->HasInteger32Value()) return; int32_t sign = binary_operation->IsSub() ? -1 : 1; int32_t value = constant->Integer32Value() * sign; - 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; + // 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; array_operation->SetKey(subexpression); if (binary_operation->HasNoUses()) { binary_operation->DeleteAndReplaceWith(NULL); } - array_operation->IncreaseBaseOffset(value); + value <<= ElementsKindToShiftSize(array_operation->elements_kind()); + array_operation->IncreaseBaseOffset(static_cast(value)); array_operation->SetDehoisted(true); } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 1fc800e..d1f2372 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -6404,9 +6404,8 @@ class ArrayInstructionInterface { virtual HValue* GetKey() = 0; virtual void SetKey(HValue* key) = 0; virtual ElementsKind elements_kind() const = 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 void IncreaseBaseOffset(uint32_t base_offset) = 0; + virtual int MaxBaseOffsetBits() = 0; virtual bool IsDehoisted() = 0; virtual void SetDehoisted(bool is_dehoisted) = 0; virtual ~ArrayInstructionInterface() { } @@ -6452,20 +6451,13 @@ class HLoadKeyed V8_FINAL return OperandAt(2); } bool HasDependency() const { return OperandAt(0) != OperandAt(2); } - uint32_t base_offset() { - int32_t base_offset_value = BaseOffsetField::decode(bit_field_); - ASSERT(base_offset_value >= 0); - return static_cast(base_offset_value); + 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(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); + virtual int MaxBaseOffsetBits() { + return kBitsForBaseOffset; } HValue* GetKey() { return key(); } void SetKey(HValue* key) { SetOperandAt(1, key); } @@ -6615,7 +6607,7 @@ class HLoadKeyed V8_FINAL public BitField {}; // NOLINT class BaseOffsetField: - public BitField + public BitField {}; // NOLINT class IsDehoistedField: public BitField @@ -6929,18 +6921,12 @@ class HStoreKeyed V8_FINAL } StoreFieldOrKeyedMode store_mode() const { return store_mode_; } ElementsKind elements_kind() const { return elements_kind_; } - uint32_t base_offset() { - ASSERT(base_offset_ >= 0); - return static_cast(base_offset_); - } - bool CanIncreaseBaseOffset(int32_t increase_by_value) { - ASSERT(increase_by_value >= 0); - // Guard against overflow - return (increase_by_value + base_offset_) >= 0; + uint32_t base_offset() { return base_offset_; } + void IncreaseBaseOffset(uint32_t base_offset) { + base_offset_ += base_offset; } - void IncreaseBaseOffset(int32_t increase_by_value) { - ASSERT(increase_by_value >= 0); - base_offset_ += increase_by_value; + virtual int MaxBaseOffsetBits() { + return 31 - ElementsKindToShiftSize(elements_kind_); } HValue* GetKey() { return key(); } void SetKey(HValue* key) { SetOperandAt(1, key); } @@ -7031,7 +7017,7 @@ class HStoreKeyed V8_FINAL } ElementsKind elements_kind_; - int32_t base_offset_; + uint32_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 deleted file mode 100644 index a763435..0000000 --- a/test/mjsunit/regress/regress-380092.js +++ /dev/null @@ -1,14 +0,0 @@ -// 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); -- 2.7.4