Fix crash in ScriptDebugServer::wrapCallFrames

test/cctest/test-debug.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 651 matching lines @@
     }
   }
 }
 
 
 // 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;
+v8::Handle<v8::Function> debug_event_listener_callback;
+int debug_event_listener_callback_result;
 
 static void DebugEventCounterClear() {
   break_point_hit_count = 0;
   exception_hit_count = 0;
   uncaught_exception_hit_count = 0;
 }
 
 static void DebugEventCounter(
     const v8::Debug::EventDetails& event_details) {
   v8::DebugEvent event = event_details.GetEvent();
@@ skipping 20 matching lines @@
       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);
+    v8::Handle<v8::Value> result = frame_count->Call(exec_state, kArgc, argv);
     last_js_stack_height = result->Int32Value();
   }
+
+  // Run callback from DebugEventListener and check the result.
+  if (!debug_event_listener_callback.IsEmpty()) {
+    v8::Handle<v8::Value> result =
+        debug_event_listener_callback->Call(event_data, 0, NULL);
+    CHECK(!result.IsEmpty());
+    CHECK_EQ(debug_event_listener_callback_result, result->Int32Value());
+  }
 }
 
 
 // Debug event handler which evaluates a number of expressions when a break
 // point is hit. Each evaluated expression is compared with an expected value.
 // For this debug event handler to work the following two global varaibles
 // must be initialized.
 //   checks: An array of expressions and expected results
 //   evaluate_check_function: A JavaScript function (see below)
 
@@ skipping 3235 matching lines @@
   CHECK_EQ(2, exception_hit_count);
   CHECK_EQ(1, uncaught_exception_hit_count);
   CHECK_EQ(1, message_callback_count);
 
   v8::Debug::SetDebugEventListener(NULL);
   CheckDebuggerUnloaded();
   v8::V8::RemoveMessageListeners(MessageCallbackCount);
 }
 
 
+TEST(EvalJSInDebugEventListenerOnNativeReThrownException) {
+  DebugLocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+  env.ExposeDebug();
+
+  // Create functions for testing break on exception.
+  v8::Local<v8::Function> reThrowJS = CompileFunction(
+      &env, "function reThrowJS(){try {throw 1;} finally {};}", "reThrowJS");
+  v8::Local<v8::Function> noThrowJS = CompileFunction(
+      &env, "function noThrowJS(){var a=[1]; a.push(2); return a.length;}",
+      "noThrowJS");
+
+  debug_event_listener_callback = noThrowJS;
+  debug_event_listener_callback_result = 2;
+
+  v8::V8::AddMessageListener(MessageCallbackCount);
+  v8::Debug::SetDebugEventListener(DebugEventCounter);
+  // Break on uncaught exception
+  ChangeBreakOnException(false, true);
+  DebugEventCounterClear();
+  MessageCallbackCountClear();
+
+  // ReThrow error from JavaScript
+  reThrowJS->Call(env->Global(), 0, NULL);
+  CHECK_EQ(1, exception_hit_count);
+  CHECK_EQ(1, uncaught_exception_hit_count);
+  CHECK_EQ(1, message_callback_count);
+  CHECK(!debug_event_listener_callback.IsEmpty());
+
+  // ReThrow native error
+  {
+    v8::TryCatch tryCatch;
+    env->GetIsolate()->ThrowException(v8::Exception::TypeError(
+        v8::String::NewFromUtf8(env->GetIsolate(), "Type error")));
+    CHECK(tryCatch.HasCaught());
+    tryCatch.ReThrow();
+  }
+  CHECK_EQ(2, exception_hit_count);
+  CHECK_EQ(2, uncaught_exception_hit_count);
+  CHECK_EQ(1, message_callback_count);  // FIXME: Should it be 2 ?

[Yang, 2014/09/10 14:46:13]

I think 1 is correct.

[aandrey, 2014/09/10 14:51:19]

hm... why? the native version of JS's try-finally should report a message as
well, right?

[Yang, 2014/09/10 14:56:29]

If I understood correctly, the exception thrown in reThrowJS is caught by the
TryCatch scope, so it's a caught exception, for which we don't trigger an event.
Only on ReThrow, the destructor of the TryCatch triggers an uncaught exception
event.

[aandrey, 2014/09/10 15:01:53]

Right, but destructor should be already executed by this moment.

[aandrey, 2014/09/11 05:48:29]

Note, that if I remove the v8::TryCatch and call just ThrowException(), the
message_callback_count is still 1.

+  CHECK(!debug_event_listener_callback.IsEmpty());
+
+  debug_event_listener_callback.Clear();
+}
+
+
 // 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) {
   DebugLocalContext env;
   v8::HandleScope scope(env->GetIsolate());
 
   // For this test, we want to break on uncaught exceptions:
   ChangeBreakOnException(false, true);
 
@@ skipping 3420 matching lines @@
   v8::Isolate* isolate = env->GetIsolate();
   v8::HandleScope scope(isolate);
   v8::Debug::SetDebugEventListener(DebugBreakTriggerTerminate);
   TerminationThread terminator(isolate);
   terminator.Start();
   v8::TryCatch try_catch;
   v8::Debug::DebugBreak(isolate);
   CompileRun("while (true);");
   CHECK(try_catch.HasTerminated());
 }