Revision: 5270
Author: [email protected]
Date: Mon Aug 16 04:43:30 2010
Log: ARM: Ensure that we are not in a spilled scope when calling
Load() or constructing a reference.
Review URL: http://codereview.chromium.org/3125011
http://code.google.com/p/v8/source/detail?r=5270
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-815.js
Modified:
/branches/bleeding_edge/src/arm/codegen-arm.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-815.js Mon Aug 16
04:43:30 2010
@@ -0,0 +1,49 @@
+// 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.
+
+// 815 describes a situation in which the ARM code generator could
+// end up in a spilled scope in code that only worked in a register
+// allocated scope. Test that this no longer happens.
+//
+// The code generated for unary + assumes that we are not in a spilled
+// scope.
+
+var o = new Object();
+
+// The code for the iterated-over object in for-in used to be emitted
+// in a spilled scope:
+for (x in +o) { }
+
+// Emitting code for the left hand side of a for-in.
+for (a[+o] in o) {}
+
+// The receiver in an obj[index](1, 2, 3) call:
+try {
+ o[+o](1,2,3)
+} catch(e) {
+ // It's OK as long as it does not hit an assert.
+}
=======================================
--- /branches/bleeding_edge/src/arm/codegen-arm.cc Mon Aug 16 00:52:49 2010
+++ /branches/bleeding_edge/src/arm/codegen-arm.cc Mon Aug 16 04:43:30 2010
@@ -519,6 +519,10 @@
void CodeGenerator::Load(Expression* expr) {
+ // We generally assume that we are not in a spilled scope for most
+ // of the code generator. A failure to ensure this caused issue 815
+ // and this assert is designed to catch similar issues.
+ frame_->AssertIsNotSpilled();
#ifdef DEBUG
int original_height = frame_->height();
#endif
@@ -675,6 +679,10 @@
expression_(expression),
type_(ILLEGAL),
persist_after_get_(persist_after_get) {
+ // We generally assume that we are not in a spilled scope for most
+ // of the code generator. A failure to ensure this caused issue 815
+ // and this assert is designed to catch similar issues.
+ cgen->frame()->AssertIsNotSpilled();
cgen->LoadReference(this);
}
@@ -1899,19 +1907,17 @@
void CodeGenerator::VisitReturnStatement(ReturnStatement* node) {
- frame_->SpillAll();
Comment cmnt(masm_, "[ ReturnStatement");
CodeForStatementPosition(node);
Load(node->expression());
+ frame_->PopToR0();
+ frame_->PrepareForReturn();
if (function_return_is_shadowed_) {
- frame_->EmitPop(r0);
function_return_.Jump();
} else {
// Pop the result from the frame and prepare the frame for
// returning thus making it easier to merge.
- frame_->PopToR0();
- frame_->PrepareForReturn();
if (function_return_.is_bound()) {
// If the function return label is already bound we reuse the
// code by jumping to the return site.
@@ -2307,7 +2313,6 @@
#ifdef DEBUG
int original_height = frame_->height();
#endif
- VirtualFrame::SpilledScope spilled_scope(frame_);
Comment cmnt(masm_, "[ ForInStatement");
CodeForStatementPosition(node);
@@ -2321,6 +2326,7 @@
// Get the object to enumerate over (converted to JSObject).
Load(node->enumerable());
+ VirtualFrame::SpilledScope spilled_scope(frame_);
// Both SpiderMonkey and kjs ignore null and undefined in contrast
// to the specification. 12.6.4 mandates a call to ToObject.
frame_->EmitPop(r0);
@@ -2490,25 +2496,31 @@
// Store the entry in the 'each' expression and take another spin in the
// loop. r3: i'th entry of the enum cache (or string there of)
frame_->EmitPush(r3); // push entry
- { Reference each(this, node->each());
+ { VirtualFrame::RegisterAllocationScope scope(this);
+ Reference each(this, node->each());
if (!each.is_illegal()) {
if (each.size() > 0) {
+ // Loading a reference may leave the frame in an unspilled state.
+ frame_->SpillAll(); // Sync stack to memory.
+ // Get the value (under the reference on the stack) from memory.
__ ldr(r0, frame_->ElementAt(each.size()));
frame_->EmitPush(r0);
each.SetValue(NOT_CONST_INIT, UNLIKELY_SMI);
- frame_->Drop(2);
+ frame_->Drop(2); // The result of the set and the extra pushed
value.
} else {
// If the reference was to a slot we rely on the convenient
property
- // that it doesn't matter whether a value (eg, r3 pushed above) is
+ // that it doesn't matter whether a value (eg, ebx pushed above) is
// right on top of or right underneath a zero-sized reference.
each.SetValue(NOT_CONST_INIT, UNLIKELY_SMI);
- frame_->Drop();
+ frame_->Drop(1); // Drop the result of the set operation.
}
}
}
// Body.
CheckStack(); // TODO(1222600): ignore if body contains calls.
- Visit(node->body());
+ { VirtualFrame::RegisterAllocationScope scope(this);
+ Visit(node->body());
+ }
// Next. Reestablish a spilled frame in case we are coming here via
// a continue in the body.
@@ -2555,7 +2567,9 @@
// Remove the exception from the stack.
frame_->Drop();
- VisitStatements(node->catch_block()->statements());
+ { VirtualFrame::RegisterAllocationScope scope(this);
+ VisitStatements(node->catch_block()->statements());
+ }
if (frame_ != NULL) {
exit.Jump();
}
@@ -2590,7 +2604,9 @@
}
// Generate code for the statements in the try block.
- VisitStatements(node->try_block()->statements());
+ { VirtualFrame::RegisterAllocationScope scope(this);
+ VisitStatements(node->try_block()->statements());
+ }
// Stop the introduced shadowing and count the number of required
unlinks.
// After shadowing stops, the original labels are unshadowed and the
@@ -2611,7 +2627,7 @@
// the handler list and drop the rest of this handler from the
// frame.
STATIC_ASSERT(StackHandlerConstants::kNextOffset == 0);
- frame_->EmitPop(r1);
+ frame_->EmitPop(r1); // r0 can contain the return value.
__ mov(r3, Operand(handler_address));
__ str(r1, MemOperand(r3));
frame_->Drop(StackHandlerConstants::kSize / kPointerSize - 1);
@@ -2637,7 +2653,7 @@
frame_->Forget(frame_->height() - handler_height);
STATIC_ASSERT(StackHandlerConstants::kNextOffset == 0);
- frame_->EmitPop(r1);
+ frame_->EmitPop(r1); // r0 can contain the return value.
__ str(r1, MemOperand(r3));
frame_->Drop(StackHandlerConstants::kSize / kPointerSize - 1);
@@ -2704,7 +2720,9 @@
}
// Generate code for the statements in the try block.
- VisitStatements(node->try_block()->statements());
+ { VirtualFrame::RegisterAllocationScope scope(this);
+ VisitStatements(node->try_block()->statements());
+ }
// Stop the introduced shadowing and count the number of required
unlinks.
// After shadowing stops, the original labels are unshadowed and the
@@ -2794,7 +2812,9 @@
// and the state - while evaluating the finally block.
//
// Generate code for the statements in the finally block.
- VisitStatements(node->finally_block()->statements());
+ { VirtualFrame::RegisterAllocationScope scope(this);
+ VisitStatements(node->finally_block()->statements());
+ }
if (has_valid_frame()) {
// Restore state and return value or faked TOS.
@@ -3974,7 +3994,6 @@
} else if (var != NULL && var->slot() != NULL &&
var->slot()->type() == Slot::LOOKUP) {
- VirtualFrame::SpilledScope spilled_scope(frame_);
// ----------------------------------
// JavaScript examples:
//
@@ -3987,8 +4006,6 @@
// }
// ----------------------------------
- // JumpTargets do not yet support merging frames so the frame must be
- // spilled when jumping to these targets.
JumpTarget slow, done;
// Generate fast case for loading functions from slots that
@@ -4002,8 +4019,7 @@
slow.Bind();
// Load the function
frame_->EmitPush(cp);
- __ mov(r0, Operand(var->name()));
- frame_->EmitPush(r0);
+ frame_->EmitPush(Operand(var->name()));
frame_->CallRuntime(Runtime::kLoadContextSlot, 2);
// r0: slot value; r1: receiver
@@ -4019,7 +4035,7 @@
call.Jump();
done.Bind();
frame_->EmitPush(r0); // function
- LoadGlobalReceiver(r1); // receiver
+ LoadGlobalReceiver(VirtualFrame::scratch0()); // receiver
call.Bind();
}
@@ -4074,8 +4090,6 @@
// -------------------------------------------
// JavaScript example: 'array[index](1, 2, 3)'
// -------------------------------------------
- VirtualFrame::SpilledScope spilled_scope(frame_);
-
Load(property->obj());
if (property->is_synthetic()) {
Load(property->key());
@@ -4083,7 +4097,7 @@
// Put the function below the receiver.
// Use the global receiver.
frame_->EmitPush(r0); // Function.
- LoadGlobalReceiver(r0);
+ LoadGlobalReceiver(VirtualFrame::scratch0());
// Call the function.
CallWithArguments(args, RECEIVER_MIGHT_BE_VALUE, node->position());
frame_->EmitPush(r0);
@@ -4096,6 +4110,7 @@
// Set the name register and call the IC initialization code.
Load(property->key());
+ frame_->SpillAll();
frame_->EmitPop(r2); // Function name.
InLoopFlag in_loop = loop_nesting() > 0 ? IN_LOOP : NOT_IN_LOOP;
@@ -4115,10 +4130,8 @@
// Load the function.
Load(function);
- VirtualFrame::SpilledScope spilled_scope(frame_);
-
// Pass the global proxy as the receiver.
- LoadGlobalReceiver(r0);
+ LoadGlobalReceiver(VirtualFrame::scratch0());
// Call the function.
CallWithArguments(args, NO_CALL_FUNCTION_FLAGS, node->position());
@@ -4173,8 +4186,8 @@
void CodeGenerator::GenerateClassOf(ZoneList<Expression*>* args) {
- JumpTarget leave, null, function, non_function_constructor;
Register scratch = VirtualFrame::scratch0();
+ JumpTarget null, function, leave, non_function_constructor;
// Load the object into register.
ASSERT(args->length() == 1);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
