Support stepping out for recursive functions.Simply flooding JS function from...

Support stepping out for recursive functions.Simply flooding JS function from the calling stack frame with one shot breakpoints is not enough to support step out action in all cases since the function on top of the stack may be turn recursive and we may end up flooding itself. To overcome this a pointer to the stack frame where the debugger should be invoked after stepping out is strored in the debugger.Chromium bug:http://code.google.com/p/chromium/issues/detail?id=17967
Review URL: http://codereview.chromium.org/200041

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2850 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/debug.cc

@@ -518,6 +518,7 @@ void Debug::ThreadInit() {
   thread_local_.step_count_ = 0;
   thread_local_.last_fp_ = 0;
   thread_local_.step_into_fp_ = 0;
+  thread_local_.step_out_fp_ = 0;
   thread_local_.after_break_target_ = 0;
   thread_local_.debugger_entry_ = NULL;
   thread_local_.pending_interrupts_ = 0;
@@ -864,11 +865,18 @@ Object* Debug::Break(Arguments args) {
     break_points_hit = CheckBreakPoints(break_point_objects);
   }
 
-  // Notify debugger if a real break point is triggered or if performing single
-  // stepping with no more steps to perform. Otherwise do another step.
-  if (!break_points_hit->IsUndefined() ||
-    (thread_local_.last_step_action_ != StepNone &&
-     thread_local_.step_count_ == 0)) {
+  // If step out is active skip everything until the frame where we need to step
+  // out to is reached, unless real breakpoint is hit.
+  if (Debug::StepOutActive() && frame->fp() != Debug::step_out_fp() &&
+      break_points_hit->IsUndefined() ) {
+      // Step count should always be 0 for StepOut.
+      ASSERT(thread_local_.step_count_ == 0);
+  } else if (!break_points_hit->IsUndefined() ||
+             (thread_local_.last_step_action_ != StepNone &&
+              thread_local_.step_count_ == 0)) {
+    // Notify debugger if a real break point is triggered or if performing
+    // single stepping with no more steps to perform. Otherwise do another step.
+
     // Clear all current stepping setup.
     ClearStepping();
 
@@ -1104,7 +1112,13 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
 
   // Remember this step action and count.
   thread_local_.last_step_action_ = step_action;
-  thread_local_.step_count_ = step_count;
+  if (step_action == StepOut) {
+    // For step out target frame will be found on the stack so there is no need
+    // to set step counter for it. It's expected to always be 0 for StepOut.
+    thread_local_.step_count_ = 0;
+  } else {
+    thread_local_.step_count_ = step_count;
+  }
 
   // Get the frame where the execution has stopped and skip the debug frame if
   // any. The debug frame will only be present if execution was stopped due to
@@ -1183,13 +1197,28 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
 
   // If this is the last break code target step out is the only possibility.
   if (it.IsExit() || step_action == StepOut) {
+    if (step_action == StepOut) {
+      // Skip step_count frames starting with the current one.
+      while(step_count-- > 0 && !frames_it.done()) {
+        frames_it.Advance();
+      }
+    } else {
+      ASSERT(it.IsExit());
+      frames_it.Advance();
+    }
+    // Skip builtin functions on the stack.
+    while(!frames_it.done() &&
+          JSFunction::cast(frames_it.frame()->function())->IsBuiltin()) {
+      frames_it.Advance();
+    }
     // Step out: If there is a JavaScript caller frame, we need to
     // flood it with breakpoints.
-    frames_it.Advance();
     if (!frames_it.done()) {
       // Fill the function to return to with one-shot break points.
       JSFunction* function = JSFunction::cast(frames_it.frame()->function());
       FloodWithOneShot(Handle<SharedFunctionInfo>(function->shared()));
+      // Set target frame pointer.
+      ActivateStepOut(frames_it.frame());
     }
   } else if (!(is_inline_cache_stub || RelocInfo::IsConstructCall(it.rmode()) ||
                !call_function_stub.is_null())
@@ -1445,6 +1474,7 @@ void Debug::ClearStepping() {
   // Clear the various stepping setup.
   ClearOneShot();
   ClearStepIn();
+  ClearStepOut();
   ClearStepNext();
 
   // Clear multiple step counter.
@@ -1472,6 +1502,7 @@ void Debug::ClearOneShot() {
 
 
 void Debug::ActivateStepIn(StackFrame* frame) {
+  ASSERT(!StepOutActive());
   thread_local_.step_into_fp_ = frame->fp();
 }
 
@@ -1481,6 +1512,17 @@ void Debug::ClearStepIn() {
 }
 
 
+void Debug::ActivateStepOut(StackFrame* frame) {
+  ASSERT(!StepInActive());
+  thread_local_.step_out_fp_ = frame->fp();
+}
+
+
+void Debug::ClearStepOut() {
+  thread_local_.step_out_fp_ = 0;
+}
+
+
 void Debug::ClearStepNext() {
   thread_local_.last_step_action_ = StepNone;
   thread_local_.last_statement_position_ = RelocInfo::kNoPosition;

src/debug.h

@@ -282,6 +282,9 @@ class Debug {
   static Address step_in_fp() { return thread_local_.step_into_fp_; }
   static Address* step_in_fp_addr() { return &thread_local_.step_into_fp_; }
 
+  static bool StepOutActive() { return thread_local_.step_out_fp_ != 0; }
+  static Address step_out_fp() { return thread_local_.step_out_fp_; }
+
   static EnterDebugger* debugger_entry() {
     return thread_local_.debugger_entry_;
   }
@@ -393,6 +396,8 @@ class Debug {
   static void ClearOneShot();
   static void ActivateStepIn(StackFrame* frame);
   static void ClearStepIn();
+  static void ActivateStepOut(StackFrame* frame);
+  static void ClearStepOut();
   static void ClearStepNext();
   // Returns whether the compile succeeded.
   static bool EnsureCompiled(Handle<SharedFunctionInfo> shared);
@@ -445,6 +450,10 @@ class Debug {
     // Frame pointer for frame from which step in was performed.
     Address step_into_fp_;
 
+    // Frame pointer for the frame where debugger should be called when current
+    // step out action is completed.
+    Address step_out_fp_;
+
     // Storage location for jump when exiting debug break calls.
     Address after_break_target_;
 

src/runtime.cc

@@ -6934,7 +6934,8 @@ static Object* Runtime_ChangeBreakOnException(Arguments args) {
 // Prepare for stepping
 // args[0]: break id for checking execution state
 // args[1]: step action from the enumeration StepAction
-// args[2]: number of times to perform the step
+// args[2]: number of times to perform the step, for step out it is the number
+//          of frames to step down.
 static Object* Runtime_PrepareStep(Arguments args) {
   HandleScope scope;
   ASSERT(args.length() == 3);
@@ -6961,6 +6962,9 @@ static Object* Runtime_PrepareStep(Arguments args) {
     return Top::Throw(Heap::illegal_argument_symbol());
   }
 
+  // Clear all current stepping setup.
+  Debug::ClearStepping();
+
   // Prepare step.
   Debug::PrepareStep(static_cast<StepAction>(step_action), step_count);
   return Heap::undefined_value();

test/mjsunit/debug-stepout-recursive-function.js

+// 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.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+Debug = debug.Debug
+
+var exception = null;
+var step_out_count = 1;
+
+// Simple debug event handler which counts the number of breaks hit and steps.
+var break_point_hit_count = 0;
+function listener(event, exec_state, event_data, data) {
+  try {
+    if (event == Debug.DebugEvent.Break) {
+      break_point_hit_count++;
+      // Continue stepping until returned to bottom frame.
+      if (exec_state.frameCount() > 1) {
+        exec_state.prepareStep(Debug.StepAction.StepOut, step_out_count);
+      }
+
+    }
+  } catch(e) {
+    exception = e;
+  }
+
+};
+
+function BeginTest(name) {
+  test_name = name;
+  break_point_hit_count = 0;
+  exception = null;
+}
+
+function EndTest(expected_break_point_hit_count) {
+  assertEquals(expected_break_point_hit_count, break_point_hit_count, test_name);
+  assertNull(exception, test_name);
+  test_name = null;
+}
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+
+var shouldBreak = null;
+function fact(x) {
+  if (shouldBreak(x)) {
+    debugger;
+  }
+  if (x < 2) {
+    return 1;
+  } else {
+    return x*fact(x-1);
+  }
+}
+
+BeginTest('Test 1');
+shouldBreak = function(x) { return x == 3; };
+step_out_count = 1;
+fact(3);
+EndTest(2);
+
+BeginTest('Test 2');
+shouldBreak = function(x) { return x == 2; };
+step_out_count = 1;
+fact(3);
+EndTest(3);
+
+BeginTest('Test 3');
+shouldBreak = function(x) { return x == 1; };
+step_out_count = 2;
+fact(3);
+EndTest(2);
+
+BeginTest('Test 4');
+shouldBreak = function(x) { print(x); return x == 1 || x == 3; };
+step_out_count = 2;
+fact(3);
+EndTest(3);
+
+// Get rid of the debug event listener.
+Debug.setListener(null);

test/mjsunit/debug-stepout-to-builtin.js

+// 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.
+
+// Flags: --expose-debug-as debug
+
+// Get the Debug object exposed from the debug context global object.
+Debug = debug.Debug
+
+var exception = null;
+var state = 1;
+var expected_source_line_text = null;
+var expected_function_name = null;
+
+// Simple debug event handler which first time will cause 'step out' action
+// and than check that execution is paused inside function
+// expected_function_name.
+function listener(event, exec_state, event_data, data) {
+  try {
+    if (event == Debug.DebugEvent.Break) {
+      if (state == 1) {
+        exec_state.prepareStep(Debug.StepAction.StepOut, 2);
+        state = 2;
+      } else if (state == 2) {
+        assertEquals(expected_function_name, event_data.func().name());
+        assertEquals(expected_source_line_text,
+                     event_data.sourceLineText());
+        state = 3;
+      }
+    }
+  } catch(e) {
+    exception = e;
+  }
+};
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+var obj = {key:10};
+
+function replacer(key, value) {
+  if (key == 'key') {
+    debugger;
+  }
+  return value;
+}
+
+// Test step into function call from a function without local variables.
+function testStepOutToBuiltIn() {
+  expected_function_name = 'testStepOutToBuiltIn';
+  expected_source_line_text = '}  // expected line';
+  JSON.stringify(obj, replacer);
+}  // expected line
+
+state = 1;
+testStepOutToBuiltIn();
+assertNull(exception);
+assertEquals(3, state);
+
+// Get rid of the debug event listener.
+Debug.setListener(null);

test/mjsunit/mjsunit.status

@@ -63,6 +63,8 @@ debug-stepin-builtin: CRASH || FAIL
 debug-stepin-call-function-stub: CRASH || FAIL
 debug-stepin-constructor: CRASH, FAIL
 debug-stepin-function-call: CRASH || FAIL
+debug-stepout-recursive-function: CRASH || FAIL
+debug-stepout-to-builtin: CRASH || FAIL
 debug-step: SKIP
 debug-breakpoints: PASS || FAIL
 debug-handle: CRASH || FAIL || PASS