Revision: 5718
Author: [email protected]
Date: Wed Oct 27 04:37:59 2010
Log: Fix a bug that prevents constants from overwriting function values in
object literals.
BUG=http://code.google.com/p/v8/issues/detail?id=907
Review URL: http://codereview.chromium.org/4004006
http://code.google.com/p/v8/source/detail?r=5718
Added:
/branches/bleeding_edge/test/mjsunit/object-literal-conversions.js
/branches/bleeding_edge/test/mjsunit/object-literal-overwrite.js
Modified:
/branches/bleeding_edge/src/arm/codegen-arm.cc
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/ast.cc
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/ia32/codegen-ia32.cc
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/x64/codegen-x64.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/object-literal-conversions.js Wed
Oct 27 04:37:59 2010
@@ -0,0 +1,46 @@
+// Copyright 2010 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.
+
+// Test that the various conversions between property names are correctly
+// used when overwriting initializers.
+
+var test1 = { 13: 6, "13": 7 };
+var test2 = { 13: 7, "13.0": 6 };
+var test3 = { "13": 6, 13.0000000000000000: 7 };
+var test4 = { 13.213000: 6, "13.213": 7 };
+
+assertEquals(7, test1[13]);
+assertEquals(7, test2[13]);
+assertEquals(7, test3[13]);
+assertEquals(7, test4[13.213]);
+
+var test5 = { 13: function() {}, "13": 7 };
+var test6 = { 17.31: function() {}, "17.31": 7 };
+
+assertEquals(7, test5[13]);
+assertEquals(7, test6[17.31]);
+
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/object-literal-overwrite.js Wed
Oct 27 04:37:59 2010
@@ -0,0 +1,118 @@
+// Copyright 2010 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.
+
+// Check that constants and computed properties are overwriting each other
+// correctly, i.e., the last initializer for any name is stored in the
object.
+
+
+// Tests for the full code generator (if active).
+
+var foo1 = {
+ bar: 6,
+ bar: 7
+};
+
+var foo2 = {
+ bar: function(a){},
+ bar: 7
+};
+
+var foo3 = {
+ bar: function(a){},
+ bar: function(b){},
+ bar: 7
+};
+
+var foo4 = {
+ bar: function(b){},
+ bar: 7,
+ bar: function(){return 7},
+};
+
+var foo5 = {
+ 13: function(a){},
+ 13: 7
+}
+
+var foo6 = {
+ 14.31: function(a){},
+ 14.31: 7
+}
+
+var foo7 = {
+ 15: 6,
+ 15: 7
+}
+
+assertEquals(7, foo1.bar);
+assertEquals(7, foo2.bar);
+assertEquals(7, foo3.bar);
+assertEquals(7, foo4.bar());
+assertEquals(7, foo5[13]);
+assertEquals(7, foo6[14.31]);
+assertEquals(7, foo7[15]);
+
+// Test for the classic code generator.
+
+function fun(x) {
+ var inner = { j: function(x) { return x; }, j: 7 };
+ return inner.j;
+}
+
+assertEquals(7, fun(7) );
+
+// Check that the initializers of computed properties are executed, even if
+// no store instructions are generated for the literals.
+
+var glob1 = 0;
+
+var bar1 = { x: glob1++, x: glob1++, x: glob1++, x: 7};
+
+assertEquals(3, glob1);
+
+
+var glob2 = 0;
+
+function fun2() {
+ var r = { y: glob2++, y: glob2++, y: glob2++, y: 7};
+ return r.y;
+}
+
+var y = fun2();
+assertEquals(7, y);
+assertEquals(3, glob2);
+
+var glob3 = 0;
+
+function fun3() {
+ var r = { 113: glob3++, 113: glob3++, 113: glob3++, 113: 7};
+ return r[113];
+}
+
+var y = fun3();
+assertEquals(7, y);
+assertEquals(3, glob3);
=======================================
--- /branches/bleeding_edge/src/arm/codegen-arm.cc Wed Oct 20 05:01:17 2010
+++ /branches/bleeding_edge/src/arm/codegen-arm.cc Wed Oct 27 04:37:59 2010
@@ -3596,6 +3596,12 @@
frame_->CallRuntime(Runtime::kCreateObjectLiteralShallow, 4);
}
frame_->EmitPush(r0); // save the result
+
+ // Mark all computed expressions that are bound to a key that
+ // is shadowed by a later occurrence of the same key. For the
+ // marked expressions, no store code is emitted.
+ node->CalculateEmitStore();
+
for (int i = 0; i < node->properties()->length(); i++) {
// At the start of each iteration, the top of stack contains
// the newly created object literal.
@@ -3612,11 +3618,15 @@
if (key->handle()->IsSymbol()) {
Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
Load(value);
- frame_->PopToR0();
- // Fetch the object literal.
- frame_->SpillAllButCopyTOSToR1();
- __ mov(r2, Operand(key->handle()));
- frame_->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);
+ if (property->emit_store()) {
+ frame_->PopToR0();
+ // Fetch the object literal.
+ frame_->SpillAllButCopyTOSToR1();
+ __ mov(r2, Operand(key->handle()));
+ frame_->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);
+ } else {
+ frame_->Drop();
+ }
break;
}
// else fall through
@@ -3624,7 +3634,11 @@
frame_->Dup();
Load(key);
Load(value);
- frame_->CallRuntime(Runtime::kSetProperty, 3);
+ if (property->emit_store()) {
+ frame_->CallRuntime(Runtime::kSetProperty, 3);
+ } else {
+ frame_->Drop(3);
+ }
break;
}
case ObjectLiteral::Property::SETTER: {
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Fri Oct 15 04:45:05
2010
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Wed Oct 27 04:37:59
2010
@@ -1169,6 +1169,11 @@
// result_saved is false the result is in r0.
bool result_saved = false;
+ // Mark all computed expressions that are bound to a key that
+ // is shadowed by a later occurrence of the same key. For the
+ // marked expressions, no store code is emitted.
+ expr->CalculateEmitStore();
+
for (int i = 0; i < expr->properties()->length(); i++) {
ObjectLiteral::Property* property = expr->properties()->at(i);
if (property->IsCompileTimeValue()) continue;
@@ -1190,8 +1195,10 @@
VisitForAccumulatorValue(value);
__ mov(r2, Operand(key->handle()));
__ ldr(r1, MemOperand(sp));
- Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
- EmitCallIC(ic, RelocInfo::CODE_TARGET);
+ if (property->emit_store()) {
+ Handle<Code>
ic(Builtins::builtin(Builtins::StoreIC_Initialize));
+ EmitCallIC(ic, RelocInfo::CODE_TARGET);
+ }
break;
}
// Fall through.
@@ -1201,7 +1208,11 @@
__ push(r0);
VisitForStackValue(key);
VisitForStackValue(value);
- __ CallRuntime(Runtime::kSetProperty, 3);
+ if (property->emit_store()) {
+ __ CallRuntime(Runtime::kSetProperty, 3);
+ } else {
+ __ Drop(3);
+ }
break;
case ObjectLiteral::Property::GETTER:
case ObjectLiteral::Property::SETTER:
=======================================
--- /branches/bleeding_edge/src/ast.cc Tue Oct 19 07:00:01 2010
+++ /branches/bleeding_edge/src/ast.cc Wed Oct 27 04:37:59 2010
@@ -140,6 +140,7 @@
ObjectLiteral::Property::Property(Literal* key, Expression* value) {
+ emit_store_ = true;
key_ = key;
value_ = value;
Object* k = *key->handle();
@@ -156,6 +157,7 @@
ObjectLiteral::Property::Property(bool is_getter, FunctionLiteral* value) {
+ emit_store_ = true;
key_ = new Literal(value->name());
value_ = value;
kind_ = is_getter ? GETTER : SETTER;
@@ -167,6 +169,78 @@
(kind_ == MATERIALIZED_LITERAL &&
CompileTimeValue::IsCompileTimeValue(value_));
}
+
+
+void ObjectLiteral::Property::set_emit_store(bool emit_store) {
+ emit_store_ = emit_store;
+}
+
+
+bool ObjectLiteral::Property::emit_store() {
+ return emit_store_;
+}
+
+
+bool IsEqualString(void* first, void* second) {
+ Handle<String> h1(reinterpret_cast<String**>(first));
+ Handle<String> h2(reinterpret_cast<String**>(second));
+ return (*h1)->Equals(*h2);
+}
+
+bool IsEqualSmi(void* first, void* second) {
+ Handle<Smi> h1(reinterpret_cast<Smi**>(first));
+ Handle<Smi> h2(reinterpret_cast<Smi**>(second));
+ return (*h1)->value() == (*h2)->value();
+}
+
+void ObjectLiteral::CalculateEmitStore() {
+ HashMap properties(&IsEqualString);
+ HashMap elements(&IsEqualSmi);
+ for (int i = this->properties()->length() - 1; i >= 0; i--) {
+ ObjectLiteral::Property* property = this->properties()->at(i);
+ Literal* literal = property->key();
+ Handle<Object> handle = literal->handle();
+
+ if (handle->IsNull()) {
+ continue;
+ }
+
+ uint32_t hash;
+ HashMap* table;
+ void* key;
+ uint32_t index;
+ if (handle->IsSymbol()) {
+ Handle<String> name(String::cast(*handle));
+ ASSERT(!name->AsArrayIndex(&index));
+ key = name.location();
+ hash = name->Hash();
+ table = &properties;
+ } else if (handle->ToArrayIndex(&index)) {
+ key = handle.location();
+ hash = index;
+ table = &elements;
+ } else {
+ ASSERT(handle->IsNumber());
+ double num = handle->Number();
+ char arr[100];
+ Vector<char> buffer(arr, ARRAY_SIZE(arr));
+ const char* str = DoubleToCString(num, buffer);
+ Handle<String> name = Factory::NewStringFromAscii(CStrVector(str));
+ key = name.location();
+ hash = name->Hash();
+ table = &properties;
+ }
+ // If the key of a computed property is in the table, do not emit
+ // a store for the property later.
+ if (property->kind() == ObjectLiteral::Property::COMPUTED) {
+ if (table->Lookup(literal, hash, false) != NULL) {
+ property->set_emit_store(false);
+ }
+ }
+ // Add key to the table.
+ table->Lookup(literal, hash, true);
+ }
+}
void TargetCollector::AddTarget(BreakTarget* target) {
=======================================
--- /branches/bleeding_edge/src/ast.h Tue Oct 19 07:00:01 2010
+++ /branches/bleeding_edge/src/ast.h Wed Oct 27 04:37:59 2010
@@ -832,10 +832,14 @@
bool IsCompileTimeValue();
+ void set_emit_store(bool emit_store);
+ bool emit_store();
+
private:
Literal* key_;
Expression* value_;
Kind kind_;
+ bool emit_store_;
};
ObjectLiteral(Handle<FixedArray> constant_properties,
@@ -857,6 +861,12 @@
ZoneList<Property*>* properties() const { return properties_; }
bool fast_elements() const { return fast_elements_; }
+
+
+ // Mark all computed expressions that are bound to a key that
+ // is shadowed by a later occurrence of the same key. For the
+ // marked expressions, no store code is emitted.
+ void CalculateEmitStore();
private:
Handle<FixedArray> constant_properties_;
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Wed Oct 6 01:47:08
2010
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Wed Oct 27 04:37:59
2010
@@ -5559,6 +5559,11 @@
}
frame_->Push(&clone);
+ // Mark all computed expressions that are bound to a key that
+ // is shadowed by a later occurrence of the same key. For the
+ // marked expressions, no store code is emitted.
+ node->CalculateEmitStore();
+
for (int i = 0; i < node->properties()->length(); i++) {
ObjectLiteral::Property* property = node->properties()->at(i);
switch (property->kind()) {
@@ -5573,24 +5578,32 @@
// Duplicate the object as the IC receiver.
frame_->Dup();
Load(property->value());
- Result ignored =
- frame_->CallStoreIC(Handle<String>::cast(key), false);
- // A test eax instruction following the store IC call would
- // indicate the presence of an inlined version of the
- // store. Add a nop to indicate that there is no such
- // inlined version.
- __ nop();
+ if (property->emit_store()) {
+ Result ignored =
+ frame_->CallStoreIC(Handle<String>::cast(key), false);
+ // A test eax instruction following the store IC call would
+ // indicate the presence of an inlined version of the
+ // store. Add a nop to indicate that there is no such
+ // inlined version.
+ __ nop();
+ } else {
+ frame_->Drop(2);
+ }
break;
}
// Fall through
}
case ObjectLiteral::Property::PROTOTYPE: {
- // Duplicate the object as an argument to the runtime call.
- frame_->Dup();
- Load(property->key());
- Load(property->value());
- Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
- // Ignore the result.
+ // Duplicate the object as an argument to the runtime call.
+ frame_->Dup();
+ Load(property->key());
+ Load(property->value());
+ if (property->emit_store()) {
+ // Ignore the result.
+ Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
+ } else {
+ frame_->Drop(3);
+ }
break;
}
case ObjectLiteral::Property::SETTER: {
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Oct 15
04:45:05 2010
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Wed Oct 27
04:37:59 2010
@@ -1202,6 +1202,11 @@
// result_saved is false the result is in eax.
bool result_saved = false;
+ // Mark all computed expressions that are bound to a key that
+ // is shadowed by a later occurrence of the same key. For the
+ // marked expressions, no store code is emitted.
+ expr->CalculateEmitStore();
+
for (int i = 0; i < expr->properties()->length(); i++) {
ObjectLiteral::Property* property = expr->properties()->at(i);
if (property->IsCompileTimeValue()) continue;
@@ -1221,8 +1226,10 @@
VisitForAccumulatorValue(value);
__ mov(ecx, Immediate(key->handle()));
__ mov(edx, Operand(esp, 0));
- Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
- EmitCallIC(ic, RelocInfo::CODE_TARGET);
+ if (property->emit_store()) {
+ Handle<Code>
ic(Builtins::builtin(Builtins::StoreIC_Initialize));
+ EmitCallIC(ic, RelocInfo::CODE_TARGET);
+ }
break;
}
// Fall through.
@@ -1230,7 +1237,11 @@
__ push(Operand(esp, 0)); // Duplicate receiver.
VisitForStackValue(key);
VisitForStackValue(value);
- __ CallRuntime(Runtime::kSetProperty, 3);
+ if (property->emit_store()) {
+ __ CallRuntime(Runtime::kSetProperty, 3);
+ } else {
+ __ Drop(3);
+ }
break;
case ObjectLiteral::Property::SETTER:
case ObjectLiteral::Property::GETTER:
=======================================
--- /branches/bleeding_edge/src/x64/codegen-x64.cc Wed Oct 6 01:47:08 2010
+++ /branches/bleeding_edge/src/x64/codegen-x64.cc Wed Oct 27 04:37:59 2010
@@ -4866,6 +4866,11 @@
}
frame_->Push(&clone);
+ // Mark all computed expressions that are bound to a key that
+ // is shadowed by a later occurrence of the same key. For the
+ // marked expressions, no store code is emitted.
+ node->CalculateEmitStore();
+
for (int i = 0; i < node->properties()->length(); i++) {
ObjectLiteral::Property* property = node->properties()->at(i);
switch (property->kind()) {
@@ -4880,13 +4885,17 @@
// Duplicate the object as the IC receiver.
frame_->Dup();
Load(property->value());
- Result ignored =
- frame_->CallStoreIC(Handle<String>::cast(key), false);
- // A test rax instruction following the store IC call would
- // indicate the presence of an inlined version of the
- // store. Add a nop to indicate that there is no such
- // inlined version.
- __ nop();
+ if (property->emit_store()) {
+ Result ignored =
+ frame_->CallStoreIC(Handle<String>::cast(key), false);
+ // A test rax instruction following the store IC call would
+ // indicate the presence of an inlined version of the
+ // store. Add a nop to indicate that there is no such
+ // inlined version.
+ __ nop();
+ } else {
+ frame_->Drop(2);
+ }
break;
}
// Fall through
@@ -4896,8 +4905,12 @@
frame_->Dup();
Load(property->key());
Load(property->value());
- Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
- // Ignore the result.
+ if (property->emit_store()) {
+ // Ignore the result.
+ Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
+ } else {
+ frame_->Drop(3);
+ }
break;
}
case ObjectLiteral::Property::SETTER: {
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Fri Oct 15 04:45:05
2010
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Wed Oct 27 04:37:59
2010
@@ -1158,6 +1158,11 @@
// result_saved is false the result is in rax.
bool result_saved = false;
+ // Mark all computed expressions that are bound to a key that
+ // is shadowed by a later occurrence of the same key. For the
+ // marked expressions, no store code is emitted.
+ expr->CalculateEmitStore();
+
for (int i = 0; i < expr->properties()->length(); i++) {
ObjectLiteral::Property* property = expr->properties()->at(i);
if (property->IsCompileTimeValue()) continue;
@@ -1179,8 +1184,10 @@
VisitForAccumulatorValue(value);
__ Move(rcx, key->handle());
__ movq(rdx, Operand(rsp, 0));
- Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
- EmitCallIC(ic, RelocInfo::CODE_TARGET);
+ if (property->emit_store()) {
+ Handle<Code>
ic(Builtins::builtin(Builtins::StoreIC_Initialize));
+ EmitCallIC(ic, RelocInfo::CODE_TARGET);
+ }
break;
}
// Fall through.
@@ -1188,7 +1195,11 @@
__ push(Operand(rsp, 0)); // Duplicate receiver.
VisitForStackValue(key);
VisitForStackValue(value);
- __ CallRuntime(Runtime::kSetProperty, 3);
+ if (property->emit_store()) {
+ __ CallRuntime(Runtime::kSetProperty, 3);
+ } else {
+ __ Drop(3);
+ }
break;
case ObjectLiteral::Property::SETTER:
case ObjectLiteral::Property::GETTER:
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
