[debugger] flood function for stepping on throw.

We used to flood the handler when preparing for stepping,
even if we may not throw. Instead, we now flood the
handler only when we actually throw.

This also solves an issue with step-next when we throw and
leave the function unexpectedly. In combination with
microtasks, this could cause a crash.

R=mstarzinger@chromium.org
BUG=chromium:568477
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#32856}

src/debug/debug.cc

@@ -767,22 +767,6 @@ void Debug::FloodWithOneShot(Handle<JSFunction> function,
 }
 
 
-void Debug::FloodHandlerWithOneShot() {
-  // Iterate through the JavaScript stack looking for handlers.
-  DCHECK_NE(StackFrame::NO_ID, break_frame_id());
-  for (JavaScriptFrameIterator it(isolate_, break_frame_id()); !it.done();
-       it.Advance()) {
-    JavaScriptFrame* frame = it.frame();
-    int stack_slots = 0;  // The computed stack slot count is not used.
-    if (frame->LookupExceptionHandlerInTable(&stack_slots, NULL) > 0) {
-      // Flood the function with the catch/finally block with break points.
-      FloodWithOneShot(Handle<JSFunction>(frame->function()));
-      return;
-    }
-  }
-}
-
-
 void Debug::ChangeBreakOnException(ExceptionBreakType type, bool enable) {
   if (type == BreakUncaughtException) {
     break_on_uncaught_exception_ = enable;
@@ -820,6 +804,35 @@ void Debug::PrepareStepIn(Handle<JSFunction> function) {
 }
 
 
+void Debug::PrepareStepOnThrow() {
+  if (!is_active()) return;
+  if (!IsStepping()) return;
+  if (last_step_action() == StepNone) return;
+  if (in_debug_scope()) return;
+
+  ClearOneShot();
+
+  // Iterate through the JavaScript stack looking for handlers.
+  JavaScriptFrameIterator it(isolate_);
+  while (!it.done()) {
+    JavaScriptFrame* frame = it.frame();
+    int stack_slots = 0;  // The computed stack slot count is not used.
+    if (frame->LookupExceptionHandlerInTable(&stack_slots, NULL) > 0) break;
+    it.Advance();
+  }
+
+  // Find the closest Javascript frame we can flood with one-shots.
+  while (!it.done() &&
+         !it.frame()->function()->shared()->IsSubjectToDebugging()) {
+    it.Advance();
+  }
+
+  if (it.done()) return;  // No suitable Javascript catch handler.
+
+  FloodWithOneShot(Handle<JSFunction>(it.frame()->function()));
+}
+
+
 void Debug::PrepareStep(StepAction step_action,
                         int step_count,
                         StackFrame::Id frame_id) {
@@ -856,10 +869,6 @@ void Debug::PrepareStep(StepAction step_action,
     thread_local_.step_count_ = step_count;
   }
 
-  // First of all ensure there is one-shot break points in the top handler
-  // if any.
-  FloodHandlerWithOneShot();
-
   // If the function on the top frame is unresolved perform step out. This will
   // be the case when calling unknown function and having the debugger stopped
   // in an unhandled exception.
@@ -1659,6 +1668,7 @@ MaybeHandle<Object> Debug::MakeAsyncTaskEvent(Handle<JSObject> task_event) {
 
 void Debug::OnThrow(Handle<Object> exception) {
   if (in_debug_scope() || ignore_events()) return;
+  PrepareStepOnThrow();
   // Temporarily clear any scheduled_exception to allow evaluating
   // JavaScript from the debug event handler.
   HandleScope scope(isolate_);
@@ -1720,9 +1730,6 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
   DebugScope debug_scope(this);
   if (debug_scope.failed()) return;
 
-  // Clear all current stepping setup.
-  ClearStepping();
-
   // Create the event data object.
   Handle<Object> event_data;
   // Bail out and don't call debugger if exception.

src/debug/debug.h

@@ -404,7 +404,6 @@ class Debug {
   void ClearAllBreakPoints();
   void FloodWithOneShot(Handle<JSFunction> function,
                         BreakLocatorType type = ALL_BREAK_LOCATIONS);
-  void FloodHandlerWithOneShot();
   void ChangeBreakOnException(ExceptionBreakType type, bool enable);
   bool IsBreakOnException(ExceptionBreakType type);
 
@@ -413,6 +412,7 @@ class Debug {
                    int step_count,
                    StackFrame::Id frame_id);
   void PrepareStepIn(Handle<JSFunction> function);
+  void PrepareStepOnThrow();
   void ClearStepping();
   void ClearStepOut();
   void EnableStepIn();

src/execution.cc

@@ -110,10 +110,6 @@ MUST_USE_RESULT MaybeHandle<Object> Invoke(Isolate* isolate, bool is_construct,
   DCHECK(has_exception == isolate->has_pending_exception());
   if (has_exception) {
     isolate->ReportPendingMessages();
-    // Reset stepping state when script exits with uncaught exception.
-    if (isolate->debug()->is_active()) {
-      isolate->debug()->ClearStepping();
-    }
     return MaybeHandle<Object>();
   } else {
     isolate->clear_pending_message();

test/mjsunit/mjsunit.status

@@ -990,6 +990,10 @@
   'regress/regress-crbug-527364': [SKIP],
   'regress/regress-crbug-530598': [SKIP],
   'regress/regress-crbug-546968': [SKIP],
+  'regress/regress-crbug-568477-1': [SKIP],
+  'regress/regress-crbug-568477-2': [SKIP],
+  'regress/regress-crbug-568477-3': [SKIP],
+  'regress/regress-crbug-568477-4': [SKIP],
   'regress/regress-deopt-gcb': [SKIP],
   'regress/regress-deopt-gc': [SKIP],
   'regress/regress-deopt-in-array-literal-spread': [SKIP],

test/mjsunit/regress/regress-crbug-568477-1.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug --allow-natives-syntax
+
+var Debug = debug.Debug;
+var expected = ["debugger;", "var x = y;", "debugger;", "var x = y;"];
+var log = [];
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    log.push(exec_state.frame(0).sourceLineText().trimLeft());
+    exec_state.prepareStep(Debug.StepAction.StepNext, 1);
+  } catch (e) {
+    %AbortJS(e + "\n" + e.stack);
+  }
+}
+
+Debug.setListener(listener);
+
+function f() {
+  var a = 1;
+  debugger;
+  var x = y;
+  print(x);
+}
+
+function call_f_with_deeper_stack() {
+  (() => () => () => f())()()();
+}
+
+Promise.resolve().then(f).catch(call_f_with_deeper_stack);
+
+// Schedule microtask to check against expectation at the end.
+function testDone(iteration) {
+  function checkResult() {
+    try {
+      assertTrue(iteration < 10);
+      if (expected.length == log.length) {
+        assertEquals(expected, log);
+      } else {
+        testDone(iteration + 1);
+      }
+    } catch (e) {
+      %AbortJS(e + "\n" + e.stack);
+    }
+  }
+
+  %EnqueueMicrotask(checkResult);
+}
+
+testDone(0);

test/mjsunit/regress/regress-crbug-568477-2.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug --allow-natives-syntax
+
+var Debug = debug.Debug;
+var expected = ["debugger;",
+                "var x = y;",
+                "new Promise(f).catch(call_f_with_deeper_stack);",
+                "var a = 1;", "", "var a = 1;",
+                "debugger;",
+                "var x = y;"];
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    assertEquals(expected.shift(), exec_state.frame(0).sourceLineText().trimLeft());
+    exec_state.prepareStep(Debug.StepAction.StepNext, 1);
+  } catch (e) {
+    %AbortJS(e + "\n" + e.stack);
+  }
+}
+
+Debug.setListener(listener);
+
+function f() {
+  var a = 1;
+  debugger;
+  var x = y;
+  print(x);
+}
+
+function call_f_with_deeper_stack() {
+  (() => () => () => f())()()();
+}
+
+new Promise(f).catch(call_f_with_deeper_stack);
+var a = 1;

test/mjsunit/regress/regress-crbug-568477-3.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug --allow-natives-syntax
+
+var Debug = debug.Debug;
+var expected = ["debugger;", "var x = y;", "debugger;", "var x = y;"];
+var log = [];
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    log.push(exec_state.frame(0).sourceLineText().trimLeft());
+    exec_state.prepareStep(Debug.StepAction.StepNext, 1);
+  } catch (e) {
+    %AbortJS(e + "\n" + e.stack);
+  }
+}
+
+Debug.setListener(listener);
+
+function f() {
+  var a = 1;
+  debugger;
+  var x = y;
+  print(x);
+}
+
+function call_f_with_deeper_stack() {
+  (() => () => () => f())()()();
+}
+
+var p = Promise.resolve();
+p.then(f);
+p.then(call_f_with_deeper_stack);
+
+// Schedule microtask to check against expectation at the end.
+function testDone(iteration) {
+  function checkResult() {
+    try {
+      assertTrue(iteration < 10);
+      if (expected.length == log.length) {
+        assertEquals(expected, log);
+      } else {
+        testDone(iteration + 1);
+      }
+    } catch (e) {
+      %AbortJS(e + "\n" + e.stack);
+    }
+  }
+
+  %EnqueueMicrotask(checkResult);
+}
+
+testDone(0);

test/mjsunit/regress/regress-crbug-568477-4.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug --allow-natives-syntax
+
+var Debug = debug.Debug;
+var expected =
+    ["debugger;", "var x = y;", "var b = 2;", "Debug.setListener(null);"];
+var log = [];
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    log.push(exec_state.frame(0).sourceLineText().trimLeft());
+    exec_state.prepareStep(Debug.StepAction.StepNext, 1);
+  } catch (e) {
+    %AbortJS(e + "\n" + e.stack);
+  }
+}
+
+Debug.setListener(listener);
+
+function f() {
+  var a = 1;
+  debugger;
+  var x = y;
+  print(x);
+}
+
+try {
+  %Call(f, {});
+} catch (e) {
+  var b = 2;
+}
+
+Debug.setListener(null);
+
+assertEquals(expected, log);