Debugger should not stop in its own code and in code of built-in functions...

Debugger should not stop in its own code and in code of built-in functions since it may confuse user.Debug break handler checks whether current function is a built-in or a debugger one and just resumes execution if it is.

CallCheckStackGuardState is no longer called in loop in the RegExp code as it hangs if debug break flag is not reset after calling Execution::HandleStackGuardInterrupt.
Review URL: http://codereview.chromium.org/160001

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

src/execution.cc

@@ -587,6 +587,23 @@ Object* Execution::DebugBreakHelper() {
     return Heap::undefined_value();
   }
 
+  {
+    JavaScriptFrameIterator it;
+    ASSERT(!it.done());
+    Object* fun = it.frame()->function();
+    if (fun && fun->IsJSFunction()) {
+      GlobalObject* global = JSFunction::cast(fun)->context()->global();
+      // Don't stop in builtin functions.
+      if (global == Top::context()->builtins()) {
+       return Heap::undefined_value();
+      }
+      // Don't stop in debugger functions.
+      if (Debug::IsDebugGlobal(global)) {
+       return Heap::undefined_value();
+      }
+    }
+  }
+
   // Collect the break state before clearing the flags.
   bool debug_command_only =
       StackGuard::IsDebugCommand() && !StackGuard::IsDebugBreak();

src/ia32/regexp-macro-assembler-ia32.cc

@@ -634,11 +634,9 @@ Handle<Object> RegExpMacroAssemblerIA32::GetCode(Handle<String> source) {
   __ push(Immediate(0));  // Make room for "input start - 1" constant.
 
   // Check if we have space on the stack for registers.
-  Label retry_stack_check;
   Label stack_limit_hit;
   Label stack_ok;
 
-  __ bind(&retry_stack_check);
   ExternalReference stack_guard_limit =
       ExternalReference::address_of_stack_guard_limit();
   __ mov(ecx, esp);
@@ -658,10 +656,7 @@ Handle<Object> RegExpMacroAssemblerIA32::GetCode(Handle<String> source) {
   CallCheckStackGuardState(ebx);
   __ or_(eax, Operand(eax));
   // If returned value is non-zero, we exit with the returned value as result.
-  // Otherwise it was a preemption and we just check the limit again.
-  __ j(equal, &retry_stack_check);
-  // Return value was non-zero. Exit with exception or retry.
-  __ jmp(&exit_label_);
+  __ j(not_zero, &exit_label_);
 
   __ bind(&stack_ok);
 
@@ -762,19 +757,11 @@ Handle<Object> RegExpMacroAssemblerIA32::GetCode(Handle<String> source) {
     __ push(backtrack_stackpointer());
     __ push(edi);
 
-    Label retry;
-
-    __ bind(&retry);
     CallCheckStackGuardState(ebx);
     __ or_(eax, Operand(eax));
     // If returning non-zero, we should end execution with the given
     // result as return value.
     __ j(not_zero, &exit_label_);
-    // Check if we are still preempted.
-    ExternalReference stack_guard_limit =
-        ExternalReference::address_of_stack_guard_limit();
-    __ cmp(esp, Operand::StaticVariable(stack_guard_limit));
-    __ j(below_equal, &retry);
 
     __ pop(edi);
     __ pop(backtrack_stackpointer());

test/cctest/test-debug.cc

@@ -4875,7 +4875,7 @@ TEST(DebugBreakInMessageHandler) {
   v8::Debug::SetMessageHandler2(DebugBreakMessageHandler);
 
   // Test functions.
-  const char* script = "function f() { debugger; } function g() { }";
+  const char* script = "function f() { debugger; g(); } function g() { }";
   CompileRun(script);
   v8::Local<v8::Function> f =
       v8::Local<v8::Function>::Cast(env->Global()->Get(v8::String::New("f")));
@@ -4954,8 +4954,10 @@ TEST(RegExpDebugBreak) {
   v8::Debug::DebugBreak();
   result = f->Call(env->Global(), argc, argv);
 
-  CHECK_EQ(20, break_point_hit_count);
-  CHECK_EQ("exec", last_function_hit);
+  // Check that there was only one break event. Matching RegExp should not
+  // cause Break events.
+  CHECK_EQ(1, break_point_hit_count);
+  CHECK_EQ("f", last_function_hit);
 }
 #endif  // V8_NATIVE_REGEXP
 
@@ -5295,3 +5297,63 @@ TEST(ProvisionalBreakpointOnLineOutOfRange) {
   ClearBreakPointFromJS(sbp2);
   v8::Debug::SetMessageHandler2(NULL);
 }
+
+
+static void BreakMessageHandler(const v8::Debug::Message& message) {
+  if (message.IsEvent() && message.GetEvent() == v8::Break) {
+    // Count the number of breaks.
+    break_point_hit_count++;
+
+    v8::HandleScope scope;
+    v8::Handle<v8::String> json = message.GetJSON();
+
+    SendContinueCommand();
+  } else if (message.IsEvent() && message.GetEvent() == v8::AfterCompile) {
+    v8::HandleScope scope;
+
+    bool is_debug_break = i::StackGuard::IsDebugBreak();
+    // Force DebugBreak flag while serializer is working.
+    i::StackGuard::DebugBreak();
+
+    // Force serialization to trigger some internal JS execution.
+    v8::Handle<v8::String> json = message.GetJSON();
+
+    // Restore previous state.
+    if (is_debug_break) {
+      i::StackGuard::DebugBreak();
+    } else {
+      i::StackGuard::Continue(i::DEBUGBREAK);
+    }
+  }
+}
+
+
+// Test that if DebugBreak is forced it is ignored when code from
+// debug-delay.js is executed. 
+TEST(NoDebugBreakInAfterCompileMessageHandler) {
+  v8::HandleScope scope;
+  DebugLocalContext env;
+
+  // Register a debug event listener which sets the break flag and counts.
+  v8::Debug::SetMessageHandler2(BreakMessageHandler);
+
+  // Set the debug break flag.
+  v8::Debug::DebugBreak();
+
+  // Create a function for testing stepping.
+  const char* src = "function f() { eval('var x = 10;'); } ";
+  v8::Local<v8::Function> f = CompileFunction(&env, src, "f");
+
+  // There should be only one break event.
+  CHECK_EQ(1, break_point_hit_count);
+
+  // Set the debug break flag again.
+  v8::Debug::DebugBreak();
+  f->Call(env->Global(), 0, NULL);
+  // There should be one more break event when the script is evaluated in 'f'.
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Get rid of the debug message handler.
+  v8::Debug::SetMessageHandler2(NULL);
+  CheckDebuggerUnloaded();
+}