From: yurys@chromium.org Date: Wed, 11 Jun 2014 05:48:33 +0000 (+0000) Subject: V8 can clear exception pending message, when should not do this. X-Git-Tag: upstream/4.7.83~8748 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6cb9002057f4e92b638ae2b939051a942dc36324;p=platform%2Fupstream%2Fv8.git V8 can clear exception pending message, when should not do this. The case: v8::TryCatch try_catch; CompileRun(try { CEvaluate('throw 1;'); } finally {}); CHECK(try_catch.HasCaught()); CHECK(!try_catch.Message().IsEmpty()); CEvaluate is native call. Last check is not passed without patch. Patch contains test TryCatchFinallyStoresMessageUsingTryCatchHandler with more details. R=yangguo@chromium.org, mstarzinger@chromium.org, vsevik@chromium.org Review URL: https://codereview.chromium.org/321763002 Patch from Alexey Kozyatinskiy . git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21755 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/isolate.cc b/src/isolate.cc index 1e0b0947a..0d6872a5f 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -1166,20 +1166,15 @@ void Isolate::DoThrow(Object* exception, MessageLocation* location) { } -bool Isolate::IsExternallyCaught() { +bool Isolate::HasExternalTryCatch() { ASSERT(has_pending_exception()); - if ((thread_local_top()->catcher_ == NULL) || - (try_catch_handler() != thread_local_top()->catcher_)) { - // When throwing the exception, we found no v8::TryCatch - // which should care about this exception. - return false; - } + return (thread_local_top()->catcher_ != NULL) && + (try_catch_handler() == thread_local_top()->catcher_); +} - if (!is_catchable_by_javascript(pending_exception())) { - return true; - } +bool Isolate::IsFinallyOnTop() { // Get the address of the external handler so we can compare the address to // determine which one is closer to the top of the stack. Address external_handler_address = @@ -1199,18 +1194,18 @@ bool Isolate::IsExternallyCaught() { StackHandler::FromAddress(Isolate::handler(thread_local_top())); while (handler != NULL && handler->address() < external_handler_address) { ASSERT(!handler->is_catch()); - if (handler->is_finally()) return false; + if (handler->is_finally()) return true; handler = handler->next(); } - return true; + return false; } void Isolate::ReportPendingMessages() { ASSERT(has_pending_exception()); - PropagatePendingExceptionToExternalTryCatch(); + bool can_clear_message = PropagatePendingExceptionToExternalTryCatch(); HandleScope scope(this); if (thread_local_top_.pending_exception_ == @@ -1237,7 +1232,7 @@ void Isolate::ReportPendingMessages() { } } } - clear_pending_message(); + if (can_clear_message) clear_pending_message(); } @@ -1742,14 +1737,22 @@ void Isolate::InitializeThreadLocal() { } -void Isolate::PropagatePendingExceptionToExternalTryCatch() { +bool Isolate::PropagatePendingExceptionToExternalTryCatch() { ASSERT(has_pending_exception()); - bool external_caught = IsExternallyCaught(); - thread_local_top_.external_caught_exception_ = external_caught; + bool has_external_try_catch = HasExternalTryCatch(); + if (!has_external_try_catch) { + thread_local_top_.external_caught_exception_ = false; + return true; + } - if (!external_caught) return; + bool catchable_by_js = is_catchable_by_javascript(pending_exception()); + if (catchable_by_js && IsFinallyOnTop()) { + thread_local_top_.external_caught_exception_ = false; + return false; + } + thread_local_top_.external_caught_exception_ = true; if (thread_local_top_.pending_exception_ == heap()->termination_exception()) { try_catch_handler()->can_continue_ = false; @@ -1765,13 +1768,14 @@ void Isolate::PropagatePendingExceptionToExternalTryCatch() { handler->has_terminated_ = false; handler->exception_ = pending_exception(); // Propagate to the external try-catch only if we got an actual message. - if (thread_local_top_.pending_message_obj_->IsTheHole()) return; + if (thread_local_top_.pending_message_obj_->IsTheHole()) return true; handler->message_obj_ = thread_local_top_.pending_message_obj_; handler->message_script_ = thread_local_top_.pending_message_script_; handler->message_start_pos_ = thread_local_top_.pending_message_start_pos_; handler->message_end_pos_ = thread_local_top_.pending_message_end_pos_; } + return true; } diff --git a/src/isolate.h b/src/isolate.h index a52cfb5fd..426a73892 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -604,7 +604,8 @@ class Isolate { thread_local_top_.scheduled_exception_ = heap_.the_hole_value(); } - bool IsExternallyCaught(); + bool HasExternalTryCatch(); + bool IsFinallyOnTop(); bool is_catchable_by_javascript(Object* exception) { return exception != heap()->termination_exception(); @@ -1179,7 +1180,10 @@ class Isolate { void FillCache(); - void PropagatePendingExceptionToExternalTryCatch(); + // Propagate pending exception message to the v8::TryCatch. + // If there is no external try-catch or message was successfully propagated, + // then return true. + bool PropagatePendingExceptionToExternalTryCatch(); // Traverse prototype chain to find out whether the object is derived from // the Error object. diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index f4c618e73..89255f239 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8362,6 +8362,41 @@ TEST(TryCatchFinallyUsingTryCatchHandler) { } +void CEvaluate(const v8::FunctionCallbackInfo& args) { + v8::HandleScope scope(args.GetIsolate()); + CompileRun(args[0]->ToString()); +} + + +TEST(TryCatchFinallyStoresMessageUsingTryCatchHandler) { + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope scope(isolate); + Local templ = ObjectTemplate::New(isolate); + templ->Set(v8_str("CEvaluate"), + v8::FunctionTemplate::New(isolate, CEvaluate)); + LocalContext context(0, templ); + v8::TryCatch try_catch; + CompileRun("try {" + " CEvaluate('throw 1;');" + "} finally {" + "}"); + CHECK(try_catch.HasCaught()); + CHECK(!try_catch.Message().IsEmpty()); + String::Utf8Value exception_value(try_catch.Exception()); + CHECK_EQ(*exception_value, "1"); + try_catch.Reset(); + CompileRun("try {" + " CEvaluate('throw 1;');" + "} finally {" + " throw 2;" + "}"); + CHECK(try_catch.HasCaught()); + CHECK(!try_catch.Message().IsEmpty()); + String::Utf8Value finally_exception_value(try_catch.Exception()); + CHECK_EQ(*finally_exception_value, "2"); +} + + // For use within the TestSecurityHandler() test. static bool g_security_callback_result = false; static bool NamedSecurityTestCallback(Local global,