[debugger] tuned StepNext and StepOut at return position

src/debug/debug.cc

Index: src/debug/debug.cc
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index feca580bb86085ea09beca9b0046c8a3404c11d4..9a98431465ead30fbb101d08f2c693c7617b62ca 100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -389,6 +389,9 @@ void Debug::ThreadInit() {
   thread_local_.last_step_action_ = StepNone;
   thread_local_.last_statement_position_ = kNoSourcePosition;
   thread_local_.last_frame_count_ = -1;
+  thread_local_.ignore_next_stepping_break_ = false;
+  thread_local_.ignore_step_into_function_ = Smi::kZero;
+  thread_local_.step_into_function_call_requested_ = false;
   thread_local_.target_frame_count_ = -1;
   thread_local_.return_value_ = Smi::kZero;
   thread_local_.async_task_count_ = 0;
@@ -553,6 +556,7 @@ void Debug::Break(JavaScriptFrame* frame) {
       break;
     }
   }
+  step_break = step_break && !thread_local_.ignore_next_stepping_break_;
 
   // Clear all current stepping setup.
   ClearStepping();
@@ -814,7 +818,8 @@ void Debug::ClearAllBreakPoints() {
   }
 }
 
-void Debug::FloodWithOneShot(Handle<SharedFunctionInfo> shared) {
+void Debug::FloodWithOneShot(Handle<SharedFunctionInfo> shared,
+                             bool returns_only) {
   if (IsBlackboxed(shared)) return;
   // Make sure the function is compiled and has set up the debug info.
   if (!EnsureDebugInfo(shared)) return;
@@ -822,11 +827,13 @@ void Debug::FloodWithOneShot(Handle<SharedFunctionInfo> shared) {
   // Flood the function with break points.
   if (debug_info->HasDebugCode()) {
     for (CodeBreakIterator it(debug_info); !it.Done(); it.Next()) {
+      if (returns_only && !it.GetBreakLocation().IsReturn()) continue;
       it.SetDebugBreak();
     }
   }
   if (debug_info->HasDebugBytecodeArray()) {
     for (BytecodeArrayBreakIterator it(debug_info); !it.Done(); it.Next()) {
+      if (returns_only && !it.GetBreakLocation().IsReturn()) continue;
       it.SetDebugBreak();
     }
   }
@@ -876,10 +883,12 @@ MaybeHandle<FixedArray> Debug::GetHitBreakPointObjects(
 }
 
 void Debug::PrepareStepIn(Handle<JSFunction> function) {
-  CHECK(last_step_action() >= StepIn);
+  CHECK(step_into_function_call_requested());
   if (ignore_events()) return;
   if (in_debug_scope()) return;
   if (break_disabled()) return;
+  if (*function == thread_local_.ignore_step_into_function_) return;
+  thread_local_.last_step_action_ = StepIn;

[Yang, 2017/03/20 14:29:40]

We should clear thread_local_.ignore_step_into_function_ here. Otherwise it may
lead to memory leak. The function would hold onto the entire native context.

[kozy, 2017/03/20 15:52:54]

Done.

   FloodWithOneShot(Handle<SharedFunctionInfo>(function->shared(), isolate_));
 }
 
@@ -985,7 +994,6 @@ void Debug::PrepareStep(StepAction step_action) {
   feature_tracker()->Track(DebugFeatureTracker::kStepping);
 
   thread_local_.last_step_action_ = step_action;
-  UpdateHookOnFunctionCall();
 
   StackTraceFrameIterator frames_it(isolate_, frame_id);
   StandardFrame* frame = frames_it.frame();
@@ -1012,6 +1020,16 @@ void Debug::PrepareStep(StepAction step_action) {
 
   BreakLocation location = BreakLocation::FromFrame(debug_info, js_frame);
 
+  thread_local_.step_into_function_call_requested_ = step_action == StepIn;
+  if (location.IsReturn()) {
+    thread_local_.step_into_function_call_requested_ = true;
+    if (last_step_action() == StepOut) {
+      thread_local_.ignore_step_into_function_ = *function;
+    }
+    step_action = StepOut;
+  }
+  UpdateHookOnFunctionCall();
+
   // Any step at a return is a step-out.
   if (location.IsReturn()) step_action = StepOut;
   // A step-next at a tail call is a step-out.
@@ -1034,6 +1052,12 @@ void Debug::PrepareStep(StepAction step_action) {
       // Clear last position info. For stepping out it does not matter.
       thread_local_.last_statement_position_ = kNoSourcePosition;
       thread_local_.last_frame_count_ = -1;
+      if (!location.IsReturn() && !IsBlackboxed(shared)) {
+        thread_local_.target_frame_count_ = current_frame_count;
+        thread_local_.ignore_next_stepping_break_ = true;
+        FloodWithOneShot(shared, true);
+        return;
+      }
       // Skip the current frame, find the first frame we want to step out to
       // and deoptimize every frame along the way.
       bool in_current_frame = true;
@@ -1124,6 +1148,9 @@ void Debug::ClearStepping() {
 
   thread_local_.last_step_action_ = StepNone;
   thread_local_.last_statement_position_ = kNoSourcePosition;
+  thread_local_.ignore_step_into_function_ = Smi::kZero;
+  thread_local_.step_into_function_call_requested_ = false;
+  thread_local_.ignore_next_stepping_break_ = false;
   thread_local_.last_frame_count_ = -1;
   thread_local_.target_frame_count_ = -1;
   UpdateHookOnFunctionCall();
@@ -2108,7 +2135,7 @@ void Debug::UpdateState() {
 
 void Debug::UpdateHookOnFunctionCall() {
   STATIC_ASSERT(LastStepAction == StepIn);
-  hook_on_function_call_ = thread_local_.last_step_action_ == StepIn ||
+  hook_on_function_call_ = thread_local_.step_into_function_call_requested_ ||
                            isolate_->needs_side_effect_check();
 }