Revision: 3370 Author: [email protected] Date: Thu Nov 26 13:13:20 2009 Log: Fix bug in the fast compiler's object literal code
Fixes issue 526: BUG=http://code.google.com/p/v8/issues/detail?id=526 The object literals code in the fast compiler returned an incorrect result when getter or setters are defined together with computed properties. Added a regression test that captures the most reduced version of this problem. Also added a test for object literals with getters/setters and prototype properties. Review URL: http://codereview.chromium.org/444001 http://code.google.com/p/v8/source/detail?r=3370 Added: /branches/bleeding_edge/test/mjsunit/compiler/objectliterals.js /branches/bleeding_edge/test/mjsunit/regress/regress-526.js Modified: /branches/bleeding_edge/src/arm/fast-codegen-arm.cc /branches/bleeding_edge/src/flag-definitions.h /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc /branches/bleeding_edge/src/x64/fast-codegen-x64.cc ======================================= --- /dev/null +++ /branches/bleeding_edge/test/mjsunit/compiler/objectliterals.js Thu Nov 26 13:13:20 2009 @@ -0,0 +1,57 @@ +// Copyright 2009 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 object literals with getter, setter and prototype properties. + +var o = { x: 41, get bar() { return {x:42} } }; + +assertEquals(41, o.x); +assertEquals(42, o.bar.x); + +o = { f: function() { return 41 }, + get bar() { return this.x }, + x:0, + set bar(t) { this.x = t }, + g: function() { return 43 } +}; +o.bar = 7; +assertEquals(7, o.bar); +assertEquals(7, o.x); +assertEquals(41, o.f()); +assertEquals(43, o.g()); + +p = {x:42}; +o = {get foo() { return this.x; }, + f: function() { return this.foo + 1 }, + set bar(t) { this.x = t; }, + __proto__: p, +}; +assertEquals(42, o.x); +assertEquals(42, o.foo); +assertEquals(43, o.f()); +o.bar = 44; +assertEquals(44, o.foo); ======================================= --- /dev/null +++ /branches/bleeding_edge/test/mjsunit/regress/regress-526.js Thu Nov 26 13:13:20 2009 @@ -0,0 +1,32 @@ +// Copyright 2009 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 object literals with computed property and getter. + +var o = { foo: function() { }, get bar() { return {x:42} } }; + +assertEquals(42, o.bar.x); ======================================= --- /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Thu Nov 26 02:28:32 2009 +++ /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Thu Nov 26 13:13:20 2009 @@ -665,8 +665,9 @@ __ CallRuntime(Runtime::kCloneShallowLiteralBoilerplate, 1); } - // If result_saved == true: the result is saved on top of the stack. - // If result_saved == false: the result is in r0. + // If result_saved == true: The result is saved on top of the + // stack and in r0. + // If result_saved == false: The result not on the stack, just in r0. bool result_saved = false; for (int i = 0; i < expr->properties()->length(); i++) { @@ -694,6 +695,7 @@ Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize)); __ Call(ic, RelocInfo::CODE_TARGET); // StoreIC leaves the receiver on the stack. + __ ldr(r0, MemOperand(sp)); // Restore result into r0. break; } // Fall through. @@ -705,7 +707,7 @@ Visit(value); ASSERT_EQ(Expression::kValue, value->context()); __ CallRuntime(Runtime::kSetProperty, 3); - __ ldr(r0, MemOperand(sp)); // Restore result into r0 + __ ldr(r0, MemOperand(sp)); // Restore result into r0. break; case ObjectLiteral::Property::GETTER: // Fall through. ======================================= --- /branches/bleeding_edge/src/flag-definitions.h Wed Nov 25 02:11:34 2009 +++ /branches/bleeding_edge/src/flag-definitions.h Thu Nov 26 13:13:20 2009 @@ -143,7 +143,7 @@ DEFINE_bool(strict, false, "strict error checking") DEFINE_int(min_preparse_length, 1024, "minimum length for automatic enable preparsing") -DEFINE_bool(fast_compiler, false, +DEFINE_bool(fast_compiler, true, "use the fast-mode compiler for some top-level code") DEFINE_bool(trace_bailout, false, "print reasons for failing to use fast compilation") ======================================= --- /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc Thu Nov 26 02:28:32 2009 +++ /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc Thu Nov 26 13:13:20 2009 @@ -665,8 +665,9 @@ __ CallRuntime(Runtime::kCloneLiteralBoilerplate, 1); } - // If result_saved == true: the result is saved on top of the stack. - // If result_saved == false: the result not on the stack, just is in eax. + // If result_saved == true: The result is saved on top of the + // stack and in eax. + // If result_saved == false: The result not on the stack, just in eax. bool result_saved = false; for (int i = 0; i < expr->properties()->length(); i++) { @@ -691,6 +692,7 @@ Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize)); __ call(ic, RelocInfo::CODE_TARGET); // StoreIC leaves the receiver on the stack. + __ mov(eax, Operand(esp, 0)); // Restore result into eax. break; } // fall through ======================================= --- /branches/bleeding_edge/src/x64/fast-codegen-x64.cc Thu Nov 26 02:28:32 2009 +++ /branches/bleeding_edge/src/x64/fast-codegen-x64.cc Thu Nov 26 13:13:20 2009 @@ -666,8 +666,9 @@ __ CallRuntime(Runtime::kCloneLiteralBoilerplate, 1); } - // If result_saved == true: the result is saved on top of the stack. - // If result_saved == false: the result is not on the stack, just in rax. + // If result_saved == true: The result is saved on top of the + // stack and in rax. + // If result_saved == false: The result not on the stack, just in rax. bool result_saved = false; for (int i = 0; i < expr->properties()->length(); i++) { @@ -692,6 +693,7 @@ Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize)); __ call(ic, RelocInfo::CODE_TARGET); // StoreIC leaves the receiver on the stack. + __ movq(rax, Operand(rsp, 0)); // Restore result back into rax. break; } // fall through -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
