Cleanup of http://codereview.chromium.org/8101.

Changed the catcher_ field to a boolean value and renamed it. Modified the
propagation of the external caught exception to also clear the current
TryCatch if there is no exception as it might hold an exception which has
been bypassed by code in a finally block.

Minor formatting changes to a test.
Review URL: http://codereview.chromium.org/8102

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

diff --git a/src/execution.cc b/src/execution.cc
index 0598ad718fa6300b9710320f3ca1ba10ce30ac4b..03541a8f3a82fa466f097a5d980c0c367bf370f0 100644 (file)
--- a/src/execution.cc
+++ b/src/execution.cc
@@ -91,12 +91,10 @@ static Handle<Object> Invoke(bool construct,
   value->Verify();
 #endif
 
-  // Update the pending exception flag and return the value.
+  // Update the pending exception and external caught flag and return the value.
   *has_pending_exception = value->IsException();
   ASSERT(*has_pending_exception == Top::has_pending_exception());
-  if (*has_pending_exception) {
-    Top::setup_external_caught();
-  }
+  Top::propagate_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

diff --git a/src/top.cc b/src/top.cc
index a78ba32b57fe71e60802085c7ed73087c294d70f..8df808e561cdebfb2251f6355cf4b287c14115ea 100644 (file)
--- a/src/top.cc
+++ b/src/top.cc
@@ -101,7 +101,7 @@ void Top::InitializeThreadLocal() {
   clear_pending_exception();
   clear_scheduled_exception();
   thread_local_.save_context_ = NULL;
-  thread_local_.catcher_ = NULL;
+  thread_local_.pending_external_caught_exception_ = false;
 }
 
 
@@ -607,7 +607,7 @@ Failure* Top::Throw(Object* exception, MessageLocation* location) {
 
 
 Failure* Top::ReThrow(Object* exception, MessageLocation* location) {
-  // Set the exception beeing re-thrown.
+  // Set the exception being re-thrown.
   set_pending_exception(exception);
   return Failure::Exception();
 }
@@ -789,9 +789,8 @@ void Top::DoThrow(Object* exception,
   // 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;
+  thread_local_.pending_external_caught_exception_ = is_caught_externally;
   if (is_caught_externally) {
-    thread_local_.catcher_ = thread_local_.try_catch_handler_;
     thread_local_.try_catch_handler_->exception_ =
       reinterpret_cast<void*>(*exception_handle);
     if (!message_obj.is_null()) {

diff --git a/src/top.h b/src/top.h
index f7595b14a987ee6bff7b83dd48edc8a3f672f3a5..ac953d87846381fdfde1efaae146e5ec6aa9d598 100644 (file)
--- a/src/top.h
+++ b/src/top.h
@@ -53,7 +53,9 @@ class ThreadLocalTop BASE_EMBEDDED {
   bool external_caught_exception_;
   v8::TryCatch* try_catch_handler_;
   SaveContext* save_context_;
-  v8::TryCatch* catcher_;
+  // Flag indicating that the try_catch_handler_ contains an exception to be
+  // caught.
+  bool pending_external_caught_exception_;
 
   // Stack.
   Address c_entry_fp_;  // the frame pointer of the top c entry frame
@@ -144,10 +146,20 @@ class Top {
     thread_local_.scheduled_exception_ = Heap::the_hole_value();
   }
 
-  static void setup_external_caught() {
-    thread_local_.external_caught_exception_ =
-        (thread_local_.catcher_ != NULL) &&
-        (Top::thread_local_.try_catch_handler_ == Top::thread_local_.catcher_);
+  // Propagate the external caught exception flag. If there is no external
+  // caught exception always clear the TryCatch handler as it might contain
+  // an exception object from a throw which was bypassed by a finally block
+  // doing an explicit return/break/continue.
+  static void propagate_external_caught() {
+    if (has_pending_exception()) {
+      thread_local_.external_caught_exception_ =
+          thread_local_.pending_external_caught_exception_;
+    } else {
+      if (thread_local_.try_catch_handler_ != NULL) {
+        thread_local_.try_catch_handler_->Reset();
+      }
+    }
+    thread_local_.pending_external_caught_exception_ = false;
   }
 
   // Tells whether the current context has experienced an out of memory

diff --git a/test/mjsunit/api-call-after-bypassed-exception.js b/test/mjsunit/api-call-after-bypassed-exception.js
new file mode 100644 (file)
index 0000000..f77b514
--- /dev/null
+++ b/test/mjsunit/api-call-after-bypassed-exception.js
@@ -0,0 +1,39 @@
+// Copyright 2008 the V8 project authors. All rights reserved.\r
+// Redistribution and use in source and binary forms, with or without\r
+// modification, are permitted provided that the following conditions are\r
+// met:\r
+//\r
+//     * Redistributions of source code must retain the above copyright\r
+//       notice, this list of conditions and the following disclaimer.\r
+//     * Redistributions in binary form must reproduce the above\r
+//       copyright notice, this list of conditions and the following\r
+//       disclaimer in the documentation and/or other materials provided\r
+//       with the distribution.\r
+//     * Neither the name of Google Inc. nor the names of its\r
+//       contributors may be used to endorse or promote products derived\r
+//       from this software without specific prior written permission.\r
+//\r
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS\r
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT\r
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR\r
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT\r
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,\r
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT\r
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,\r
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY\r
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT\r
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE\r
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\r
+
+// This is a test of making an API call after an exception thrown in JavaScript
+// has been bypassed by a return in the finally block.
+function foo() {
+  try {
+    throw "bar";
+  } finally {
+    return "baz";
+  }
+}
+
+foo();
+print({});

diff --git a/test/mjsunit/api-call-after-overridden-exception.js b/test/mjsunit/api-call-after-overridden-exception.js
deleted file mode 100644 (file)
index 78a2aed..0000000
--- a/test/mjsunit/api-call-after-overridden-exception.js
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright 2008 the V8 project authors. All rights reserved.\r
-// Redistribution and use in source and binary forms, with or without\r
-// modification, are permitted provided that the following conditions are\r
-// met:\r
-//\r
-//     * Redistributions of source code must retain the above copyright\r
-//       notice, this list of conditions and the following disclaimer.\r
-//     * Redistributions in binary form must reproduce the above\r
-//       copyright notice, this list of conditions and the following\r
-//       disclaimer in the documentation and/or other materials provided\r
-//       with the distribution.\r
-//     * Neither the name of Google Inc. nor the names of its\r
-//       contributors may be used to endorse or promote products derived\r
-//       from this software without specific prior written permission.\r
-//\r
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS\r
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT\r
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR\r
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT\r
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,\r
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT\r
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,\r
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY\r
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT\r
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE\r
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\r
-
-// This is a test of making an API call after an exception thrown in JavaScript
-// has been swallowed by a return in the finally block.
-function foo()
-{
-  try {
-    throw "bar";
-  }
-  finally {
-    return "baz";
-  }
-}
-
-foo();
-print({});