Changed the debugger break handling to support situations where there are no...

Changed the debugger break handling to support situations where there are no stack frames. This can happen when an exception is thrown when compiling code.

This is related to Chromium issue 5349 (http://code.google.com/p/chromium/issues/detail?id=5349).
Review URL: http://codereview.chromium.org/13720

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

src/debug.cc

@@ -849,7 +849,12 @@ void Debug::FloodWithOneShot(Handle<SharedFunctionInfo> shared) {
 
 
 void Debug::FloodHandlerWithOneShot() {
+  // Iterate through the JavaScript stack looking for handlers.
   StackFrame::Id id = Top::break_frame_id();
+  if (id == StackFrame::NO_ID) {
+    // If there is no JavaScript stack don't do anything.
+    return;
+  }
   for (JavaScriptFrameIterator it(id); !it.done(); it.Advance()) {
     JavaScriptFrame* frame = it.frame();
     if (frame->HasHandler()) {
@@ -886,6 +891,10 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
   // hitting a break point. In other situations (e.g. unhandled exception) the
   // debug frame is not present.
   StackFrame::Id id = Top::break_frame_id();
+  if (id == StackFrame::NO_ID) {
+    // If there is no JavaScript stack don't do anything.
+    return;
+  }
   JavaScriptFrameIterator frames_it(id);
   JavaScriptFrame* frame = frames_it.frame();
 

src/debug.h

@@ -489,16 +489,17 @@ class DebugMessageThread: public Thread {
 // some reason could not be entered FailedToEnter will return true.
 class EnterDebugger BASE_EMBEDDED {
  public:
-  EnterDebugger() : set_(!it_.done()) {
-    // If there is no JavaScript frames on the stack don't switch to new break
-    // and break frame.
-    if (set_) {
-      // Store the previous break is and frame id.
-      break_id_ = Top::break_id();
-      break_frame_id_ = Top::break_frame_id();
-
-      // Create the new break info.
+  EnterDebugger() : has_js_frames_(!it_.done()) {
+    // Store the previous break id and frame id.
+    break_id_ = Top::break_id();
+    break_frame_id_ = Top::break_frame_id();
+
+    // Create the new break info. If there is no JavaScript frames there is no
+    // break frame id.
+    if (has_js_frames_) {
       Top::new_break(it_.frame()->id());
+    } else {
+      Top::new_break(StackFrame::NO_ID);
     }
 
     // Make sure that debugger is loaded and enter the debugger context.
@@ -511,21 +512,19 @@ class EnterDebugger BASE_EMBEDDED {
   }
 
   ~EnterDebugger() {
-    if (set_) {
-      // Restore to the previous break state.
-      Top::set_break(break_frame_id_, break_id_);
-    }
+    // Restore to the previous break state.
+    Top::set_break(break_frame_id_, break_id_);
   }
 
   // Check whether the debugger could be entered.
   inline bool FailedToEnter() { return load_failed_; }
 
   // Check whether there are any JavaScript frames on the stack.
-  inline bool HasJavaScriptFrames() { return set_; }
+  inline bool HasJavaScriptFrames() { return has_js_frames_; }
 
  private:
   JavaScriptFrameIterator it_;
-  const bool set_;  // Was the break actually set?
+  const bool has_js_frames_;  // Were there any JavaScript frames?
   StackFrame::Id break_frame_id_;  // Previous break frame id.
   int break_id_;  // Previous break id.
   bool load_failed_;  // Did the debugger fail to load?

src/runtime.cc

@@ -4764,10 +4764,8 @@ static Object* Runtime_DebugIndexedInterceptorElementValue(Arguments args) {
 static Object* Runtime_CheckExecutionState(Arguments args) {
   ASSERT(args.length() >= 1);
   CONVERT_NUMBER_CHECKED(int, break_id, Int32, args[0]);
-  // Check that the break id is valid and that there is a valid frame
-  // where execution is broken.
-  if (break_id != Top::break_id() ||
-      Top::break_frame_id() == StackFrame::NO_ID) {
+  // Check that the break id is valid.
+  if (Top::break_id() == 0 || break_id != Top::break_id()) {
     return Top::Throw(Heap::illegal_execution_state_symbol());
   }
 
@@ -4786,6 +4784,10 @@ static Object* Runtime_GetFrameCount(Arguments args) {
   // Count all frames which are relevant to debugging stack trace.
   int n = 0;
   StackFrame::Id id = Top::break_frame_id();
+  if (id == StackFrame::NO_ID) {
+    // If there is no JavaScript stack frame count is 0.
+    return Smi::FromInt(0);
+  }
   for (JavaScriptFrameIterator it(id); !it.done(); it.Advance()) n++;
   return Smi::FromInt(n);
 }
@@ -4827,6 +4829,10 @@ static Object* Runtime_GetFrameDetails(Arguments args) {
 
   // Find the relevant frame with the requested index.
   StackFrame::Id id = Top::break_frame_id();
+  if (id == StackFrame::NO_ID) {
+    // If there are no JavaScript stack frames return undefined.
+    return Heap::undefined_value();
+  }
   int count = 0;
   JavaScriptFrameIterator it(id);
   for (; !it.done(); it.Advance()) {

test/cctest/test-debug.cc

@@ -434,6 +434,15 @@ const char* frame_function_name_source =
     "}";
 v8::Local<v8::Function> frame_function_name;
 
+
+// Source for The JavaScript function which returns the number of frames.
+static const char* frame_count_source =
+    "function frame_count(exec_state) {"
+    "  return exec_state.frameCount();"
+    "}";
+v8::Handle<v8::Function> frame_count;
+
+
 // Global variable to store the last function hit - used by some tests.
 char last_function_hit[80];
 
@@ -443,6 +452,9 @@ static void DebugEventBreakPointHitCount(v8::DebugEvent event,
                                          v8::Handle<v8::Object> exec_state,
                                          v8::Handle<v8::Object> event_data,
                                          v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   // Count the number of breaks.
   if (event == v8::Break) {
     break_point_hit_count++;
@@ -464,9 +476,11 @@ static void DebugEventBreakPointHitCount(v8::DebugEvent event,
 }
 
 
-// Debug event handler which counts a number of events.
+// Debug event handler which counts a number of events and collects the stack
+// height if there is a function compiled for that.
 int exception_hit_count = 0;
 int uncaught_exception_hit_count = 0;
+int last_js_stack_height = -1;
 
 static void DebugEventCounterClear() {
   break_point_hit_count = 0;
@@ -478,6 +492,9 @@ static void DebugEventCounter(v8::DebugEvent event,
                               v8::Handle<v8::Object> exec_state,
                               v8::Handle<v8::Object> event_data,
                               v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   // Count the number of breaks.
   if (event == v8::Break) {
     break_point_hit_count++;
@@ -493,6 +510,16 @@ static void DebugEventCounter(v8::DebugEvent event,
       uncaught_exception_hit_count++;
     }
   }
+
+  // Collect the JavsScript stack height if the function frame_count is
+  // compiled.
+  if (!frame_count.IsEmpty()) {
+    static const int kArgc = 1;
+    v8::Handle<v8::Value> argv[kArgc] = { exec_state };
+    // Using exec_state as receiver is just to have a receiver.
+    v8::Handle<v8::Value> result =  frame_count->Call(exec_state, kArgc, argv);
+    last_js_stack_height = result->Int32Value();
+  }
 }
 
 
@@ -523,6 +550,9 @@ static void DebugEventEvaluate(v8::DebugEvent event,
                                v8::Handle<v8::Object> exec_state,
                                v8::Handle<v8::Object> event_data,
                                v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   if (event == v8::Break) {
     for (int i = 0; checks[i].expr != NULL; i++) {
       const int argc = 3;
@@ -546,6 +576,9 @@ static void DebugEventRemoveBreakPoint(v8::DebugEvent event,
                                        v8::Handle<v8::Object> exec_state,
                                        v8::Handle<v8::Object> event_data,
                                        v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   if (event == v8::Break) {
     break_point_hit_count++;
     v8::Handle<v8::Function> fun = v8::Handle<v8::Function>::Cast(data);
@@ -561,6 +594,9 @@ static void DebugEventStep(v8::DebugEvent event,
                            v8::Handle<v8::Object> exec_state,
                            v8::Handle<v8::Object> event_data,
                            v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   if (event == v8::Break) {
     break_point_hit_count++;
     PrepareStep(step_action);
@@ -584,6 +620,9 @@ static void DebugEventStepSequence(v8::DebugEvent event,
                                    v8::Handle<v8::Object> exec_state,
                                    v8::Handle<v8::Object> event_data,
                                    v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   if (event == v8::Break || event == v8::Exception) {
     // Check that the current function is the expected.
     CHECK(break_point_hit_count <
@@ -611,6 +650,9 @@ static void DebugEventBreakPointCollectGarbage(
     v8::Handle<v8::Object> exec_state,
     v8::Handle<v8::Object> event_data,
     v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   // Perform a garbage collection when break point is hit and continue. Based
   // on the number of break points hit either scavenge or mark compact
   // collector is used.
@@ -633,6 +675,9 @@ static void DebugEventBreak(v8::DebugEvent event,
                             v8::Handle<v8::Object> exec_state,
                             v8::Handle<v8::Object> event_data,
                             v8::Handle<v8::Value> data) {
+  // When hitting a debug event listener there must be a break set.
+  CHECK(v8::internal::Top::is_break());
+
   if (event == v8::Break) {
     // Count the number of breaks.
     break_point_hit_count++;
@@ -2164,7 +2209,7 @@ TEST(DebugStepNatives) {
 
 // Test break on exceptions. For each exception break combination the number
 // of debug event exception callbacks and message callbacks are collected. The
-// number of debug event exception callbacks are cused to check that the
+// number of debug event exception callbacks are used to check that the
 // debugger is called correctly and the number of message callbacks is used to
 // check that uncaught exceptions are still returned even if there is a break
 // for them.
@@ -2309,6 +2354,60 @@ TEST(BreakOnException) {
 }
 
 
+// Test break on exception from compiler errors. When compiling using
+// v8::Script::Compile there is no JavaScript stack whereas when compiling using
+// eval there are JavaScript frames.
+TEST(BreakOnCompileException) {
+  v8::HandleScope scope;
+  DebugLocalContext env;
+
+  v8::internal::Top::TraceException(false);
+
+  // Create a function for checking the function when hitting a break point.
+  frame_count = CompileFunction(&env, frame_count_source, "frame_count");
+
+  v8::V8::AddMessageListener(MessageCallbackCount);
+  v8::Debug::AddDebugEventListener(DebugEventCounter);
+
+  DebugEventCounterClear();
+  MessageCallbackCountClear();
+
+  // Check initial state.
+  CHECK_EQ(0, exception_hit_count);
+  CHECK_EQ(0, uncaught_exception_hit_count);
+  CHECK_EQ(0, message_callback_count);
+  CHECK_EQ(-1, last_js_stack_height);
+
+  // Throws SyntaxError: Unexpected end of input
+  v8::Script::Compile(v8::String::New("+++"));
+  CHECK_EQ(1, exception_hit_count);
+  CHECK_EQ(1, uncaught_exception_hit_count);
+  CHECK_EQ(1, message_callback_count);
+  CHECK_EQ(0, last_js_stack_height);  // No JavaScript stack.
+
+  // Throws SyntaxError: Unexpected identifier
+  v8::Script::Compile(v8::String::New("x x"));
+  CHECK_EQ(2, exception_hit_count);
+  CHECK_EQ(2, uncaught_exception_hit_count);
+  CHECK_EQ(2, message_callback_count);
+  CHECK_EQ(0, last_js_stack_height);  // No JavaScript stack.
+
+  // Throws SyntaxError: Unexpected end of input
+  v8::Script::Compile(v8::String::New("eval('+++')"))->Run();
+  CHECK_EQ(3, exception_hit_count);
+  CHECK_EQ(3, uncaught_exception_hit_count);
+  CHECK_EQ(3, message_callback_count);
+  CHECK_EQ(1, last_js_stack_height);
+
+  // Throws SyntaxError: Unexpected identifier
+  v8::Script::Compile(v8::String::New("eval('x x')"))->Run();
+  CHECK_EQ(4, exception_hit_count);
+  CHECK_EQ(4, uncaught_exception_hit_count);
+  CHECK_EQ(4, message_callback_count);
+  CHECK_EQ(1, last_js_stack_height);
+}
+
+
 TEST(StepWithException) {
   v8::HandleScope scope;
   DebugLocalContext env;
@@ -3168,14 +3267,6 @@ TEST(SendCommandToUninitializedVM) {
 }
 
 
-// Source for The JavaScript function which returns the number of frames.
-static const char* frame_count_source =
-    "function frame_count(exec_state) {"
-    "  return exec_state.frameCount();"
-    "}";
-v8::Handle<v8::Function> frame_count;
-
-
 // Source for a JavaScript function which returns the source line for the top
 // frame.
 static const char* frame_source_line_source =