Add dependency to HLoadKeyed* instructions to prevent invalid hoisting

BUG=chromium:137768
TEST=test/mjsunit/regress/regress-137768.js

Review URL: https://chromiumcodereview.appspot.com/10802038

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

src/hydrogen-instructions.cc

@@ -1803,7 +1803,8 @@ void HLoadKeyedFastElement::PrintDataTo(StringStream* stream) {
   object()->PrintNameTo(stream);
   stream->Add("[");
   key()->PrintNameTo(stream);
-  stream->Add("]");
+  stream->Add("] ");
+  dependency()->PrintNameTo(stream);
   if (RequiresHoleCheck()) {
     stream->Add(" check_hole");
   }
@@ -1828,7 +1829,8 @@ void HLoadKeyedFastDoubleElement::PrintDataTo(StringStream* stream) {
   elements()->PrintNameTo(stream);
   stream->Add("[");
   key()->PrintNameTo(stream);
-  stream->Add("]");
+  stream->Add("] ");
+  dependency()->PrintNameTo(stream);
 }
 
 
@@ -1857,6 +1859,7 @@ HValue* HLoadKeyedGeneric::Canonicalize() {
             new(block()->zone()) HCheckMapValue(object(), names_cache->map());
         HInstruction* index = new(block()->zone()) HLoadKeyedFastElement(
             index_cache,
+            key_load->key(),
             key_load->key());
         map_check->InsertBefore(this);
         index->InsertBefore(this);
@@ -1917,7 +1920,8 @@ void HLoadKeyedSpecializedArrayElement::PrintDataTo(
   }
   stream->Add("[");
   key()->PrintNameTo(stream);
-  stream->Add("]");
+  stream->Add("] ");
+  dependency()->PrintNameTo(stream);
 }
 
 

src/hydrogen-instructions.h

@@ -4053,10 +4053,11 @@ class ArrayInstructionInterface {
 };
 
 class HLoadKeyedFastElement
-    : public HTemplateInstruction<2>, public ArrayInstructionInterface {
+    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
  public:
   HLoadKeyedFastElement(HValue* obj,
                         HValue* key,
+                        HValue* dependency,
                         ElementsKind elements_kind = FAST_ELEMENTS)
       : bit_field_(0) {
     ASSERT(IsFastSmiOrObjectElementsKind(elements_kind));
@@ -4067,6 +4068,7 @@ class HLoadKeyedFastElement
     }
     SetOperandAt(0, obj);
     SetOperandAt(1, key);
+    SetOperandAt(2, dependency);
     set_representation(Representation::Tagged());
     SetGVNFlag(kDependsOnArrayElements);
     SetFlag(kUseGVN);
@@ -4074,6 +4076,7 @@ class HLoadKeyedFastElement
 
   HValue* object() { return OperandAt(0); }
   HValue* key() { return OperandAt(1); }
+  HValue* dependency() { return OperandAt(2); }
   uint32_t index_offset() { return IndexOffsetField::decode(bit_field_); }
   void SetIndexOffset(uint32_t index_offset) {
     bit_field_ = IndexOffsetField::update(bit_field_, index_offset);
@@ -4090,9 +4093,9 @@ class HLoadKeyedFastElement
 
   virtual Representation RequiredInputRepresentation(int index) {
     // The key is supposed to be Integer32.
-    return index == 0
-      ? Representation::Tagged()
-      : Representation::Integer32();
+    if (index == 0) return Representation::Tagged();
+    if (index == 1) return Representation::Integer32();
+    return Representation::None();
   }
 
   virtual void PrintDataTo(StringStream* stream);
@@ -4122,17 +4125,19 @@ enum HoleCheckMode { PERFORM_HOLE_CHECK, OMIT_HOLE_CHECK };
 
 
 class HLoadKeyedFastDoubleElement
-    : public HTemplateInstruction<2>, public ArrayInstructionInterface {
+    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
  public:
   HLoadKeyedFastDoubleElement(
     HValue* elements,
     HValue* key,
+    HValue* dependency,
     HoleCheckMode hole_check_mode = PERFORM_HOLE_CHECK)
       : index_offset_(0),
         is_dehoisted_(false),
         hole_check_mode_(hole_check_mode) {
     SetOperandAt(0, elements);
     SetOperandAt(1, key);
+    SetOperandAt(2, dependency);
     set_representation(Representation::Double());
     SetGVNFlag(kDependsOnDoubleArrayElements);
     SetFlag(kUseGVN);
@@ -4140,6 +4145,7 @@ class HLoadKeyedFastDoubleElement
 
   HValue* elements() { return OperandAt(0); }
   HValue* key() { return OperandAt(1); }
+  HValue* dependency() { return OperandAt(2); }
   uint32_t index_offset() { return index_offset_; }
   void SetIndexOffset(uint32_t index_offset) { index_offset_ = index_offset; }
   HValue* GetKey() { return key(); }
@@ -4149,9 +4155,9 @@ class HLoadKeyedFastDoubleElement
 
   virtual Representation RequiredInputRepresentation(int index) {
     // The key is supposed to be Integer32.
-    return index == 0
-      ? Representation::Tagged()
-      : Representation::Integer32();
+    if (index == 0) return Representation::Tagged();
+    if (index == 1) return Representation::Integer32();
+    return Representation::None();
   }
 
   bool RequiresHoleCheck() {
@@ -4178,16 +4184,18 @@ class HLoadKeyedFastDoubleElement
 
 
 class HLoadKeyedSpecializedArrayElement
-    : public HTemplateInstruction<2>, public ArrayInstructionInterface {
+    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
  public:
   HLoadKeyedSpecializedArrayElement(HValue* external_elements,
                                     HValue* key,
+                                    HValue* dependency,
                                     ElementsKind elements_kind)
       :  elements_kind_(elements_kind),
          index_offset_(0),
          is_dehoisted_(false) {
     SetOperandAt(0, external_elements);
     SetOperandAt(1, key);
+    SetOperandAt(2, dependency);
     if (elements_kind == EXTERNAL_FLOAT_ELEMENTS ||
         elements_kind == EXTERNAL_DOUBLE_ELEMENTS) {
       set_representation(Representation::Double());
@@ -4203,15 +4211,15 @@ class HLoadKeyedSpecializedArrayElement
   virtual void PrintDataTo(StringStream* stream);
 
   virtual Representation RequiredInputRepresentation(int index) {
-    // The key is supposed to be Integer32, but the base pointer
-    // for the element load is a naked pointer.
-    return index == 0
-      ? Representation::External()
-      : Representation::Integer32();
+    // The key is supposed to be Integer32.
+    if (index == 0) return Representation::External();
+    if (index == 1) return Representation::Integer32();
+    return Representation::None();
   }
 
   HValue* external_pointer() { return OperandAt(0); }
   HValue* key() { return OperandAt(1); }
+  HValue* dependency() { return OperandAt(2); }
   ElementsKind elements_kind() const { return elements_kind_; }
   uint32_t index_offset() { return index_offset_; }
   void SetIndexOffset(uint32_t index_offset) { index_offset_ = index_offset; }

src/hydrogen.cc

@@ -4368,7 +4368,8 @@ void HGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
   HValue* key = AddInstruction(
       new(zone()) HLoadKeyedFastElement(
           environment()->ExpressionStackAt(2),  // Enum cache.
-          environment()->ExpressionStackAt(0)));  // Iteration index.
+          environment()->ExpressionStackAt(0),  // Iteration index.
+          environment()->ExpressionStackAt(0)));
 
   // Check if the expected map still matches that of the enumerable.
   // If not just deoptimize.
@@ -5708,6 +5709,7 @@ HInstruction* HGraphBuilder::BuildExternalArrayElementAccess(
     HValue* external_elements,
     HValue* checked_key,
     HValue* val,
+    HValue* dependency,
     ElementsKind elements_kind,
     bool is_store) {
   if (is_store) {
@@ -5751,7 +5753,7 @@ HInstruction* HGraphBuilder::BuildExternalArrayElementAccess(
   } else {
     ASSERT(val == NULL);
     return new(zone()) HLoadKeyedSpecializedArrayElement(
-        external_elements, checked_key, elements_kind);
+        external_elements, checked_key, dependency, elements_kind);
   }
 }
 
@@ -5759,6 +5761,7 @@ HInstruction* HGraphBuilder::BuildExternalArrayElementAccess(
 HInstruction* HGraphBuilder::BuildFastElementAccess(HValue* elements,
                                                     HValue* checked_key,
                                                     HValue* val,
+                                                    HValue* load_dependency,
                                                     ElementsKind elements_kind,
                                                     bool is_store) {
   if (is_store) {
@@ -5787,10 +5790,11 @@ HInstruction* HGraphBuilder::BuildFastElementAccess(HValue* elements,
       OMIT_HOLE_CHECK :
       PERFORM_HOLE_CHECK;
   if (IsFastDoubleElementsKind(elements_kind)) {
-    return new(zone()) HLoadKeyedFastDoubleElement(elements, checked_key, mode);
+    return new(zone()) HLoadKeyedFastDoubleElement(elements, checked_key,
+                                                   load_dependency, mode);
   } else {  // Smi or Object elements.
     return new(zone()) HLoadKeyedFastElement(elements, checked_key,
-                                             elements_kind);
+                                             load_dependency, elements_kind);
   }
 }
 
@@ -5847,8 +5851,9 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
     HLoadExternalArrayPointer* external_elements =
         new(zone()) HLoadExternalArrayPointer(elements);
     AddInstruction(external_elements);
-    return BuildExternalArrayElementAccess(external_elements, checked_key,
-                                           val, map->elements_kind(), is_store);
+    return BuildExternalArrayElementAccess(
+        external_elements, checked_key, val, mapcheck,
+        map->elements_kind(), is_store);
   }
   ASSERT(fast_smi_only_elements ||
          fast_elements ||
@@ -5860,7 +5865,7 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
     length = AddInstruction(new(zone()) HFixedArrayBaseLength(elements));
   }
   checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
-  return BuildFastElementAccess(elements, checked_key, val,
+  return BuildFastElementAccess(elements, checked_key, val, mapcheck,
                                 map->elements_kind(), is_store);
 }
 
@@ -6081,7 +6086,8 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
         checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length,
                                                               ALLOW_SMI_KEY));
         access = AddInstruction(BuildFastElementAccess(
-            elements, checked_key, val, elements_kind, is_store));
+            elements, checked_key, val, elements_kind_branch,
+            elements_kind, is_store));
         if (!is_store) {
           Push(access);
         }
@@ -6097,7 +6103,8 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
         checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length,
                                                               ALLOW_SMI_KEY));
         access = AddInstruction(BuildFastElementAccess(
-            elements, checked_key, val, elements_kind, is_store));
+            elements, checked_key, val, elements_kind_branch,
+            elements_kind, is_store));
       } else if (elements_kind == DICTIONARY_ELEMENTS) {
         if (is_store) {
           access = AddInstruction(BuildStoreKeyedGeneric(object, key, val));
@@ -6106,7 +6113,8 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
         }
       } else {  // External array elements.
         access = AddInstruction(BuildExternalArrayElementAccess(
-            external_elements, checked_key, val, elements_kind, is_store));
+            external_elements, checked_key, val, elements_kind_branch,
+            elements_kind, is_store));
       }
       *has_side_effects |= access->HasObservableSideEffects();
       if (position != RelocInfo::kNoPosition) access->set_position(position);

src/hydrogen.h

@@ -1100,11 +1100,13 @@ class HGraphBuilder: public AstVisitor {
       HValue* external_elements,
       HValue* checked_key,
       HValue* val,
+      HValue* dependency,
       ElementsKind elements_kind,
       bool is_store);
   HInstruction* BuildFastElementAccess(HValue* elements,
                                        HValue* checked_key,
                                        HValue* val,
+                                       HValue* dependency,
                                        ElementsKind elements_kind,
                                        bool is_store);
 

test/mjsunit/regress/regress-137768.js

+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+// Create elements in a constructor function to ensure map sharing.
+function TestConstructor() {
+  this[0] = 1;
+  this[1] = 2;
+  this[2] = 3;
+}
+
+function bad_func(o,a) {
+  var s = 0;
+  for (var i = 0; i < 1; ++i) {
+    o.newFileToChangeMap = undefined;
+    var x = a[0];
+    s += x;
+  }
+  return s;
+}
+
+o = new Object();
+a = new TestConstructor();
+bad_func(o, a);
+
+// Make sure that we're out of pre-monomorphic state for the member add of
+// 'newFileToChangeMap' which causes a map transition.
+o = new Object();
+a = new TestConstructor();
+bad_func(o, a);
+
+// Optimize, before the fix, the element load and subsequent tagged-to-i were
+// hoisted above the map check, which can't be hoisted due to the map-changing
+// store.
+o = new Object();
+a = new TestConstructor();
+%OptimizeFunctionOnNextCall(bad_func);
+bad_func(o, a);
+
+// Pass in a array of doubles. Before the fix, the optimized load and
+// tagged-to-i will treat part of a double value as a pointer and de-ref it
+// before the map check was executed that should have deopt.
+o = new Object();
+// Pass in an elements buffer where the bit representation of the double numbers
+// are two adjacent small 32-bit values with the lowest bit set to one, causing
+// tagged-to-i to SIGSEGV.
+a = [2.122e-314, 2.122e-314, 2.122e-314];
+bad_func(o, a);