Commit 2bb6e197 authored by yangguo's avatar yangguo Committed by Commit bot

[debugger] simplify step over recursive function call.

The problem is this: when stepping over a recursive function call,
the recursive function is flooded with one-shot break points so that
we break after the call, but since the callee is the same function,
the callee is also flooded, resulting a break in the callee. That
however would have been a "step in" instead of "step over".

The original solution was to recognize this by comparing FP. If we
end up in Debug::Break, we still have to check the current FP against
the remembered FP to see whether we are on the same stack height.
If we are deeper, then it's not a "step over", and we do not trigger
a debug break event. In that case, we queue up the step-over, and
temporarily step out until we hit the desired stack height. Note that
in order to step out, we flood the caller, which in our example is
the same function as the callee. So we break at every flooded break
location, and comparing with FP to make sure we stepped out prevents
us from triggering debug break events.

The new solution simply ignores breaks when the FP compare fails.
We simply carry on until we hit a break where the FP compare succeeds.
There is no need to do a step out. The number of calls to Debug::Break
that do not trigger a debug break event due to failing FP compare is
the same. But the code is a lot easier to read.

R=jkummerow@chromium.org

Review URL: https://codereview.chromium.org/1527253002

Cr-Commit-Position: refs/heads/master@{#32897}
parent 4d1906d2
......@@ -329,7 +329,6 @@ void Debug::ThreadInit() {
thread_local_.last_statement_position_ = RelocInfo::kNoPosition;
thread_local_.step_count_ = 0;
thread_local_.last_fp_ = 0;
thread_local_.queued_step_count_ = 0;
thread_local_.step_out_fp_ = 0;
thread_local_.step_in_enabled_ = false;
// TODO(isolates): frames_are_dropped_?
......@@ -484,47 +483,17 @@ void Debug::Break(Arguments args, JavaScriptFrame* frame) {
// Clear all current stepping setup.
ClearStepping();
if (thread_local_.queued_step_count_ > 0) {
// Perform queued steps
int step_count = thread_local_.queued_step_count_;
// Clear queue
thread_local_.queued_step_count_ = 0;
PrepareStep(StepNext, step_count);
} else {
// Notify the debug event listeners.
OnDebugBreak(break_points_hit, false);
}
// Notify the debug event listeners.
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 goes deeper in code, StepOut until original frame
// and keep step count queued up in the meantime.
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();
}
// Check that we indeed found the frame we are looking for.
CHECK(!it.done() && (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_count_ = step_count - 1;
}
// Set up for StepOut to reach target frame.
step_action = StepOut;
step_count = count;
}
// If StepNext goes deeper into code, just return. The functions we need
// to have flooded with one-shots are already flooded.
if (step_action == StepNext && frame->fp() < thread_local_.last_fp_) return;
// Clear all current stepping setup.
ClearStepping();
......
......@@ -630,9 +630,6 @@ class Debug {
// Frame pointer from last step next or step frame action.
Address last_fp_;
// Number of queued steps left to perform before debug event.
int queued_step_count_;
// Frame pointer for the frame where debugger should be called when current
// step out action is completed.
Address step_out_fp_;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment