Fix exception propagation problem where undefined was returned instead

of an empty handle in case of an exception.  This problem can break
C++ programs that are not interested in catching exceptions and just
want to propagate them out by testing for empty handles.

The issue is that exceptions are not rescheduled if they are
externally caught.  Externally caught exceptions have to be
rescheduled if there is a JavaScript frame on the way to the C++ frame
that holds the external handler.
Review URL: http://codereview.chromium.org/43070

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

src/top.cc

@@ -823,16 +823,36 @@ void Top::TraceException(bool flag) {
 
 
 bool Top::optional_reschedule_exception(bool is_bottom_call) {
-  if (!is_out_of_memory() &&
-      (thread_local_.external_caught_exception_ || is_bottom_call)) {
+  // Allways reschedule out of memory exceptions.
+  if (!is_out_of_memory()) {
+    // Never reschedule the exception if this is the bottom call.
+    bool clear_exception = is_bottom_call;
+
+    // If the exception is externally caught, clear it if there are no
+    // JavaScript frames on the way to the C++ frame that has the
+    // external handler.
+    if (thread_local_.external_caught_exception_) {
+      ASSERT(thread_local_.try_catch_handler_ != NULL);
+      Address external_handler_address =
+          reinterpret_cast<Address>(thread_local_.try_catch_handler_);
+      JavaScriptFrameIterator it;
+      if (it.done() || (it.frame()->sp() > external_handler_address)) {
+        clear_exception = true;
+      }
+    }
+
+    // Clear the exception if needed.
+    if (clear_exception) {
       thread_local_.external_caught_exception_ = false;
       clear_pending_exception();
       return false;
-  } else {
+    }
+  }
+
+  // Reschedule the exception.
   thread_local_.scheduled_exception_ = pending_exception();
   clear_pending_exception();
   return true;
-  }
 }
 
 

src/top.h

@@ -158,10 +158,12 @@ class Top {
   }
 
   static void setup_external_caught() {
+    if (!thread_local_.external_caught_exception_) {
       thread_local_.external_caught_exception_ =
-        (!thread_local_.pending_exception_->IsTheHole()) &&
+          has_pending_exception() &&
           (thread_local_.catcher_ != NULL) &&
-        (Top::thread_local_.try_catch_handler_ == Top::thread_local_.catcher_);
+          (thread_local_.try_catch_handler_ == thread_local_.catcher_);
+    }
   }
 
   // Tells whether the current context has experienced an out of memory

test/cctest/test-api.cc

@@ -1722,7 +1722,8 @@ v8::Handle<Value> CCatcher(const v8::Arguments& args) {
   if (args.Length() < 1) return v8::Boolean::New(false);
   v8::HandleScope scope;
   v8::TryCatch try_catch;
-  v8::Script::Compile(args[0]->ToString())->Run();
+  Local<Value> result = v8::Script::Compile(args[0]->ToString())->Run();
+  CHECK(!try_catch.HasCaught() || result.IsEmpty());
   return v8::Boolean::New(try_catch.HasCaught());
 }
 
@@ -1806,8 +1807,9 @@ TEST(APIThrowMessageAndVerboseTryCatch) {
   LocalContext context(0, templ);
   v8::TryCatch try_catch;
   try_catch.SetVerbose(true);
-  CompileRun("ThrowFromC();");
+  Local<Value> result = CompileRun("ThrowFromC();");
   CHECK(try_catch.HasCaught());
+  CHECK(result.IsEmpty());
   CHECK(message_received);
   v8::V8::RemoveMessageListeners(check_message);
 }
@@ -1853,6 +1855,7 @@ v8::Handle<Value> CThrowCountDown(const v8::Arguments& args) {
       int expected = args[3]->Int32Value();
       if (try_catch.HasCaught()) {
         CHECK_EQ(expected, count);
+        CHECK(result.IsEmpty());
         CHECK(!i::Top::has_scheduled_exception());
       } else {
         CHECK_NE(expected, count);