Hello [EMAIL PROTECTED], [EMAIL PROTECTED], I'd like you to do a code review. To review this change, run
gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL PROTECTED] Alternatively, to review the latest snapshot of this change branch, run gvn --project https://v8.googlecode.com/svn review [EMAIL PROTECTED]/xxx to review the following change: [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-10-24 14:08:31 +-100 (Fri, 24 Oct 2008) Description: Revorked version of http://codereview.chromium.org/8101. The only change is that the exception stored in the TryCatch is not cleared as that was the cause of the regression. So the actual difference to http://codereview.chromium.org/8101 is: Index: src/top.h =================================================================== --- src/top.h (revision 576) +++ src/top.h (working copy) @@ -154,10 +154,6 @@ 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; } @@ -312,14 +308,14 @@ }; Affected Paths: M //branches/bleeding_edge/src/execution.cc M //branches/bleeding_edge/src/top.cc M //branches/bleeding_edge/src/top.h This is a semiautomated message from "gvn mail". See <http://code.google.com/p/gvn/> to learn more. Index: src/execution.cc =================================================================== --- src/execution.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/execution.cc (^/changes/[EMAIL PROTECTED]/xxx/bleeding_edge/src/[EMAIL PROTECTED]) @@ -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 Index: src/top.cc =================================================================== --- src/top.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/top.cc (^/changes/[EMAIL PROTECTED]/xxx/bleeding_edge/src/[EMAIL PROTECTED]) @@ -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, MessageLoca 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()) { Index: src/top.h =================================================================== --- src/top.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/top.h (^/changes/[EMAIL PROTECTED]/xxx/bleeding_edge/src/[EMAIL PROTECTED]) @@ -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,16 @@ 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_; + } + thread_local_.pending_external_caught_exception_ = false; } // Tells whether the current context has experienced an out of memory --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
