Properly process try/finally blocks.
In some circumstances, try/finally block can actually catch the exception: function f() { try { throw 42; } finally { return 0; } } Therefore when propagating exception to v8::TryCatch, we must be sure there is no try/finally blocks as well. When bulding the messages we should be more conservative and expect that any v8::TryCatch with no JS try/catch in between can potentionally be the right exception handler. Plus various minor refactorings. BUG=1147 TEST=cctest/test-api/TryCatchAndFinallyHidingException, cctest/test-api/TryCatchAndFinally Review URL: http://codereview.chromium.org/6526016 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6809 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
parent
fba910c313
commit
6b4ff18b5b
@ -27,6 +27,7 @@
|
||||
|
||||
#include <v8.h>
|
||||
#include <v8-testing.h>
|
||||
#include <assert.h>
|
||||
#include <fcntl.h>
|
||||
#include <string.h>
|
||||
#include <stdio.h>
|
||||
@ -290,11 +291,13 @@ bool ExecuteString(v8::Handle<v8::String> source,
|
||||
} else {
|
||||
v8::Handle<v8::Value> result = script->Run();
|
||||
if (result.IsEmpty()) {
|
||||
assert(try_catch.HasCaught());
|
||||
// Print errors that happened during execution.
|
||||
if (report_exceptions)
|
||||
ReportException(&try_catch);
|
||||
return false;
|
||||
} else {
|
||||
assert(!try_catch.HasCaught());
|
||||
if (print_result && !result->IsUndefined()) {
|
||||
// If all went well and the result wasn't undefined then print
|
||||
// the returned value.
|
||||
|
@ -127,11 +127,13 @@ bool Shell::ExecuteString(Handle<String> source,
|
||||
} else {
|
||||
Handle<Value> result = script->Run();
|
||||
if (result.IsEmpty()) {
|
||||
ASSERT(try_catch.HasCaught());
|
||||
// Print errors that happened during execution.
|
||||
if (report_exceptions && !i::FLAG_debugger)
|
||||
ReportException(&try_catch);
|
||||
return false;
|
||||
} else {
|
||||
ASSERT(!try_catch.HasCaught());
|
||||
if (print_result && !result->IsUndefined()) {
|
||||
// If all went well and the result wasn't undefined then print
|
||||
// the returned value.
|
||||
|
87
src/top.cc
87
src/top.cc
@ -333,7 +333,7 @@ void Top::RegisterTryCatchHandler(v8::TryCatch* that) {
|
||||
|
||||
|
||||
void Top::UnregisterTryCatchHandler(v8::TryCatch* that) {
|
||||
ASSERT(thread_local_.TryCatchHandler() == that);
|
||||
ASSERT(try_catch_handler() == that);
|
||||
thread_local_.set_try_catch_handler_address(
|
||||
reinterpret_cast<Address>(that->next_));
|
||||
thread_local_.catcher_ = NULL;
|
||||
@ -732,6 +732,13 @@ Failure* Top::Throw(Object* exception, MessageLocation* location) {
|
||||
|
||||
|
||||
Failure* Top::ReThrow(MaybeObject* exception, MessageLocation* location) {
|
||||
bool can_be_caught_externally = false;
|
||||
ShouldReportException(&can_be_caught_externally,
|
||||
is_catchable_by_javascript(exception));
|
||||
if (can_be_caught_externally) {
|
||||
thread_local_.catcher_ = try_catch_handler();
|
||||
}
|
||||
|
||||
// Set the exception being re-thrown.
|
||||
set_pending_exception(exception);
|
||||
return Failure::Exception();
|
||||
@ -807,7 +814,7 @@ void Top::ComputeLocation(MessageLocation* target) {
|
||||
}
|
||||
|
||||
|
||||
bool Top::ShouldReportException(bool* is_caught_externally,
|
||||
bool Top::ShouldReportException(bool* can_be_caught_externally,
|
||||
bool catchable_by_javascript) {
|
||||
// Find the top-most try-catch handler.
|
||||
StackHandler* handler =
|
||||
@ -823,13 +830,13 @@ bool Top::ShouldReportException(bool* is_caught_externally,
|
||||
// The exception has been externally caught if and only if there is
|
||||
// an external handler which is on top of the top-most try-catch
|
||||
// handler.
|
||||
*is_caught_externally = external_handler_address != NULL &&
|
||||
*can_be_caught_externally = external_handler_address != NULL &&
|
||||
(handler == NULL || handler->address() > external_handler_address ||
|
||||
!catchable_by_javascript);
|
||||
|
||||
if (*is_caught_externally) {
|
||||
if (*can_be_caught_externally) {
|
||||
// Only report the exception if the external handler is verbose.
|
||||
return thread_local_.TryCatchHandler()->is_verbose_;
|
||||
return try_catch_handler()->is_verbose_;
|
||||
} else {
|
||||
// Report the exception if it isn't caught by JavaScript code.
|
||||
return handler == NULL;
|
||||
@ -848,14 +855,12 @@ void Top::DoThrow(MaybeObject* exception,
|
||||
Handle<Object> exception_handle(exception_object);
|
||||
|
||||
// Determine reporting and whether the exception is caught externally.
|
||||
bool is_out_of_memory = exception == Failure::OutOfMemoryException();
|
||||
bool is_termination_exception = exception == Heap::termination_exception();
|
||||
bool catchable_by_javascript = !is_termination_exception && !is_out_of_memory;
|
||||
bool catchable_by_javascript = is_catchable_by_javascript(exception);
|
||||
// Only real objects can be caught by JS.
|
||||
ASSERT(!catchable_by_javascript || is_object);
|
||||
bool is_caught_externally = false;
|
||||
bool can_be_caught_externally = false;
|
||||
bool should_report_exception =
|
||||
ShouldReportException(&is_caught_externally, catchable_by_javascript);
|
||||
ShouldReportException(&can_be_caught_externally, catchable_by_javascript);
|
||||
bool report_exception = catchable_by_javascript && should_report_exception;
|
||||
|
||||
#ifdef ENABLE_DEBUGGER_SUPPORT
|
||||
@ -869,8 +874,8 @@ void Top::DoThrow(MaybeObject* exception,
|
||||
Handle<Object> message_obj;
|
||||
MessageLocation potential_computed_location;
|
||||
bool try_catch_needs_message =
|
||||
is_caught_externally &&
|
||||
thread_local_.TryCatchHandler()->capture_message_;
|
||||
can_be_caught_externally &&
|
||||
try_catch_handler()->capture_message_;
|
||||
if (report_exception || try_catch_needs_message) {
|
||||
if (location == NULL) {
|
||||
// If no location was specified we use a computed one instead
|
||||
@ -908,8 +913,8 @@ void Top::DoThrow(MaybeObject* exception,
|
||||
}
|
||||
}
|
||||
|
||||
if (is_caught_externally) {
|
||||
thread_local_.catcher_ = thread_local_.TryCatchHandler();
|
||||
if (can_be_caught_externally) {
|
||||
thread_local_.catcher_ = try_catch_handler();
|
||||
}
|
||||
|
||||
// NOTE: Notifying the debugger or generating the message
|
||||
@ -925,22 +930,63 @@ void Top::DoThrow(MaybeObject* exception,
|
||||
}
|
||||
|
||||
|
||||
bool Top::IsExternallyCaught() {
|
||||
ASSERT(has_pending_exception());
|
||||
|
||||
if ((thread_local_.catcher_ == NULL) ||
|
||||
(try_catch_handler() != thread_local_.catcher_)) {
|
||||
// When throwing the exception, we found no v8::TryCatch
|
||||
// which should care about this exception.
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!is_catchable_by_javascript(pending_exception())) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// 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 = thread_local_.try_catch_handler_address();
|
||||
ASSERT(external_handler_address != NULL);
|
||||
|
||||
// The exception has been externally caught if and only if there is
|
||||
// an external handler which is on top of the top-most try-finally
|
||||
// handler.
|
||||
// There should be no try-catch blocks as they would prohibit us from
|
||||
// finding external catcher in the first place (see catcher_ check above).
|
||||
//
|
||||
// Note, that finally clause would rethrow an exception unless it's
|
||||
// aborted by jumps in control flow like return, break, etc. and we'll
|
||||
// have another chances to set proper v8::TryCatch.
|
||||
StackHandler* handler =
|
||||
StackHandler::FromAddress(Top::handler(Top::GetCurrentThread()));
|
||||
while (handler != NULL && handler->address() < external_handler_address) {
|
||||
ASSERT(!handler->is_try_catch());
|
||||
if (handler->is_try_finally()) return false;
|
||||
|
||||
handler = handler->next();
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
void Top::ReportPendingMessages() {
|
||||
ASSERT(has_pending_exception());
|
||||
setup_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
|
||||
// since the GenerateThrowOutOfMemory stub cannot make a RuntimeCall to
|
||||
// set it.
|
||||
bool external_caught = thread_local_.external_caught_exception_;
|
||||
bool external_caught = IsExternallyCaught();
|
||||
thread_local_.external_caught_exception_ = external_caught;
|
||||
HandleScope scope;
|
||||
if (thread_local_.pending_exception_ == Failure::OutOfMemoryException()) {
|
||||
context()->mark_out_of_memory();
|
||||
} else if (thread_local_.pending_exception_ ==
|
||||
Heap::termination_exception()) {
|
||||
if (external_caught) {
|
||||
thread_local_.TryCatchHandler()->can_continue_ = false;
|
||||
thread_local_.TryCatchHandler()->exception_ = Heap::null_value();
|
||||
try_catch_handler()->can_continue_ = false;
|
||||
try_catch_handler()->exception_ = Heap::null_value();
|
||||
}
|
||||
} else {
|
||||
// At this point all non-object (failure) exceptions have
|
||||
@ -949,9 +995,8 @@ void Top::ReportPendingMessages() {
|
||||
Handle<Object> exception(pending_exception_object);
|
||||
thread_local_.external_caught_exception_ = false;
|
||||
if (external_caught) {
|
||||
thread_local_.TryCatchHandler()->can_continue_ = true;
|
||||
thread_local_.TryCatchHandler()->exception_ =
|
||||
thread_local_.pending_exception_;
|
||||
try_catch_handler()->can_continue_ = true;
|
||||
try_catch_handler()->exception_ = thread_local_.pending_exception_;
|
||||
if (!thread_local_.pending_message_obj_->IsTheHole()) {
|
||||
try_catch_handler()->message_ = thread_local_.pending_message_obj_;
|
||||
}
|
||||
|
14
src/top.h
14
src/top.h
@ -249,12 +249,7 @@ class Top {
|
||||
thread_local_.scheduled_exception_ = Heap::the_hole_value();
|
||||
}
|
||||
|
||||
static void setup_external_caught() {
|
||||
thread_local_.external_caught_exception_ =
|
||||
has_pending_exception() &&
|
||||
(thread_local_.catcher_ != NULL) &&
|
||||
(try_catch_handler() == thread_local_.catcher_);
|
||||
}
|
||||
static bool IsExternallyCaught();
|
||||
|
||||
static void SetCaptureStackTraceForUncaughtExceptions(
|
||||
bool capture,
|
||||
@ -265,6 +260,11 @@ class Top {
|
||||
// exception.
|
||||
static bool is_out_of_memory();
|
||||
|
||||
static bool is_catchable_by_javascript(MaybeObject* exception) {
|
||||
return (exception != Failure::OutOfMemoryException()) &&
|
||||
(exception != Heap::termination_exception());
|
||||
}
|
||||
|
||||
// JS execution stack (see frames.h).
|
||||
static Address c_entry_fp(ThreadLocalTop* thread) {
|
||||
return thread->c_entry_fp_;
|
||||
@ -397,7 +397,7 @@ class Top {
|
||||
const char* message);
|
||||
// Checks if exception should be reported and finds out if it's
|
||||
// caught externally.
|
||||
static bool ShouldReportException(bool* is_caught_externally,
|
||||
static bool ShouldReportException(bool* can_be_caught_externally,
|
||||
bool catchable_by_javascript);
|
||||
|
||||
// Attempts to compute the current source location, storing the
|
||||
|
@ -2691,6 +2691,41 @@ THREADED_TEST(CatchExceptionFromWith) {
|
||||
}
|
||||
|
||||
|
||||
THREADED_TEST(TryCatchAndFinallyHidingException) {
|
||||
v8::HandleScope scope;
|
||||
LocalContext context;
|
||||
v8::TryCatch try_catch;
|
||||
CHECK(!try_catch.HasCaught());
|
||||
CompileRun("function f(k) { try { this[k]; } finally { return 0; } };");
|
||||
CompileRun("f({toString: function() { throw 42; }});");
|
||||
CHECK(!try_catch.HasCaught());
|
||||
}
|
||||
|
||||
|
||||
v8::Handle<v8::Value> WithTryCatch(const v8::Arguments& args) {
|
||||
v8::TryCatch try_catch;
|
||||
return v8::Undefined();
|
||||
}
|
||||
|
||||
|
||||
THREADED_TEST(TryCatchAndFinally) {
|
||||
v8::HandleScope scope;
|
||||
LocalContext context;
|
||||
context->Global()->Set(
|
||||
v8_str("native_with_try_catch"),
|
||||
v8::FunctionTemplate::New(WithTryCatch)->GetFunction());
|
||||
v8::TryCatch try_catch;
|
||||
CHECK(!try_catch.HasCaught());
|
||||
CompileRun(
|
||||
"try {\n"
|
||||
" throw new Error('a');\n"
|
||||
"} finally {\n"
|
||||
" native_with_try_catch();\n"
|
||||
"}\n");
|
||||
CHECK(try_catch.HasCaught());
|
||||
}
|
||||
|
||||
|
||||
THREADED_TEST(Equality) {
|
||||
v8::HandleScope scope;
|
||||
LocalContext context;
|
||||
|
Loading…
Reference in New Issue
Block a user