Re-land "Clusterfuzz identified overflow check needed in dehoisting."

BUG=380092
LOG=N
R=jkummerow@chromium.org

Review URL: https://codereview.chromium.org/335063005

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21920 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/hydrogen-dehoist.cc

@@ -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 @@ 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;
+
+  // 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);
 }
 

src/hydrogen-instructions.cc

@@ -25,6 +25,8 @@
 #error Unsupported target architecture.
 #endif
 
+#include "src/base/safe_math.h"
+
 namespace v8 {
 namespace internal {
 
@@ -3484,6 +3486,22 @@ void HLoadKeyed::PrintDataTo(StringStream* stream) {
 }
 
 
+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 {
   if (IsFastPackedElementsKind(elements_kind())) {
     return false;
@@ -4062,6 +4080,19 @@ void HAllocate::PrintDataTo(StringStream* stream) {
 }
 
 
+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() {
   // If value is an integer or smi or comes from the result of a keyed load or
   // constant then it is either be a non-hole value or in the case of a constant

src/hydrogen-instructions.h

@@ -6451,8 +6451,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;
+  // 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 @@ 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) {
-    // 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 @@ 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) {
-    // 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_; }

test/mjsunit/regress/regress-380092.js

+// 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);