Debugger: fix stepping next with trycatch recursion

src/debug.cc

Index: src/debug.cc
diff --git a/src/debug.cc b/src/debug.cc
index 2d58ce1f54aefba1e93be618b9a1f76a69f7bcd3..d3fff70ebdb125ef40c425740507659a26098355 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -542,6 +542,8 @@ void Debug::ThreadInit() {
   thread_local_.last_statement_position_ = RelocInfo::kNoPosition;
   thread_local_.step_count_ = 0;
   thread_local_.last_fp_ = 0;
+  thread_local_.queued_step_action_ = StepNone;
+  thread_local_.queued_step_count_ = 0;
   thread_local_.step_into_fp_ = 0;
   thread_local_.step_out_fp_ = 0;
   thread_local_.after_break_target_ = 0;
@@ -957,14 +959,52 @@ Object* Debug::Break(Arguments args) {
     // Clear all current stepping setup.
     ClearStepping();
 
-    // Notify the debug event listeners.
-    isolate_->debugger()->OnDebugBreak(break_points_hit, false);
+    if (thread_local_.queued_step_count_ > 0) {
+      // Perform queued step
+      StepAction step_action = thread_local_.queued_step_action_;
+      int step_count = thread_local_.queued_step_count_;
+
+      // Clear queue
+      thread_local_.queued_step_action_ = StepNone;
+      thread_local_.queued_step_count_ = 0;
+
+      PrepareStep(step_action, step_count);
+    } else {
+      // Notify the debug event listeners.
+      isolate_->debugger()->OnDebugBreak(break_points_hit, false);
+    }
   } else if (thread_local_.last_step_action_ != StepNone) {
     // Hold on to last step action as it is cleared by the call to
     // ClearStepping.
     StepAction step_action = thread_local_.last_step_action_;
     int step_count = thread_local_.step_count_;
 
+    // If StepNext gone deeper in code
+    // StepOut until original frame
+    if (step_action == StepNext && frame->fp() < thread_local_.last_fp_) {
+      // Count frames until target frame
+      int count = 0;
+      JavaScriptFrameIterator it(isolate_);
+      while (!it.done() && it.frame()->fp() != thread_local_.last_fp_) {
+        count++;
+        it.Advance();
+      }
+
+      // If we found original frame
+      if (it.frame()->fp() == thread_local_.last_fp_) {
+        if (step_count > 1) {
+          // Save old count and action to continue stepping after
+          // StepOut
+          thread_local_.queued_step_action_ = step_action;

[Yang, 2011/09/13 14:23:16]

From what I can see, the only step action that is ever queued up is StepNext.
Can't we remove queued_step_action_ altogether then? We already use
queued_step_count_ as indicator for whether there is anything queued up, there
is no point in storing queued_step_action_ when we already know that it can only
be StepNext (or StepNone in the uninteresting case, but then queued_step_count_
== 0 is indication enough).

+          thread_local_.queued_step_count_ = step_count - 1;
+        }
+
+        // Set up for StepOut to reach target frame
+        step_action = StepOut;
+        step_count = count;
+      }
+    }
+
     // Clear all current stepping setup.
     ClearStepping();
 
@@ -1448,6 +1488,13 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
 // steps before reporting break back to the debugger.
 bool Debug::StepNextContinue(BreakLocationIterator* break_location_iterator,
                              JavaScriptFrame* frame) {
+  // StepNext and StepOut shouldn't bring us deeper in code
+  // So last frame shouldn't be a one of parents of current frame
+  if (thread_local_.last_step_action_ == StepNext ||
+      thread_local_.last_step_action_ == StepOut) {
+    if (frame->fp() < thread_local_.last_fp_) return true;
+  }
+
   // If the step last action was step next or step in make sure that a new
   // statement is hit.
   if (thread_local_.last_step_action_ == StepNext ||