Reland exception propagation fix.

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.

A couple of tests will fail on the ARM simulator because the simulator
has separate stacks for C++ and JavaScript.  I have marked the tests
as failing only on the simulator.
Review URL: http://codereview.chromium.org/56105

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

src/top.cc

@@ -285,6 +285,7 @@ void Top::RegisterTryCatchHandler(v8::TryCatch* that) {
 void Top::UnregisterTryCatchHandler(v8::TryCatch* that) {
   ASSERT(thread_local_.try_catch_handler_ == that);
   thread_local_.try_catch_handler_ = that->next_;
+  thread_local_.catcher_ = NULL;
 }
 
 
@@ -724,10 +725,9 @@ void Top::DoThrow(Object* exception,
   // Determine reporting and whether the exception is caught externally.
   bool is_caught_externally = false;
   bool is_out_of_memory = exception == Failure::OutOfMemoryException();
-  bool should_return_exception =  ShouldReportException(&is_caught_externally);
+  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);
 
@@ -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)) {
-    thread_local_.external_caught_exception_ = false;
-    clear_pending_exception();
-    return false;
-  } else {
-    thread_local_.scheduled_exception_ = pending_exception();
-    clear_pending_exception();
-    return true;
+  // 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;
+    }
   }
+
+  // Reschedule the exception.
+  thread_local_.scheduled_exception_ = pending_exception();
+  clear_pending_exception();
+  return true;
 }
 
 

src/top.h

@@ -127,7 +127,6 @@ class Top {
     return !thread_local_.pending_exception_->IsTheHole();
   }
   static void clear_pending_message() {
-    thread_local_.catcher_ = NULL;
     thread_local_.has_pending_message_ = false;
     thread_local_.pending_message_ = NULL;
     thread_local_.pending_message_obj_ = Heap::the_hole_value();
@@ -159,9 +158,9 @@ class Top {
 
   static void setup_external_caught() {
     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/cctest.status

@@ -42,3 +42,11 @@ test-spaces/LargeObjectSpace: PASS || FAIL
 
 # BUG(240): Test seems flaky on ARM.
 test-api/RegExpInterruption: SKIP
+
+[ $simulator == arm ]
+
+# BUG(271): During exception propagation, we compare pointers into the
+# stack.  These tests fail on the ARM simulator because the C++ and
+# the JavaScript stacks are separate.
+test-api/ExceptionOrder: FAIL
+test-api/TryCatchInTryFinally: FAIL

test/cctest/test-api.cc

@@ -1812,7 +1812,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());
 }
 
@@ -1849,7 +1850,12 @@ THREADED_TEST(APIThrowTryCatch) {
 
 // Test that a try-finally block doesn't shadow a try-catch block
 // when setting up an external handler.
-THREADED_TEST(TryCatchInTryFinally) {
+//
+// BUG(271): Some of the exception propagation does not work on the
+// ARM simulator because the simulator separates the C++ stack and the
+// JS stack.  This test therefore fails on the simulator.  The test is
+// not threaded to allow the threading tests to run on the simulator.
+TEST(TryCatchInTryFinally) {
   v8::HandleScope scope;
   Local<ObjectTemplate> templ = ObjectTemplate::New();
   templ->Set(v8_str("CCatcher"),
@@ -1868,6 +1874,7 @@ THREADED_TEST(TryCatchInTryFinally) {
 
 static void receive_message(v8::Handle<v8::Message> message,
                             v8::Handle<v8::Value> data) {
+  message->Get();
   message_received = true;
 }
 
@@ -1896,8 +1903,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);
 }
@@ -1943,6 +1951,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);
@@ -2000,7 +2009,12 @@ THREADED_TEST(EvalInTryFinally) {
 // Each entry is an activation, either JS or C.  The index is the count at that
 // level.  Stars identify activations with exception handlers, the @ identifies
 // the exception handler that should catch the exception.
-THREADED_TEST(ExceptionOrder) {
+//
+// BUG(271): Some of the exception propagation does not work on the
+// ARM simulator because the simulator separates the C++ stack and the
+// JS stack.  This test therefore fails on the simulator.  The test is
+// not threaded to allow the threading tests to run on the simulator.
+TEST(ExceptionOrder) {
   v8::HandleScope scope;
   Local<ObjectTemplate> templ = ObjectTemplate::New();
   templ->Set(v8_str("check"), v8::FunctionTemplate::New(JSCheck));

tools/test.py

@@ -1261,7 +1261,8 @@ def Main():
       env = {
         'mode': mode,
         'system': utils.GuessOS(),
-        'arch': options.arch
+        'arch': options.arch,
+        'simulator': options.simulator
       }
       test_list = root.ListTests([], path, context, mode)
       unclassified_tests += test_list