Revision: 12173
Author: [email protected]
Date: Mon Jul 23 06:59:24 2012
Log: 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
http://code.google.com/p/v8/source/detail?r=12173
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-137768.js
Modified:
/branches/bleeding_edge/src/hydrogen-instructions.cc
/branches/bleeding_edge/src/hydrogen-instructions.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/hydrogen.h
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-137768.js Mon Jul
23 06:59:24 2012
@@ -0,0 +1,73 @@
+// 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);
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Wed Jul 18
04:40:39 2012
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Mon Jul 23
06:59:24 2012
@@ -1803,7 +1803,8 @@
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 @@
elements()->PrintNameTo(stream);
stream->Add("[");
key()->PrintNameTo(stream);
- stream->Add("]");
+ stream->Add("] ");
+ dependency()->PrintNameTo(stream);
}
@@ -1857,6 +1859,7 @@
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 @@
}
stream->Add("[");
key()->PrintNameTo(stream);
- stream->Add("]");
+ stream->Add("] ");
+ dependency()->PrintNameTo(stream);
}
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Fri Jul 20 04:00:33
2012
+++ /branches/bleeding_edge/src/hydrogen-instructions.h Mon Jul 23 06:59:24
2012
@@ -4053,10 +4053,11 @@
};
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 @@
}
SetOperandAt(0, obj);
SetOperandAt(1, key);
+ SetOperandAt(2, dependency);
set_representation(Representation::Tagged());
SetGVNFlag(kDependsOnArrayElements);
SetFlag(kUseGVN);
@@ -4074,6 +4076,7 @@
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 @@
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 @@
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 @@
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 @@
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 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 @@
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; }
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Fri Jul 20 04:00:33 2012
+++ /branches/bleeding_edge/src/hydrogen.cc Mon Jul 23 06:59:24 2012
@@ -4368,7 +4368,8 @@
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 @@
HValue* external_elements,
HValue* checked_key,
HValue* val,
+ HValue* dependency,
ElementsKind elements_kind,
bool is_store) {
if (is_store) {
@@ -5751,7 +5753,7 @@
} 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::BuildFastElementAccess(HValue* elements,
HValue* checked_key,
HValue* val,
+ HValue*
load_dependency,
ElementsKind
elements_kind,
bool is_store) {
if (is_store) {
@@ -5787,10 +5790,11 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
} 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);
=======================================
--- /branches/bleeding_edge/src/hydrogen.h Thu Jul 19 03:46:03 2012
+++ /branches/bleeding_edge/src/hydrogen.h Mon Jul 23 06:59:24 2012
@@ -1100,11 +1100,13 @@
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);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
