Author: [email protected]
Date: Tue Mar 10 23:17:19 2009
New Revision: 1482
Added:
branches/bleeding_edge/test/mjsunit/regress/regress-265.js
Removed:
branches/bleeding_edge/test/mjsunit/bugs/bug-265.js
Modified:
branches/bleeding_edge/src/assembler-arm.h
branches/bleeding_edge/src/assembler-ia32.h
branches/bleeding_edge/src/ast.h
branches/bleeding_edge/src/codegen-arm.cc
branches/bleeding_edge/src/codegen-arm.h
branches/bleeding_edge/src/codegen-ia32.cc
branches/bleeding_edge/src/codegen-ia32.h
branches/bleeding_edge/src/jump-target.cc
branches/bleeding_edge/src/jump-target.h
branches/bleeding_edge/src/virtual-frame-arm.cc
branches/bleeding_edge/src/virtual-frame-arm.h
branches/bleeding_edge/src/virtual-frame-ia32.cc
branches/bleeding_edge/src/virtual-frame-ia32.h
branches/bleeding_edge/src/virtual-frame.cc
branches/bleeding_edge/test/mjsunit/mjsunit.status
Log:
Fix issue 265 by handling extra statement state on the frame based on
the expectation at the break, continue, and return labels (including
shadowed ones) instead of based on the AST nodes.
See http://code.google.com/p/v8/issues/detail?id=265
Review URL: http://codereview.chromium.org/42017
Modified: branches/bleeding_edge/src/assembler-arm.h
==============================================================================
--- branches/bleeding_edge/src/assembler-arm.h (original)
+++ branches/bleeding_edge/src/assembler-arm.h Tue Mar 10 23:17:19 2009
@@ -218,6 +218,9 @@
// implementations.
enum Hint { no_hint };
+// Hints are not used on the arm. Negating is trivial.
+inline Hint NegateHint(Hint ignored) { return no_hint; }
+
// The pc store offset may be 8 or 12 depending on the processor
implementation.
int PcStoreOffset();
Modified: branches/bleeding_edge/src/assembler-ia32.h
==============================================================================
--- branches/bleeding_edge/src/assembler-ia32.h (original)
+++ branches/bleeding_edge/src/assembler-ia32.h Tue Mar 10 23:17:19 2009
@@ -177,7 +177,6 @@
taken = 0x3e
};
-
// The result of negating a hint is as if the corresponding condition
// were negated by NegateCondition. That is, no_hint is mapped to
// itself and not_taken and taken are mapped to each other.
@@ -186,6 +185,7 @@
? no_hint
: ((hint == not_taken) ? taken : not_taken);
}
+
//
-----------------------------------------------------------------------------
// Machine instruction Immediates
Modified: branches/bleeding_edge/src/ast.h
==============================================================================
--- branches/bleeding_edge/src/ast.h (original)
+++ branches/bleeding_edge/src/ast.h Tue Mar 10 23:17:19 2009
@@ -202,11 +202,6 @@
// Code generation
BreakTarget* break_target() { return &break_target_; }
- // Used during code generation for restoring the stack when a
- // break/continue crosses a statement that keeps stuff on the stack.
- int break_stack_height() { return break_stack_height_; }
- void set_break_stack_height(int height) { break_stack_height_ = height; }
-
// Testers.
bool is_target_for_anonymous() const { return type_ ==
TARGET_FOR_ANONYMOUS; }
@@ -220,7 +215,6 @@
ZoneStringList* labels_;
Type type_;
BreakTarget break_target_;
- int break_stack_height_;
};
@@ -451,12 +445,12 @@
CHECK(!is_default());
return label_;
}
- BreakTarget* body_target() { return &body_target_; }
+ JumpTarget* body_target() { return &body_target_; }
ZoneList<Statement*>* statements() const { return statements_; }
private:
Expression* label_;
- BreakTarget body_target_;
+ JumpTarget body_target_;
ZoneList<Statement*>* statements_;
};
Modified: branches/bleeding_edge/src/codegen-arm.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-arm.cc (original)
+++ branches/bleeding_edge/src/codegen-arm.cc Tue Mar 10 23:17:19 2009
@@ -83,7 +83,6 @@
allocator_(NULL),
cc_reg_(al),
state_(NULL),
- break_stack_height_(0),
function_return_is_shadowed_(false),
in_spilled_code_(false) {
}
@@ -107,8 +106,6 @@
ASSERT(frame_ == NULL);
frame_ = new VirtualFrame(this);
cc_reg_ = al;
- function_return_.Initialize(this, JumpTarget::BIDIRECTIONAL);
- function_return_is_shadowed_ = false;
set_in_spilled_code(false);
{
CodeGenState state(this);
@@ -133,6 +130,10 @@
// Allocate space for locals and initialize them.
frame_->AllocateStackSlots(scope_->num_stack_slots());
+ // Initialize the function return target after the locals are set
+ // up, because it needs the expected frame height from the frame.
+ function_return_.Initialize(this, JumpTarget::BIDIRECTIONAL);
+ function_return_is_shadowed_ = false;
VirtualFrame::SpilledScope spilled_scope(this);
if (scope_->num_heap_slots() > 0) {
@@ -1127,7 +1128,6 @@
VirtualFrame::SpilledScope spilled_scope(this);
Comment cmnt(masm_, "[ Block");
CodeForStatementPosition(node);
- node->set_break_stack_height(break_stack_height_);
node->break_target()->Initialize(this);
VisitStatementsAndSpill(node->statements());
if (node->break_target()->is_linked()) {
@@ -1341,18 +1341,10 @@
}
-void CodeGenerator::CleanStack(int num_bytes) {
- VirtualFrame::SpilledScope spilled_scope(this);
- ASSERT(num_bytes % kPointerSize == 0);
- frame_->Drop(num_bytes / kPointerSize);
-}
-
-
void CodeGenerator::VisitContinueStatement(ContinueStatement* node) {
VirtualFrame::SpilledScope spilled_scope(this);
Comment cmnt(masm_, "[ ContinueStatement");
CodeForStatementPosition(node);
- CleanStack(break_stack_height_ - node->target()->break_stack_height());
node->target()->continue_target()->Jump();
}
@@ -1361,7 +1353,6 @@
VirtualFrame::SpilledScope spilled_scope(this);
Comment cmnt(masm_, "[ BreakStatement");
CodeForStatementPosition(node);
- CleanStack(break_stack_height_ - node->target()->break_stack_height());
node->target()->break_target()->Jump();
}
@@ -1521,7 +1512,6 @@
VirtualFrame::SpilledScope spilled_scope(this);
Comment cmnt(masm_, "[ SwitchStatement");
CodeForStatementPosition(node);
- node->set_break_stack_height(break_stack_height_);
node->break_target()->Initialize(this);
LoadAndSpill(node->tag());
@@ -1615,7 +1605,6 @@
VirtualFrame::SpilledScope spilled_scope(this);
Comment cmnt(masm_, "[ LoopStatement");
CodeForStatementPosition(node);
- node->set_break_stack_height(break_stack_height_);
node->break_target()->Initialize(this);
// Simple condition analysis. ALWAYS_TRUE and ALWAYS_FALSE represent a
@@ -1805,15 +1794,6 @@
Comment cmnt(masm_, "[ ForInStatement");
CodeForStatementPosition(node);
- // We keep stuff on the stack while the body is executing.
- // Record it, so that a break/continue crossing this statement
- // can restore the stack.
- const int kForInStackSize = 5 * kPointerSize;
- break_stack_height_ += kForInStackSize;
- node->set_break_stack_height(break_stack_height_);
- node->break_target()->Initialize(this);
- node->continue_target()->Initialize(this);
-
JumpTarget primitive(this);
JumpTarget jsobject(this);
JumpTarget fixed_array(this);
@@ -1902,6 +1882,11 @@
// sp[2] : array or enum cache
// sp[3] : 0 or map
// sp[4] : enumerable
+ // Grab the current frame's height for the break and continue
+ // targets only after all the state is pushed on the frame.
+ node->break_target()->Initialize(this);
+ node->continue_target()->Initialize(this);
+
__ ldr(r0, frame_->ElementAt(0)); // load the current count
__ ldr(r1, frame_->ElementAt(1)); // load the length
__ cmp(r0, Operand(r1)); // compare to the array length
@@ -1986,7 +1971,6 @@
// Exit.
exit.Bind();
- break_stack_height_ -= kForInStackSize;
ASSERT(frame_->height() == original_height);
}
@@ -2259,16 +2243,11 @@
frame_->EmitPush(r2);
// We keep two elements on the stack - the (possibly faked) result
- // and the state - while evaluating the finally block. Record it, so
- // that a break/continue crossing this statement can restore the
- // stack.
- const int kFinallyStackSize = 2 * kPointerSize;
- break_stack_height_ += kFinallyStackSize;
-
+ // and the state - while evaluating the finally block.
+ //
// Generate code for the statements in the finally block.
VisitStatementsAndSpill(node->finally_block()->statements());
- break_stack_height_ -= kFinallyStackSize;
if (has_valid_frame()) {
JumpTarget exit(this);
// Restore state and return value or faked TOS.
Modified: branches/bleeding_edge/src/codegen-arm.h
==============================================================================
--- branches/bleeding_edge/src/codegen-arm.h (original)
+++ branches/bleeding_edge/src/codegen-arm.h Tue Mar 10 23:17:19 2009
@@ -330,7 +330,6 @@
// Control flow
void Branch(bool if_true, JumpTarget* target);
void CheckStack();
- void CleanStack(int num_bytes);
bool CheckForInlineRuntimeCall(CallRuntime* node);
Handle<JSFunction> BuildBoilerplate(FunctionLiteral* node);
@@ -446,7 +445,6 @@
RegisterAllocator* allocator_;
Condition cc_reg_;
CodeGenState* state_;
- int break_stack_height_;
// Jump targets
BreakTarget function_return_;
Modified: branches/bleeding_edge/src/codegen-ia32.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-ia32.cc (original)
+++ branches/bleeding_edge/src/codegen-ia32.cc Tue Mar 10 23:17:19 2009
@@ -80,7 +80,6 @@
frame_(NULL),
allocator_(NULL),
state_(NULL),
- break_stack_height_(0),
loop_nesting_(0),
function_return_is_shadowed_(false),
in_spilled_code_(false) {
@@ -107,8 +106,6 @@
allocator_ = ®ister_allocator;
ASSERT(frame_ == NULL);
frame_ = new VirtualFrame(this);
- function_return_.Initialize(this, JumpTarget::BIDIRECTIONAL);
- function_return_is_shadowed_ = false;
set_in_spilled_code(false);
// Adjust for function-level loop nesting.
@@ -125,7 +122,7 @@
// esi: callee's context
allocator_->Initialize();
frame_->Enter();
- // tos: code slot
+
#ifdef DEBUG
if (strlen(FLAG_stop_at) > 0 &&
fun->name()->IsEqualTo(CStrVector(FLAG_stop_at))) {
@@ -136,6 +133,10 @@
// Allocate space for locals and initialize them.
frame_->AllocateStackSlots(scope_->num_stack_slots());
+ // Initialize the function return target after the locals are set
+ // up, because it needs the expected frame height from the frame.
+ function_return_.Initialize(this, JumpTarget::BIDIRECTIONAL);
+ function_return_is_shadowed_ = false;
// Allocate the arguments object and copy the parameters into it.
if (scope_->arguments() != NULL) {
@@ -1561,7 +1562,6 @@
ASSERT(!in_spilled_code());
Comment cmnt(masm_, "[ Block");
CodeForStatementPosition(node);
- node->set_break_stack_height(break_stack_height_);
node->break_target()->Initialize(this);
VisitStatements(node->statements());
if (node->break_target()->is_linked()) {
@@ -1763,17 +1763,10 @@
}
-void CodeGenerator::CleanStack(int num_bytes) {
- ASSERT(num_bytes % kPointerSize == 0);
- frame_->Drop(num_bytes / kPointerSize);
-}
-
-
void CodeGenerator::VisitContinueStatement(ContinueStatement* node) {
ASSERT(!in_spilled_code());
Comment cmnt(masm_, "[ ContinueStatement");
CodeForStatementPosition(node);
- CleanStack(break_stack_height_ - node->target()->break_stack_height());
node->target()->continue_target()->Jump();
}
@@ -1782,7 +1775,6 @@
ASSERT(!in_spilled_code());
Comment cmnt(masm_, "[ BreakStatement");
CodeForStatementPosition(node);
- CleanStack(break_stack_height_ - node->target()->break_stack_height());
node->target()->break_target()->Jump();
}
@@ -2020,7 +2012,6 @@
ASSERT(!in_spilled_code());
Comment cmnt(masm_, "[ SwitchStatement");
CodeForStatementPosition(node);
- node->set_break_stack_height(break_stack_height_);
node->break_target()->Initialize(this);
// Compile the switch value.
@@ -2148,7 +2139,6 @@
ASSERT(!in_spilled_code());
Comment cmnt(masm_, "[ LoopStatement");
CodeForStatementPosition(node);
- node->set_break_stack_height(break_stack_height_);
node->break_target()->Initialize(this);
// Simple condition analysis. ALWAYS_TRUE and ALWAYS_FALSE represent a
@@ -2471,15 +2461,6 @@
Comment cmnt(masm_, "[ ForInStatement");
CodeForStatementPosition(node);
- // We keep stuff on the stack while the body is executing.
- // Record it, so that a break/continue crossing this statement
- // can restore the stack.
- const int kForInStackSize = 5 * kPointerSize;
- break_stack_height_ += kForInStackSize;
- node->set_break_stack_height(break_stack_height_);
- node->break_target()->Initialize(this);
- node->continue_target()->Initialize(this);
-
JumpTarget primitive(this);
JumpTarget jsobject(this);
JumpTarget fixed_array(this);
@@ -2568,6 +2549,11 @@
// Condition.
entry.Bind();
+ // Grab the current frame's height for the break and continue
+ // targets only after all the state is pushed on the frame.
+ node->break_target()->Initialize(this);
+ node->continue_target()->Initialize(this);
+
__ mov(eax, frame_->ElementAt(0)); // load the current count
__ cmp(eax, frame_->ElementAt(1)); // compare to the array length
node->break_target()->Branch(above_equal);
@@ -2650,8 +2636,6 @@
// Exit.
exit.Bind();
-
- break_stack_height_ -= kForInStackSize;
}
@@ -2917,16 +2901,11 @@
frame_->EmitPush(ecx);
// We keep two elements on the stack - the (possibly faked) result
- // and the state - while evaluating the finally block. Record it, so
- // that a break/continue crossing this statement can restore the
- // stack.
- const int kFinallyStackSize = 2 * kPointerSize;
- break_stack_height_ += kFinallyStackSize;
-
+ // and the state - while evaluating the finally block.
+ //
// Generate code for the statements in the finally block.
VisitStatementsAndSpill(node->finally_block()->statements());
- break_stack_height_ -= kFinallyStackSize;
if (has_valid_frame()) {
JumpTarget exit(this);
// Restore state and return value or faked TOS.
Modified: branches/bleeding_edge/src/codegen-ia32.h
==============================================================================
--- branches/bleeding_edge/src/codegen-ia32.h (original)
+++ branches/bleeding_edge/src/codegen-ia32.h Tue Mar 10 23:17:19 2009
@@ -501,7 +501,6 @@
void CallWithArguments(ZoneList<Expression*>* arguments, int position);
void CheckStack();
- void CleanStack(int num_bytes);
bool CheckForInlineRuntimeCall(CallRuntime* node);
Handle<JSFunction> BuildBoilerplate(FunctionLiteral* node);
@@ -617,7 +616,6 @@
VirtualFrame* frame_;
RegisterAllocator* allocator_;
CodeGenState* state_;
- int break_stack_height_;
int loop_nesting_;
// Jump targets.
Modified: branches/bleeding_edge/src/jump-target.cc
==============================================================================
--- branches/bleeding_edge/src/jump-target.cc (original)
+++ branches/bleeding_edge/src/jump-target.cc Tue Mar 10 23:17:19 2009
@@ -37,23 +37,48 @@
JumpTarget::JumpTarget(CodeGenerator* cgen, Directionality direction)
: cgen_(cgen),
- masm_(cgen == NULL ? NULL : cgen->masm()),
direction_(direction),
reaching_frames_(0),
merge_labels_(0),
entry_frame_(NULL),
is_bound_(false),
is_linked_(false) {
+ ASSERT(cgen != NULL);
+ masm_ = cgen->masm();
+}
+
+
+JumpTarget::JumpTarget()
+ : cgen_(NULL),
+ masm_(NULL),
+ direction_(FORWARD_ONLY),
+ reaching_frames_(0),
+ merge_labels_(0),
+ entry_frame_(NULL),
+ is_bound_(false),
+ is_linked_(false) {
+}
+
+
+void JumpTarget::Initialize(CodeGenerator* cgen, Directionality direction)
{
+ ASSERT(cgen != NULL);
+ ASSERT(cgen_ == NULL);
+ cgen_ = cgen;
+ masm_ = cgen->masm();
+ direction_ = direction;
}
void JumpTarget::Unuse() {
ASSERT(!is_linked());
- entry_label_.Unuse();
+#ifdef DEBUG
+ for (int i = 0; i < reaching_frames_.length(); i++) {
+ ASSERT(reaching_frames_[i] == NULL);
+ }
+#endif
delete entry_frame_;
- entry_frame_ = NULL;
- is_bound_ = false;
- is_linked_ = false;
+
+ Reset();
}
@@ -501,16 +526,10 @@
//
-------------------------------------------------------------------------
// BreakTarget implementation.
-BreakTarget::BreakTarget() : JumpTarget(NULL, FORWARD_ONLY) {
-}
-
-
void BreakTarget::Initialize(CodeGenerator* cgen, Directionality
direction) {
- ASSERT(cgen != NULL);
- ASSERT(cgen_ == NULL);
- cgen_ = cgen;
- masm_ = cgen->masm();
- direction_ = direction;
+ JumpTarget::Initialize(cgen, direction);
+ ASSERT(cgen_->has_valid_frame());
+ expected_height_ = cgen_->frame()->height();
}
@@ -530,6 +549,58 @@
destination->entry_label_ = entry_label_;
destination->is_bound_ = is_bound_;
destination->is_linked_ = is_linked_;
+ destination->expected_height_ = expected_height_;
+}
+
+
+void BreakTarget::Jump() {
+ ASSERT(cgen_ != NULL);
+ ASSERT(cgen_->has_valid_frame());
+
+ // This is a break target so drop leftover statement state from the
+ // frame before merging.
+ cgen_->frame()->ForgetElements(cgen_->frame()->height() -
expected_height_);
+ JumpTarget::Jump();
+}
+
+
+void BreakTarget::Branch(Condition cc, Hint hint) {
+ ASSERT(cgen_ != NULL);
+ ASSERT(cgen_->has_valid_frame());
+
+ int count = cgen_->frame()->height() - expected_height_;
+ if (count > 0) {
+ // We negate and branch here rather than using
+ // JumpTarget::Branch's negate and branch. This gives us a hook
+ // to remove statement state from the frame.
+ JumpTarget fall_through(cgen_);
+ // Branch to fall through will not negate, because it is a
+ // forward-only target.
+ fall_through.Branch(NegateCondition(cc), NegateHint(hint));
+ Jump(); // May emit merge code here.
+ fall_through.Bind();
+ } else {
+ JumpTarget::Branch(cc, hint);
+ }
+}
+
+
+void BreakTarget::Bind(int mergable_elements) {
+#ifdef DEBUG
+ ASSERT(mergable_elements == kAllElements);
+ ASSERT(cgen_ != NULL);
+ for (int i = 0; i < reaching_frames_.length(); i++) {
+ ASSERT(reaching_frames_[i]->height() == expected_height_);
+ }
+#endif
+
+ // This is a break target so drop leftover statement state from the
+ // frame before merging.
+ if (cgen_->has_valid_frame()) {
+ int count = cgen_->frame()->height() - expected_height_;
+ cgen_->frame()->ForgetElements(count);
+ }
+ JumpTarget::Bind(mergable_elements);
}
@@ -546,15 +617,19 @@
// While shadowing this shadow target saves the state of the original.
shadowed->CopyTo(this);
+ // The original's state is reset. We do not Unuse it because that
+ // would delete the expected frame and assert that the target is not
+ // linked.
+ shadowed->Reset();
+ ASSERT(cgen_ != NULL);
+ ASSERT(cgen_->has_valid_frame());
+ shadowed->set_expected_height(cgen_->frame()->height());
+
// Setting the code generator to null prevents the shadow target from
// being used until shadowing stops.
cgen_ = NULL;
masm_ = NULL;
- // The original's state is reset. We do not Unuse it because that
- // would delete the expected frame and assert that the target is not
- // linked.
- shadowed->Reset();
}
Modified: branches/bleeding_edge/src/jump-target.h
==============================================================================
--- branches/bleeding_edge/src/jump-target.h (original)
+++ branches/bleeding_edge/src/jump-target.h Tue Mar 10 23:17:19 2009
@@ -58,8 +58,31 @@
explicit JumpTarget(CodeGenerator* cgen,
Directionality direction = FORWARD_ONLY);
+ // Construct a jump target without a code generator. A code
+ // generator must be supplied before using the jump target as a
+ // label. This is useful, eg, when break targets are embedded in
+ // AST nodes.
+ JumpTarget();
+
+ // Supply a code generator and directionality to an already
+ // constructed jump target. This function expects to be given a
+ // non-null code generator, and to be called only when the code
+ // generator is not yet set.
+ virtual void Initialize(CodeGenerator* cgen,
+ Directionality direction = FORWARD_ONLY);
+
virtual ~JumpTarget() { Unuse(); }
+ // Treat the jump target as a fresh one. The state is reset and
+ // pointed-to virtual frames are deallocated. There should be no
+ // dangling jumps to the target.
+ void Unuse();
+
+ // Reset the internal state of this jump target. Pointed-to virtual
+ // frames are not deallocated and dangling jumps to the target are
+ // left dangling.
+ void Reset();
+
// Accessors.
CodeGenerator* code_generator() const { return cgen_; }
@@ -77,19 +100,9 @@
bool is_linked() const { return is_linked_; }
bool is_unused() const { return !is_bound() && !is_linked(); }
- // Treat the jump target as a fresh one. The expected frame if any
- // will be deallocated and there should be no dangling jumps to the
- // target (thus no reaching frames).
- void Unuse();
-
- // Reset the internal state of this jump target. Pointed-to virtual
- // frames are not deallocated and dangling jumps to the target are
- // left dangling.
- void Reset();
-
// Emit a jump to the target. There must be a current frame at the
// jump and there will be no current frame after the jump.
- void Jump();
+ virtual void Jump();
void Jump(Result* arg);
void Jump(Result* arg0, Result* arg1);
void Jump(Result* arg0, Result* arg1, Result* arg2);
@@ -97,7 +110,7 @@
// Emit a conditional branch to the target. There must be a current
// frame at the branch. The current frame will fall through to the
// code after the branch.
- void Branch(Condition cc, Hint hint = no_hint);
+ virtual void Branch(Condition cc, Hint hint = no_hint);
void Branch(Condition cc, Result* arg, Hint hint = no_hint);
void Branch(Condition cc, Result* arg0, Result* arg1, Hint hint =
no_hint);
void Branch(Condition cc,
@@ -125,7 +138,7 @@
// A mergable elements argument of kAllElements indicates that all
// frame elements must be mergable. Mergable elements are ignored
// completely for forward-only jump targets.
- void Bind(int mergable_elements = kAllElements);
+ virtual void Bind(int mergable_elements = kAllElements);
void Bind(Result* arg, int mergable_elements = kAllElements);
void Bind(Result* arg0, Result* arg1, int mergable_elements =
kAllElements);
void Bind(Result* arg0,
@@ -209,14 +222,14 @@
// generator must be supplied before using the break target as a
// label. This is useful, eg, when break targets are embedded in AST
// nodes.
- BreakTarget();
+ BreakTarget() {}
- // Supply a code generator and directionality to an already
- // constructed jump target. This function expects to be given a
- // non-null code generator, and to be called only when the code
- // generator is not yet set.
- void Initialize(CodeGenerator* cgen,
- Directionality direction = FORWARD_ONLY);
+ // Supply a code generator, expected expression stack height, and
+ // directionality to an already constructed break target. This
+ // function expects to be given a non-null code generator, and to be
+ // called only when the code generator is not yet set.
+ virtual void Initialize(CodeGenerator* cgen,
+ Directionality direction = FORWARD_ONLY);
// Copy the state of this break target to the destination. The
// lists of forward-reaching frames and merge-point labels are
@@ -225,7 +238,28 @@
// overwritten, without deallocating pointed-to virtual frames.
void CopyTo(BreakTarget* destination);
+ // Emit a jump to the target. There must be a current frame at the
+ // jump and there will be no current frame after the jump.
+ virtual void Jump();
+
+ // Emit a conditional branch to the target. There must be a current
+ // frame at the branch. The current frame will fall through to the
+ // code after the branch.
+ virtual void Branch(Condition cc, Hint hint = no_hint);
+
+ // Bind a break target. If there is no current frame at the binding
+ // site, there must be at least one frame reaching via a forward
+ // jump.
+ virtual void Bind(int mergable_elements = kAllElements);
+
+ // Setter for expected height.
+ void set_expected_height(int expected) { expected_height_ = expected; }
+
private:
+ // The expected height of the expression stack where the target will
+ // be bound, statically known at initialization time.
+ int expected_height_;
+
DISALLOW_COPY_AND_ASSIGN(BreakTarget);
};
Modified: branches/bleeding_edge/src/virtual-frame-arm.cc
==============================================================================
--- branches/bleeding_edge/src/virtual-frame-arm.cc (original)
+++ branches/bleeding_edge/src/virtual-frame-arm.cc Tue Mar 10 23:17:19 2009
@@ -168,8 +168,11 @@
MergeMoveRegistersToRegisters(expected);
MergeMoveMemoryToRegisters(expected);
- // Fix any sync bit problems.
- for (int i = 0; i <= stack_pointer_; i++) {
+ // Fix any sync bit problems from the bottom-up, stopping when we
+ // hit the stack pointer or the top of the frame if the stack
+ // pointer is floating above the frame.
+ int limit = Min(stack_pointer_, elements_.length() - 1);
+ for (int i = 0; i <= limit; i++) {
FrameElement source = elements_[i];
FrameElement target = expected->elements_[i];
if (source.is_synced() && !target.is_synced()) {
@@ -197,17 +200,15 @@
// Move registers, constants, and copies to memory. Perform moves
// from the top downward in the frame in order to leave the backing
// stores of copies in registers.
- //
- // Moving memory-backed copies to memory requires a spare register
- // for the memory-to-memory moves. Since we are performing a merge,
- // we use esi (which is already saved in the frame). We keep track
- // of the index of the frame element esi is caching or kIllegalIndex
- // if esi has not been disturbed.
+ // On ARM, all elements are in memory.
- for (int i = 0; i < elements_.length(); i++) {
+#ifdef DEBUG
+ int start = Min(stack_pointer_, elements_.length() - 1);
+ for (int i = start; i >= 0; i--) {
ASSERT(elements_[i].is_memory());
ASSERT(expected->elements_[i].is_memory());
}
+#endif
}
Modified: branches/bleeding_edge/src/virtual-frame-arm.h
==============================================================================
--- branches/bleeding_edge/src/virtual-frame-arm.h (original)
+++ branches/bleeding_edge/src/virtual-frame-arm.h Tue Mar 10 23:17:19 2009
@@ -84,6 +84,11 @@
// the frame after a runtime call). No code is emitted.
void Forget(int count);
+ // Forget count elements from the top of the frame without adjusting
+ // the stack pointer downward. This is used, for example, before
+ // merging frames at break, continue, and return targets.
+ void ForgetElements(int count);
+
// Spill all values from the frame to memory.
void SpillAll();
Modified: branches/bleeding_edge/src/virtual-frame-ia32.cc
==============================================================================
--- branches/bleeding_edge/src/virtual-frame-ia32.cc (original)
+++ branches/bleeding_edge/src/virtual-frame-ia32.cc Tue Mar 10 23:17:19
2009
@@ -171,8 +171,11 @@
MergeMoveRegistersToRegisters(expected);
MergeMoveMemoryToRegisters(expected);
- // Fix any sync bit problems.
- for (int i = 0; i <= stack_pointer_; i++) {
+ // Fix any sync bit problems from the bottom-up, stopping when we
+ // hit the stack pointer or the top of the frame if the stack
+ // pointer is floating above the frame.
+ int limit = Min(stack_pointer_, elements_.length() - 1);
+ for (int i = 0; i <= limit; i++) {
FrameElement source = elements_[i];
FrameElement target = expected->elements_[i];
if (source.is_synced() && !target.is_synced()) {
@@ -209,7 +212,10 @@
int esi_caches = kIllegalIndex;
// A "singleton" memory element.
FrameElement memory_element = FrameElement::MemoryElement();
- for (int i = stack_pointer_; i >= 0; i--) {
+ // Loop downward from the stack pointer or the top of the frame if
+ // the stack pointer is floating above the frame.
+ int start = Min(stack_pointer_, elements_.length() - 1);
+ for (int i = start; i >= 0; i--) {
FrameElement target = expected->elements_[i];
if (target.is_memory()) {
FrameElement source = elements_[i];
Modified: branches/bleeding_edge/src/virtual-frame-ia32.h
==============================================================================
--- branches/bleeding_edge/src/virtual-frame-ia32.h (original)
+++ branches/bleeding_edge/src/virtual-frame-ia32.h Tue Mar 10 23:17:19 2009
@@ -80,10 +80,18 @@
// emitted.
void Adjust(int count);
- // Forget elements from the top of the frame to match an actual frame
(eg,
- // the frame after a runtime call). No code is emitted.
+ // Forget count elements from the top of the frame all in-memory
+ // (including synced) and adjust the stack pointer downward, to
+ // match an external frame effect (examples include a call removing
+ // its arguments, and exiting a try/catch removing an exception
+ // handler). No code will be emitted.
void Forget(int count);
+ // Forget count elements from the top of the frame without adjusting
+ // the stack pointer downward. This is used, for example, before
+ // merging frames at break, continue, and return targets.
+ void ForgetElements(int count);
+
// Spill all values from the frame to memory.
void SpillAll();
@@ -124,8 +132,7 @@
void Enter();
void Exit();
- // Prepare for returning from the frame by spilling locals and
- // dropping all non-locals elements in the virtual frame. This
+ // Prepare for returning from the frame by spilling locals. This
// avoids generating unnecessary merge code when jumping to the
// shared return site. Emits code for spills.
void PrepareForReturn();
Modified: branches/bleeding_edge/src/virtual-frame.cc
==============================================================================
--- branches/bleeding_edge/src/virtual-frame.cc (original)
+++ branches/bleeding_edge/src/virtual-frame.cc Tue Mar 10 23:17:19 2009
@@ -129,13 +129,27 @@
void VirtualFrame::Forget(int count) {
ASSERT(count >= 0);
ASSERT(stack_pointer_ == elements_.length() - 1);
- ASSERT(elements_.length() >= count);
stack_pointer_ -= count;
+ ForgetElements(count);
+}
+
+
+void VirtualFrame::ForgetElements(int count) {
+ ASSERT(count >= 0);
+ ASSERT(elements_.length() >= count);
+
for (int i = 0; i < count; i++) {
FrameElement last = elements_.RemoveLast();
if (last.is_register()) {
- Unuse(last.reg());
+ // A hack to properly count register references for the code
+ // generator's current frame and also for other frames. The
+ // same code appears in PrepareMergeTo.
+ if (cgen_->frame() == this) {
+ Unuse(last.reg());
+ } else {
+ frame_registers_.Unuse(last.reg());
+ }
}
}
}
@@ -324,15 +338,11 @@
void VirtualFrame::PrepareForReturn() {
// Spill all locals. This is necessary to make sure all locals have
// the right value when breaking at the return site in the debugger.
+ //
+ // TODO(203): It is also necessary to ensure that merging at the
+ // return site does not generate code to overwrite eax, where the
+ // return value is kept in a non-refcounted register reference.
for (int i = 0; i < expression_base_index(); i++) SpillElementAt(i);
-
- // Drop all non-local stack elements.
- Drop(height());
-
- // Validate state: The expression stack should be empty and the
- // stack pointer should have been updated to reflect this.
- ASSERT(height() == 0);
- ASSERT(stack_pointer_ == expression_base_index() - 1);
}
Modified: branches/bleeding_edge/test/mjsunit/mjsunit.status
==============================================================================
--- branches/bleeding_edge/test/mjsunit/mjsunit.status (original)
+++ branches/bleeding_edge/test/mjsunit/mjsunit.status Tue Mar 10 23:17:19
2009
@@ -34,10 +34,6 @@
# too long to run in debug mode on ARM.
fuzz-natives: PASS, SKIP if ($mode == release || $arch == arm)
-# Bug on IA32, realiably triggers a debug assertion. Unpredictable in
-# release mode on IA32, currently passes on ARM.
-bugs/bug-265: (PASS || FAIL || CRASH), FAIL if $mode == debug
-
[ $arch == arm ]
# Slow tests which times out in debug mode.
Added: branches/bleeding_edge/test/mjsunit/regress/regress-265.js
==============================================================================
--- (empty file)
+++ branches/bleeding_edge/test/mjsunit/regress/regress-265.js Tue Mar 10
23:17:19 2009
@@ -0,0 +1,64 @@
+// 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.
+
+// When returning or breaking out of a deeply nested try/finally, we
+// should not crash.
+
+// See http://code.google.com/p/v8/issues/detail?id=263
+
+function test0() {
+ try {
+ try {
+ return 0;
+ } finally {
+ try {
+ return 0;
+ } finally {
+ }
+ }
+ } finally {
+ }
+}
+
+test0();
+
+function test1() {
+L0:
+ try {
+ try {
+ break L0;
+ } finally {
+ try {
+ break L0;
+ } finally {
+ }
+ }
+ } finally {
+ }
+}
+
+test1();
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
