From 7c2869046ec3617d069b6076e12a3b56fec3d9cd Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Wed, 19 Jan 2011 15:26:54 +0000 Subject: [PATCH] Fix an assertion failure in the full code generator. We hit an assertion failure when we tried to record the AST ID of the (shared) .arguments variable proxy more than once. This was hit when we had multiple calls to the same parameter in a function that used the arguments object. The fix is to not visit the subexpressions of the (shared) property access expression. BUG=1060 Review URL: http://codereview.chromium.org/6368007 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@6404 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- deps/v8/src/arm/full-codegen-arm.cc | 22 +++++++++++++------ deps/v8/src/ia32/full-codegen-ia32.cc | 22 +++++++++++++------ deps/v8/src/x64/full-codegen-x64.cc | 22 +++++++++++++------ deps/v8/test/mjsunit/regress/regress-1060.js | 32 ++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 21 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/regress-1060.js diff --git a/deps/v8/src/arm/full-codegen-arm.cc b/deps/v8/src/arm/full-codegen-arm.cc index 338e39cb..d8ca130 100644 --- a/deps/v8/src/arm/full-codegen-arm.cc +++ b/deps/v8/src/arm/full-codegen-arm.cc @@ -1988,16 +1988,21 @@ void FullCodeGenerator::VisitCall(Call* expr) { // Call to a keyed property. // For a synthetic property use keyed load IC followed by function call, // for a regular property use keyed CallIC. - { PreservePositionScope scope(masm()->positions_recorder()); - VisitForStackValue(prop->obj()); - } if (prop->is_synthetic()) { - { PreservePositionScope scope(masm()->positions_recorder()); - VisitForAccumulatorValue(prop->key()); - } + // Do not visit the object and key subexpressions (they are shared + // by all occurrences of the same rewritten parameter). + ASSERT(prop->obj()->AsVariableProxy() != NULL); + ASSERT(prop->obj()->AsVariableProxy()->var()->AsSlot() != NULL); + Slot* slot = prop->obj()->AsVariableProxy()->var()->AsSlot(); + MemOperand operand = EmitSlotSearch(slot, r1); + __ ldr(r1, operand); + + ASSERT(prop->key()->AsLiteral() != NULL); + ASSERT(prop->key()->AsLiteral()->handle()->IsSmi()); + __ mov(r0, Operand(prop->key()->AsLiteral()->handle())); + // Record source code position for IC call. SetSourcePosition(prop->position()); - __ pop(r1); // We do not need to keep the receiver. Handle ic(Builtins::builtin(Builtins::KeyedLoadIC_Initialize)); EmitCallIC(ic, RelocInfo::CODE_TARGET); @@ -2006,6 +2011,9 @@ void FullCodeGenerator::VisitCall(Call* expr) { __ Push(r0, r1); // Function, receiver. EmitCallWithStub(expr); } else { + { PreservePositionScope scope(masm()->positions_recorder()); + VisitForStackValue(prop->obj()); + } EmitKeyedCallWithIC(expr, prop->key(), RelocInfo::CODE_TARGET); } } diff --git a/deps/v8/src/ia32/full-codegen-ia32.cc b/deps/v8/src/ia32/full-codegen-ia32.cc index 2622b5e..ea947ee 100644 --- a/deps/v8/src/ia32/full-codegen-ia32.cc +++ b/deps/v8/src/ia32/full-codegen-ia32.cc @@ -2339,16 +2339,21 @@ void FullCodeGenerator::VisitCall(Call* expr) { // Call to a keyed property. // For a synthetic property use keyed load IC followed by function call, // for a regular property use keyed EmitCallIC. - { PreservePositionScope scope(masm()->positions_recorder()); - VisitForStackValue(prop->obj()); - } if (prop->is_synthetic()) { - { PreservePositionScope scope(masm()->positions_recorder()); - VisitForAccumulatorValue(prop->key()); - } + // Do not visit the object and key subexpressions (they are shared + // by all occurrences of the same rewritten parameter). + ASSERT(prop->obj()->AsVariableProxy() != NULL); + ASSERT(prop->obj()->AsVariableProxy()->var()->AsSlot() != NULL); + Slot* slot = prop->obj()->AsVariableProxy()->var()->AsSlot(); + MemOperand operand = EmitSlotSearch(slot, edx); + __ mov(edx, operand); + + ASSERT(prop->key()->AsLiteral() != NULL); + ASSERT(prop->key()->AsLiteral()->handle()->IsSmi()); + __ mov(eax, prop->key()->AsLiteral()->handle()); + // Record source code position for IC call. SetSourcePosition(prop->position()); - __ pop(edx); // We do not need to keep the receiver. Handle ic(Builtins::builtin(Builtins::KeyedLoadIC_Initialize)); EmitCallIC(ic, RelocInfo::CODE_TARGET); @@ -2359,6 +2364,9 @@ void FullCodeGenerator::VisitCall(Call* expr) { __ push(FieldOperand(ecx, GlobalObject::kGlobalReceiverOffset)); EmitCallWithStub(expr); } else { + { PreservePositionScope scope(masm()->positions_recorder()); + VisitForStackValue(prop->obj()); + } EmitKeyedCallWithIC(expr, prop->key(), RelocInfo::CODE_TARGET); } } diff --git a/deps/v8/src/x64/full-codegen-x64.cc b/deps/v8/src/x64/full-codegen-x64.cc index 724a7c5..b6e81b0 100644 --- a/deps/v8/src/x64/full-codegen-x64.cc +++ b/deps/v8/src/x64/full-codegen-x64.cc @@ -2006,16 +2006,21 @@ void FullCodeGenerator::VisitCall(Call* expr) { // Call to a keyed property. // For a synthetic property use keyed load IC followed by function call, // for a regular property use keyed EmitCallIC. - { PreservePositionScope scope(masm()->positions_recorder()); - VisitForStackValue(prop->obj()); - } if (prop->is_synthetic()) { - { PreservePositionScope scope(masm()->positions_recorder()); - VisitForAccumulatorValue(prop->key()); - } + // Do not visit the object and key subexpressions (they are shared + // by all occurrences of the same rewritten parameter). + ASSERT(prop->obj()->AsVariableProxy() != NULL); + ASSERT(prop->obj()->AsVariableProxy()->var()->AsSlot() != NULL); + Slot* slot = prop->obj()->AsVariableProxy()->var()->AsSlot(); + MemOperand operand = EmitSlotSearch(slot, rdx); + __ movq(rdx, operand); + + ASSERT(prop->key()->AsLiteral() != NULL); + ASSERT(prop->key()->AsLiteral()->handle()->IsSmi()); + __ Move(rax, prop->key()->AsLiteral()->handle()); + // Record source code position for IC call. SetSourcePosition(prop->position()); - __ pop(rdx); // We do not need to keep the receiver. Handle ic(Builtins::builtin(Builtins::KeyedLoadIC_Initialize)); EmitCallIC(ic, RelocInfo::CODE_TARGET); @@ -2026,6 +2031,9 @@ void FullCodeGenerator::VisitCall(Call* expr) { __ push(FieldOperand(rcx, GlobalObject::kGlobalReceiverOffset)); EmitCallWithStub(expr); } else { + { PreservePositionScope scope(masm()->positions_recorder()); + VisitForStackValue(prop->obj()); + } EmitKeyedCallWithIC(expr, prop->key(), RelocInfo::CODE_TARGET); } } diff --git a/deps/v8/test/mjsunit/regress/regress-1060.js b/deps/v8/test/mjsunit/regress/regress-1060.js new file mode 100644 index 0000000..8abe178 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/regress-1060.js @@ -0,0 +1,32 @@ +// Copyright 2011 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. + +// Make sure that we do not record multiple bailouts in the unoptimized code +// for the (shared) .arguments proxy, even for calls. +function f(x) { arguments; return x() + x(); } + +assertEquals("hesthest", f(function () { return "hest"; })); -- 2.7.4