Revision: 2850
Author: [email protected]
Date: Wed Sep 9 01:40:59 2009
Log: 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
http://code.google.com/p/v8/source/detail?r=2850
Added:
/branches/bleeding_edge/test/mjsunit/debug-stepout-recursive-function.js
/branches/bleeding_edge/test/mjsunit/debug-stepout-to-builtin.js
Modified:
/branches/bleeding_edge/src/debug.cc
/branches/bleeding_edge/src/debug.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/mjsunit/mjsunit.status
=======================================
--- /dev/null
+++
/branches/bleeding_edge/test/mjsunit/debug-stepout-recursive-function.js
Wed Sep 9 01:40:59 2009
@@ -0,0 +1,106 @@
+// 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);
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/debug-stepout-to-builtin.js Wed
Sep 9 01:40:59 2009
@@ -0,0 +1,84 @@
+// 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);
=======================================
--- /branches/bleeding_edge/src/debug.cc Mon Sep 7 00:20:05 2009
+++ /branches/bleeding_edge/src/debug.cc Wed Sep 9 01:40:59 2009
@@ -518,6 +518,7 @@
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 @@
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 @@
// 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 @@
// 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 @@
// Clear the various stepping setup.
ClearOneShot();
ClearStepIn();
+ ClearStepOut();
ClearStepNext();
// Clear multiple step counter.
@@ -1472,6 +1502,7 @@
void Debug::ActivateStepIn(StackFrame* frame) {
+ ASSERT(!StepOutActive());
thread_local_.step_into_fp_ = frame->fp();
}
@@ -1479,6 +1510,17 @@
void Debug::ClearStepIn() {
thread_local_.step_into_fp_ = 0;
}
+
+
+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() {
=======================================
--- /branches/bleeding_edge/src/debug.h Mon Sep 7 00:20:05 2009
+++ /branches/bleeding_edge/src/debug.h Wed Sep 9 01:40:59 2009
@@ -281,6 +281,9 @@
bool is_constructor);
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 @@
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 @@
// 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_;
=======================================
--- /branches/bleeding_edge/src/runtime.cc Tue Sep 8 05:51:08 2009
+++ /branches/bleeding_edge/src/runtime.cc Wed Sep 9 01:40:59 2009
@@ -6934,7 +6934,8 @@
// 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);
@@ -6960,6 +6961,9 @@
if (step_count < 1) {
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);
=======================================
--- /branches/bleeding_edge/test/mjsunit/mjsunit.status Mon Sep 7 00:20:05
2009
+++ /branches/bleeding_edge/test/mjsunit/mjsunit.status Wed Sep 9 01:40:59
2009
@@ -63,6 +63,8 @@
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
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
