Reporting uncaught errors at the boundary between C++ and JS instead of trying...

Reporting uncaught errors at the boundary between C++ and JS instead of trying to guess whether they get caught at the time of the throw.
Review URL: http://codereview.chromium.org/12901

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

src/execution.cc

@@ -95,15 +95,10 @@ static Handle<Object> Invoke(bool construct,
   *has_pending_exception = value->IsException();
   ASSERT(*has_pending_exception == Top::has_pending_exception());
   if (*has_pending_exception) {
-    Top::setup_external_caught();
-    // If the pending exception is OutOfMemoryException set out_of_memory in
-    // the global context.  Note: We have to mark the global context here
-    // since the GenerateThrowOutOfMemory stub cannot make a RuntimeCall to
-    // set it.
-    if (Top::pending_exception() == Failure::OutOfMemoryException()) {
-      Top::context()->mark_out_of_memory();
-    }
+    Top::ReportPendingMessages();
     return Handle<Object>();
+  } else {
+    Top::clear_pending_message();
   }
 
   return Handle<Object>(value);

src/top.cc

@@ -66,6 +66,9 @@ char* Top::Iterate(ObjectVisitor* v, char* thread_storage) {
 
 void Top::Iterate(ObjectVisitor* v, ThreadLocalTop* thread) {
   v->VisitPointer(&(thread->pending_exception_));
+  v->VisitPointer(&(thread->pending_message_obj_));
+  v->VisitPointer(
+      bit_cast<Object**, Script**>(&(thread->pending_message_script_)));
   v->VisitPointer(bit_cast<Object**, Context**>(&(thread->context_)));
   v->VisitPointer(&(thread->scheduled_exception_));
 
@@ -99,6 +102,7 @@ void Top::InitializeThreadLocal() {
   thread_local_.external_caught_exception_ = false;
   thread_local_.failed_access_check_callback_ = NULL;
   clear_pending_exception();
+  clear_pending_message();
   clear_scheduled_exception();
   thread_local_.save_context_ = NULL;
   thread_local_.catcher_ = NULL;
@@ -791,8 +795,13 @@ void Top::DoThrow(Object* exception,
 
   // Determine reporting and whether the exception is caught externally.
   bool is_caught_externally = false;
-  bool report_exception = (exception != Failure::OutOfMemoryException()) &&
-      ShouldReportException(&is_caught_externally);
+  bool is_out_of_memory = exception == Failure::OutOfMemoryException();
+  bool should_return_exception =  ShouldReportException(&is_caught_externally);
+  bool report_exception = !is_out_of_memory && should_return_exception;
+
+
+  // Notify debugger of exception.
+  Debugger::OnException(exception_handle, report_exception);
 
   // Generate the message.
   Handle<Object> message_obj;
@@ -812,38 +821,66 @@ void Top::DoThrow(Object* exception,
         location, HandleVector<Object>(&exception_handle, 1), stack_trace);
   }
 
-  // If the exception is caught externally, we store it in the
-  // try/catch handler. The C code can find it later and process it if
-  // necessary.
-  thread_local_.catcher_ = NULL;
-  if (is_caught_externally) {
-    thread_local_.catcher_ = thread_local_.try_catch_handler_;
-    thread_local_.try_catch_handler_->exception_ =
-      reinterpret_cast<void*>(*exception_handle);
+  // Save the message for reporting if the the exception remains uncaught.
+  thread_local_.pending_message_ = message;
   if (!message_obj.is_null()) {
-      thread_local_.try_catch_handler_->message_ =
-        reinterpret_cast<void*>(*message_obj);
+    thread_local_.pending_message_obj_ = *message_obj;
+    if (location != NULL) {
+      thread_local_.pending_message_script_ = *location->script();
+      thread_local_.pending_message_start_pos_ = location->start_pos();
+      thread_local_.pending_message_end_pos_ = location->end_pos();
     }
   }
 
-  // Notify debugger of exception.
-  Debugger::OnException(exception_handle, report_exception);
-
-  if (report_exception) {
-    if (message != NULL) {
-      MessageHandler::ReportMessage(message);
-    } else if (!message_obj.is_null()) {
-      MessageHandler::ReportMessage(location, message_obj);
-    }
+  if (is_caught_externally) {
+    thread_local_.catcher_ = thread_local_.try_catch_handler_;
   }
 
-  // NOTE: Notifying the debugger or reporting the exception may have caused
-  // new exceptions. For now, we just ignore that and set the pending exception
-  // to the original one.
+  // NOTE: Notifying the debugger or generating the message
+  // may have caused new exceptions. For now, we just ignore
+  // that and set the pending exception to the original one.
   set_pending_exception(*exception_handle);
 }
 
 
+void Top::ReportPendingMessages() {
+  ASSERT(has_pending_exception());
+  setup_external_caught();
+  // If the pending exception is OutOfMemoryException set out_of_memory in
+  // the global context.  Note: We have to mark the global context here
+  // since the GenerateThrowOutOfMemory stub cannot make a RuntimeCall to
+  // set it.
+  HandleScope scope;
+  if (thread_local_.pending_exception_ == Failure::OutOfMemoryException()) {
+    context()->mark_out_of_memory();
+  } else {
+    Handle<Object> exception(pending_exception());
+    if (thread_local_.external_caught_exception_) {
+      thread_local_.try_catch_handler_->exception_ =
+        thread_local_.pending_exception_;
+      if (!thread_local_.pending_message_obj_->IsTheHole()) {
+        try_catch_handler()->message_ = thread_local_.pending_message_obj_;
+      }
+    } else if (thread_local_.pending_message_ != NULL) {
+      MessageHandler::ReportMessage(thread_local_.pending_message_);
+    } else if (!thread_local_.pending_message_obj_->IsTheHole()) {
+      Handle<Object> message_obj(thread_local_.pending_message_obj_);
+      if (thread_local_.pending_message_script_ != NULL) {
+        Handle<Script> script(thread_local_.pending_message_script_);
+        int start_pos = thread_local_.pending_message_start_pos_;
+        int end_pos = thread_local_.pending_message_end_pos_;
+        MessageLocation location(script, start_pos, end_pos);
+        MessageHandler::ReportMessage(&location, message_obj);
+      } else {
+        MessageHandler::ReportMessage(NULL, message_obj);
+      }
+    }
+    set_pending_exception(*exception);
+  }
+  clear_pending_message();
+}
+
+
 void Top::TraceException(bool flag) {
   FLAG_trace_exception = flag;
 }

src/top.h

@@ -46,6 +46,11 @@ class ThreadLocalTop BASE_EMBEDDED {
   // lookups.
   Context* context_;
   Object* pending_exception_;
+  const char* pending_message_;
+  Object* pending_message_obj_;
+  Script* pending_message_script_;
+  int pending_message_start_pos_;
+  int pending_message_end_pos_;
   // Use a separate value for scheduled exceptions to preserve the
   // invariants that hold about pending_exception.  We may want to
   // unify them later.
@@ -120,6 +125,12 @@ class Top {
   static bool has_pending_exception() {
     return !thread_local_.pending_exception_->IsTheHole();
   }
+  static void clear_pending_message() {
+    thread_local_.catcher_ = NULL;
+    thread_local_.pending_message_ = NULL;
+    thread_local_.pending_message_obj_ = Heap::the_hole_value();
+    thread_local_.pending_message_script_ = NULL;
+  }
   static v8::TryCatch* try_catch_handler() {
     return thread_local_.try_catch_handler_;
   }
@@ -146,6 +157,7 @@ class Top {
 
   static void setup_external_caught() {
     thread_local_.external_caught_exception_ =
+        (!thread_local_.pending_exception_->IsTheHole()) &&
         (thread_local_.catcher_ != NULL) &&
         (Top::thread_local_.try_catch_handler_ == Top::thread_local_.catcher_);
   }
@@ -207,6 +219,7 @@ class Top {
   // originally.
   static Failure* ReThrow(Object* exception, MessageLocation* location = NULL);
   static void ScheduleThrow(Object* exception);
+  static void ReportPendingMessages();
 
   // Promote a scheduled exception to pending. Asserts has_scheduled_exception.
   static Object* PromoteScheduledException();

test/cctest/test-api.cc

@@ -2960,6 +2960,7 @@ static void ApiUncaughtExceptionTestListener(v8::Handle<v8::Message>,
 // Counts uncaught exceptions, but other tests running in parallel
 // also have uncaught exceptions.
 TEST(ApiUncaughtException) {
+  report_count = 0;
   v8::HandleScope scope;
   LocalContext env;
   v8::V8::AddMessageListener(ApiUncaughtExceptionTestListener);
@@ -2983,6 +2984,27 @@ TEST(ApiUncaughtException) {
   CHECK(trouble_caller->IsFunction());
   Function::Cast(*trouble_caller)->Call(global, 0, NULL);
   CHECK_EQ(1, report_count);
+  v8::V8::RemoveMessageListeners(ApiUncaughtExceptionTestListener);
+}
+
+
+TEST(TryCatchFinallyUsingTryCatchHandler) {
+  v8::HandleScope scope;
+  LocalContext env;
+  v8::TryCatch try_catch;
+  Script::Compile(v8_str("try { throw ''; } catch (e) {}"))->Run();
+  CHECK(!try_catch.HasCaught());
+  Script::Compile(v8_str("try { throw ''; } finally {}"))->Run();
+  CHECK(try_catch.HasCaught());
+  try_catch.Reset();
+  Script::Compile(v8_str("(function() {"
+                         "try { throw ''; } finally { return; }"
+                         "})()"))->Run();
+  CHECK(!try_catch.HasCaught());
+  Script::Compile(v8_str("(function()"
+                         "  { try { throw ''; } finally { throw 0; }"
+                         "})()"))->Run();
+  CHECK(try_catch.HasCaught());
 }
 
 

test/message/try-catch-finally-return-in-finally.js

+// Copyright 2008 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+function f() {
+  try {
+    throw "foo";
+    return 7;
+  } catch (e) {
+    "bar"
+  } finally {
+    return 42;
+  }
+}
+
+print(f());

test/message/try-catch-finally-return-in-finally.out

+# Copyright 2008 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
+#       with the distribution.
+#     * Neither the name of Google Inc. nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+42