Reviewers: Søren Gjesse,
Message:
The assertion failure was benign in this case, because the subexpressions
(object is a stack or context-allocated variable proxy, key is a number
literal)
do not have side effects so they couldn't be targeted by deoptimization.
Description:
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
Please review this at http://codereview.chromium.org/6368007/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/arm/full-codegen-arm.cc
M src/ia32/full-codegen-ia32.cc
M src/x64/full-codegen-x64.cc
A test/mjsunit/regress/regress-1060.js
Index: src/arm/full-codegen-arm.cc
diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index
338e39cbe823ddbe4f32556268284ca01c770727..d8ca130da44442a962364e2fac3d80b2a5bf7b56
100644
--- a/src/arm/full-codegen-arm.cc
+++ b/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<Code>
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);
}
}
Index: src/ia32/full-codegen-ia32.cc
diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index
2622b5e5b45f2b24224ae6c6c02f3c1e30e0b08c..ea947eedaf940009e375493c78b6ecfc30511859
100644
--- a/src/ia32/full-codegen-ia32.cc
+++ b/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<Code>
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);
}
}
Index: src/x64/full-codegen-x64.cc
diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index
724a7c598ac6fbfb520cb81f8397b9e0fd729fcf..b6e81b0f14a47b2ccd515bc889cb026c3db317bb
100644
--- a/src/x64/full-codegen-x64.cc
+++ b/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<Code>
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);
}
}
Index: test/mjsunit/regress/regress-1060.js
diff --git a/test/mjsunit/regress/regress-1060.js
b/test/mjsunit/regress/regress-1060.js
new file mode 100644
index
0000000000000000000000000000000000000000..8abe178d80e2ee35476d97d77343c29ca6292f1f
--- /dev/null
+++ b/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"; }));
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
