Fix Debug::Break crash.

BUG=131642
TEST=test-debug/Regress131642

Review URL: https://chromiumcodereview.appspot.com/10698123

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

src/debug.cc

@@ -892,27 +892,6 @@ void Debug::Iterate(ObjectVisitor* v) {
 }
 
 
-// TODO(131642): Remove this when fixed.
-void Debug::PutValuesOnStackAndDie(int start,
-                                   Address c_entry_fp,
-                                   Address last_fp,
-                                   Address larger_fp,
-                                   int count,
-                                   char* stack,
-                                   int end) {
-  OS::PrintError("start:       %d\n", start);
-  OS::PrintError("c_entry_fp:  %p\n", static_cast<void*>(c_entry_fp));
-  OS::PrintError("last_fp:     %p\n", static_cast<void*>(last_fp));
-  OS::PrintError("larger_fp:   %p\n", static_cast<void*>(larger_fp));
-  OS::PrintError("count:       %d\n", count);
-  if (stack != NULL) {
-    OS::PrintError("stack:       %s\n", stack);
-  }
-  OS::PrintError("end:         %d\n", end);
-  OS::Abort();
-}
-
-
 Object* Debug::Break(Arguments args) {
   Heap* heap = isolate_->heap();
   HandleScope scope(isolate_);
@@ -1010,53 +989,16 @@ Object* Debug::Break(Arguments args) {
         it.Advance();
       }
 
-      // TODO(131642): Remove this when fixed.
-      // Catch the cases that would lead to crashes and capture
-      // - C entry FP at which to start stack crawl.
-      // - FP of the frame at which we plan to stop stepping out (last FP).
-      // - current FP that's larger than last FP.
-      // - Counter for the number of steps to step out.
-      // - stack trace string.
-      if (it.done()) {
-        // We crawled the entire stack, never reaching last_fp_.
-        Handle<String> stack = isolate_->StackTraceString();
-        char buffer[8192];
-        int length = Min(8192, stack->length());
-        String::WriteToFlat(*stack, buffer, 0, length - 1);
-        PutValuesOnStackAndDie(0xBEEEEEEE,
-                               frame->fp(),
-                               thread_local_.last_fp_,
-                               reinterpret_cast<Address>(0xDEADDEAD),
-                               count,
-                               buffer,
-                               0xCEEEEEEE);
-      } else if (it.frame()->fp() != thread_local_.last_fp_) {
-        // We crawled over last_fp_, without getting a match.
-        Handle<String> stack = isolate_->StackTraceString();
-        char buffer[8192];
-        int length = Min(8192, stack->length());
-        String::WriteToFlat(*stack, buffer, 0, length - 1);
-        PutValuesOnStackAndDie(0xDEEEEEEE,
-                               frame->fp(),
-                               thread_local_.last_fp_,
-                               it.frame()->fp(),
-                               count,
-                               buffer,
-                               0xFEEEEEEE);
+      // 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;
       }
 
-      // If we found original frame
-      if (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;
-      }
+      // Set up for StepOut to reach target frame.
+      step_action = StepOut;
+      step_count = count;
     }
 
     // Clear all current stepping setup.

src/debug.h

@@ -232,14 +232,6 @@ class Debug {
   void PreemptionWhileInDebugger();
   void Iterate(ObjectVisitor* v);
 
-  // TODO(131642): Remove this when fixed.
-  NO_INLINE(void PutValuesOnStackAndDie(int start,
-                                        Address c_entry_fp,
-                                        Address last_fp,
-                                        Address larger_fp,
-                                        int count,
-                                        char* stack,
-                                        int end));
   Object* Break(Arguments args);
   void SetBreakPoint(Handle<JSFunction> function,
                      Handle<Object> break_point_object,

src/execution.cc

@@ -132,6 +132,12 @@ static Handle<Object> Invoke(bool is_construct,
         V8::FatalProcessOutOfMemory("JS", true);
       }
     }
+#ifdef ENABLE_DEBUGGER_SUPPORT
+    // Reset stepping state when script exits with uncaught exception.
+    if (isolate->debugger()->IsDebuggerActive()) {
+      isolate->debug()->ClearStepping();
+    }
+#endif  // ENABLE_DEBUGGER_SUPPORT
     return Handle<Object>();
   } else {
     isolate->clear_pending_message();

src/runtime.cc

@@ -4893,7 +4893,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_StoreArrayLiteralElement) {
 // to a built-in function such as Array.forEach.
 RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugCallbackSupportsStepping) {
 #ifdef ENABLE_DEBUGGER_SUPPORT
-  if (!isolate->IsDebuggerActive()) return isolate->heap()->false_value();
+  if (!isolate->IsDebuggerActive() || !isolate->debug()->StepInActive()) {
+    return isolate->heap()->false_value();
+  }
   CONVERT_ARG_CHECKED(Object, callback, 0);
   // We do not step into the callback if it's a builtin or not even a function.
   if (!callback->IsJSFunction() || JSFunction::cast(callback)->IsBuiltin()) {

test/cctest/test-debug.cc

@@ -7349,4 +7349,47 @@ TEST(DebugBreakInline) {
 }
 
 
+static void DebugEventStepNext(v8::DebugEvent event,
+                               v8::Handle<v8::Object> exec_state,
+                               v8::Handle<v8::Object> event_data,
+                               v8::Handle<v8::Value> data) {
+  if (event == v8::Break) {
+    PrepareStep(StepNext);
+  }
+}
+
+
+static void RunScriptInANewCFrame(const char* source) {
+  v8::TryCatch try_catch;
+  CompileRun(source);
+  CHECK(try_catch.HasCaught());
+}
+
+
+TEST(Regress131642) {
+  // Bug description:
+  // When doing StepNext through the first script, the debugger is not reset
+  // after exiting through exception.  A flawed implementation enabling the
+  // debugger to step into Array.prototype.forEach breaks inside the callback
+  // for forEach in the second script under the assumption that we are in a
+  // recursive call.  In an attempt to step out, we crawl the stack using the
+  // recorded frame pointer from the first script and fail when not finding it
+  // on the stack.
+  v8::HandleScope scope;
+  DebugLocalContext env;
+  v8::Debug::SetDebugEventListener(DebugEventStepNext);
+
+  // We step through the first script.  It exits through an exception.  We run
+  // this inside a new frame to record a different FP than the second script
+  // would expect.
+  const char* script_1 = "debugger; throw new Error();";
+  RunScriptInANewCFrame(script_1);
+
+  // The second script uses forEach.
+  const char* script_2 = "[0].forEach(function() { });";
+  CompileRun(script_2);
+
+  v8::Debug::SetDebugEventListener(NULL);
+}
+
 #endif  // ENABLE_DEBUGGER_SUPPORT