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

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

diff --git a/src/debug.cc b/src/debug.cc
index f31b475bfb9814937f1e99a56e3e78e70fcc03fb..820a4a7a4551744eb28f14d2205b6b8ea5c0355a 100644 (file)
--- a/src/debug.cc
+++ b/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();
 

diff --git a/src/debug.h b/src/debug.h
index a5f9b201ccc52ba7ee58fce9f809092e428eaf82..b86e62bf4ca979c030ebce5804edce79ab04d84b 100644 (file)
--- a/src/debug.h
+++ b/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?

diff --git a/src/runtime.cc b/src/runtime.cc
index c9277b26b930b672682822e0e6da4e642dcebecf..8c3d043d97f7da4583ea4e1de18374a46c2852f2 100644 (file)
--- a/src/runtime.cc
+++ b/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()) {

diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc
index 570256e27a262f37c9b6af395bc702da613e7ec2..924eeecda93168a320a36c5a46ecb243bf652ddc 100644 (file)
--- a/test/cctest/test-debug.cc
+++ b/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 =