Revision: 7548
Author: [email protected]
Date: Thu Apr 7 12:52:24 2011
Log: Make exception thrown via v8 public API propagate to v8::TryCatch
as JS thrown exceptions do.
Correctly process failures which can be returned by Object::GetProperty
when performing GetRealNamedProperty* queries.
Callback properties can produce exceptions so we need to wrap access to them
into exception checks. However, despite of many other methods with
exception
checks, property access doesn't mandatroy go via JavaScript and hence we
need to inject code to propagate exception to public API TryCatch handlers.
Review URL: http://codereview.chromium.org/6685087
http://code.google.com/p/v8/source/detail?r=7548
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/debug.cc
/branches/bleeding_edge/src/handles.cc
/branches/bleeding_edge/src/handles.h
/branches/bleeding_edge/src/isolate.cc
/branches/bleeding_edge/src/isolate.h
/branches/bleeding_edge/src/messages.cc
/branches/bleeding_edge/src/messages.h
/branches/bleeding_edge/src/top.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Wed Apr 6 12:17:54 2011
+++ /branches/bleeding_edge/src/api.cc Thu Apr 7 12:52:24 2011
@@ -2580,6 +2580,9 @@
ENTER_V8(isolate);
i::Handle<i::JSObject> self = Utils::OpenHandle(this);
i::Handle<i::Object> value_obj = Utils::OpenHandle(*value);
+ // We do not allow exceptions thrown while setting the prototype
+ // to propagate outside.
+ TryCatch try_catch;
EXCEPTION_PREAMBLE(isolate);
i::Handle<i::Object> result = i::SetPrototype(self, value_obj);
has_pending_exception = result.is_null();
@@ -2790,6 +2793,26 @@
return false);
return Utils::OpenHandle(this)->HasIndexedInterceptor();
}
+
+
+static Local<Value> GetPropertyByLookup(i::Isolate* isolate,
+ i::Handle<i::JSObject> receiver,
+ i::Handle<i::String> name,
+ i::LookupResult* lookup) {
+ if (!lookup->IsProperty()) {
+ // No real property was found.
+ return Local<Value>();
+ }
+
+ // If the property being looked up is a callback, it can throw
+ // an exception.
+ EXCEPTION_PREAMBLE(isolate);
+ i::Handle<i::Object> result = i::GetProperty(receiver, name, lookup);
+ has_pending_exception = result.is_null();
+ EXCEPTION_BAILOUT_CHECK(isolate, Local<Value>());
+
+ return Utils::ToLocal(result);
+}
Local<Value> v8::Object::GetRealNamedPropertyInPrototypeChain(
@@ -2803,17 +2826,7 @@
i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
i::LookupResult lookup;
self_obj->LookupRealNamedPropertyInPrototypes(*key_obj, &lookup);
- if (lookup.IsProperty()) {
- PropertyAttributes attributes;
- i::Object* property =
- self_obj->GetProperty(*self_obj,
- &lookup,
- *key_obj,
- &attributes)->ToObjectUnchecked();
- i::Handle<i::Object> result(property);
- return Utils::ToLocal(result);
- }
- return Local<Value>(); // No real property was found in prototype chain.
+ return GetPropertyByLookup(isolate, self_obj, key_obj, &lookup);
}
@@ -2826,17 +2839,7 @@
i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
i::LookupResult lookup;
self_obj->LookupRealNamedProperty(*key_obj, &lookup);
- if (lookup.IsProperty()) {
- PropertyAttributes attributes;
- i::Object* property =
- self_obj->GetProperty(*self_obj,
- &lookup,
- *key_obj,
- &attributes)->ToObjectUnchecked();
- i::Handle<i::Object> result(property);
- return Utils::ToLocal(result);
- }
- return Local<Value>(); // No real property was found in prototype chain.
+ return GetPropertyByLookup(isolate, self_obj, key_obj, &lookup);
}
=======================================
--- /branches/bleeding_edge/src/debug.cc Tue Apr 5 02:01:47 2011
+++ /branches/bleeding_edge/src/debug.cc Thu Apr 7 12:52:24 2011
@@ -810,7 +810,7 @@
Handle<Object> message = MessageHandler::MakeMessageObject(
"error_loading_debugger", NULL, Vector<Handle<Object> >::empty(),
Handle<String>(), Handle<JSArray>());
- MessageHandler::ReportMessage(NULL, message);
+ MessageHandler::ReportMessage(Isolate::Current(), NULL, message);
return false;
}
=======================================
--- /branches/bleeding_edge/src/handles.cc Mon Mar 21 09:00:52 2011
+++ /branches/bleeding_edge/src/handles.cc Thu Apr 7 12:52:24 2011
@@ -367,6 +367,17 @@
CALL_HEAP_FUNCTION(isolate,
Runtime::GetObjectProperty(isolate, obj, key),
Object);
}
+
+
+Handle<Object> GetProperty(Handle<JSObject> obj,
+ Handle<String> name,
+ LookupResult* result) {
+ PropertyAttributes attributes;
+ Isolate* isolate = Isolate::Current();
+ CALL_HEAP_FUNCTION(isolate,
+ obj->GetProperty(*obj, result, *name, &attributes),
+ Object);
+}
Handle<Object> GetElement(Handle<Object> obj,
=======================================
--- /branches/bleeding_edge/src/handles.h Mon Mar 21 05:25:31 2011
+++ /branches/bleeding_edge/src/handles.h Thu Apr 7 12:52:24 2011
@@ -244,6 +244,11 @@
Handle<Object> GetProperty(Handle<Object> obj,
Handle<Object> key);
+Handle<Object> GetProperty(Handle<JSObject> obj,
+ Handle<String> name,
+ LookupResult* result);
+
+
Handle<Object> GetElement(Handle<Object> obj,
uint32_t index);
=======================================
--- /branches/bleeding_edge/src/isolate.cc Thu Mar 31 09:17:37 2011
+++ /branches/bleeding_edge/src/isolate.cc Thu Apr 7 12:52:24 2011
@@ -699,6 +699,33 @@
clear_pending_message();
clear_scheduled_exception();
}
+
+
+void Isolate::PropagatePendingExceptionToExternalTryCatch() {
+ ASSERT(has_pending_exception());
+
+ bool external_caught = IsExternallyCaught();
+ thread_local_top_.external_caught_exception_ = external_caught;
+
+ if (!external_caught) return;
+
+ if (thread_local_top_.pending_exception_ ==
Failure::OutOfMemoryException()) {
+ // Do not propagate OOM exception: we should kill VM asap.
+ } else if (thread_local_top_.pending_exception_ ==
+ heap()->termination_exception()) {
+ try_catch_handler()->can_continue_ = false;
+ try_catch_handler()->exception_ = heap()->null_value();
+ } else {
+ // At this point all non-object (failure) exceptions have
+ // been dealt with so this shouldn't fail.
+ ASSERT(!pending_exception()->IsFailure());
+ try_catch_handler()->can_continue_ = true;
+ try_catch_handler()->exception_ = pending_exception();
+ if (!thread_local_top_.pending_message_obj_->IsTheHole()) {
+ try_catch_handler()->message_ =
thread_local_top_.pending_message_obj_;
+ }
+ }
+}
bool Isolate::Init(Deserializer* des) {
=======================================
--- /branches/bleeding_edge/src/isolate.h Tue Apr 5 02:01:47 2011
+++ /branches/bleeding_edge/src/isolate.h Thu Apr 7 12:52:24 2011
@@ -189,6 +189,9 @@
// unify them later.
MaybeObject* scheduled_exception_;
bool external_caught_exception_;
+ // True if unhandled message is being currently reported by
+ // MessageHandler::ReportMessage.
+ bool in_exception_reporting_;
SaveContext* save_context_;
v8::TryCatch* catcher_;
@@ -495,6 +498,9 @@
bool external_caught_exception() {
return thread_local_top_.external_caught_exception_;
}
+ void set_external_caught_exception(bool value) {
+ thread_local_top_.external_caught_exception_ = value;
+ }
void set_pending_exception(MaybeObject* exception) {
thread_local_top_.pending_exception_ = exception;
}
@@ -522,6 +528,18 @@
bool* external_caught_exception_address() {
return &thread_local_top_.external_caught_exception_;
}
+ bool in_exception_reporting() {
+ return thread_local_top_.in_exception_reporting_;
+ }
+ void set_in_exception_reporting(bool value) {
+ thread_local_top_.in_exception_reporting_ = value;
+ }
+ v8::TryCatch* catcher() {
+ return thread_local_top_.catcher_;
+ }
+ void set_catcher(v8::TryCatch* catcher) {
+ thread_local_top_.catcher_ = catcher;
+ }
MaybeObject** scheduled_exception_address() {
return &thread_local_top_.scheduled_exception_;
@@ -592,6 +610,27 @@
// JavaScript code. If an exception is scheduled true is returned.
bool OptionalRescheduleException(bool is_bottom_call);
+ class ExceptionScope {
+ public:
+ explicit ExceptionScope(Isolate* isolate) :
+ // Scope currently can only be used for regular exceptions, not
+ // failures like OOM or termination exception.
+ isolate_(isolate),
+
pending_exception_(isolate_->pending_exception()->ToObjectUnchecked()),
+ catcher_(isolate_->catcher())
+ { }
+
+ ~ExceptionScope() {
+ isolate_->set_catcher(catcher_);
+ isolate_->set_pending_exception(*pending_exception_);
+ }
+
+ private:
+ Isolate* isolate_;
+ Handle<Object> pending_exception_;
+ v8::TryCatch* catcher_;
+ };
+
void SetCaptureStackTraceForUncaughtExceptions(
bool capture,
int frame_limit,
@@ -1017,6 +1056,8 @@
void FillCache();
+ void PropagatePendingExceptionToExternalTryCatch();
+
int stack_trace_nesting_level_;
StringStream* incomplete_message_;
// The preallocated memory thread singleton.
=======================================
--- /branches/bleeding_edge/src/messages.cc Fri Mar 18 13:35:07 2011
+++ /branches/bleeding_edge/src/messages.cc Thu Apr 7 12:52:24 2011
@@ -106,14 +106,34 @@
}
-void MessageHandler::ReportMessage(MessageLocation* loc,
+void MessageHandler::ReportMessage(Isolate* isolate,
+ MessageLocation* loc,
Handle<Object> message) {
+ // If we are in process of message reporting, just ignore all other
requests
+ // to report a message as they are due to unhandled exceptions thrown in
+ // message callbacks.
+ if (isolate->in_exception_reporting()) {
+ ReportMessage("uncaught exception thrown while reporting");
+ return;
+ }
+ isolate->set_in_exception_reporting(true);
+
+ // We are calling into embedder's code which can throw exceptions.
+ // Thus we need to save current exception state, reset it to the clean
one
+ // and ignore scheduled exceptions callbacks can throw.
+ Isolate::ExceptionScope exception_scope(isolate);
+ isolate->clear_pending_exception();
+ isolate->set_external_caught_exception(false);
+
v8::Local<v8::Message> api_message_obj =
v8::Utils::MessageToLocal(message);
v8::NeanderArray global_listeners(FACTORY->message_listeners());
int global_length = global_listeners.length();
if (global_length == 0) {
DefaultMessageReport(loc, message);
+ if (isolate->has_scheduled_exception()) {
+ isolate->clear_scheduled_exception();
+ }
} else {
for (int i = 0; i < global_length; i++) {
HandleScope scope;
@@ -124,8 +144,13 @@
FUNCTION_CAST<v8::MessageCallback>(callback_obj->proxy());
Handle<Object> callback_data(listener.get(1));
callback(api_message_obj, v8::Utils::ToLocal(callback_data));
+ if (isolate->has_scheduled_exception()) {
+ isolate->clear_scheduled_exception();
+ }
}
}
+
+ isolate->set_in_exception_reporting(false);
}
=======================================
--- /branches/bleeding_edge/src/messages.h Wed Feb 2 05:31:52 2011
+++ /branches/bleeding_edge/src/messages.h Thu Apr 7 12:52:24 2011
@@ -101,7 +101,9 @@
Handle<JSArray> stack_frames);
// Report a formatted message (needs JS allocation).
- static void ReportMessage(MessageLocation* loc, Handle<Object> message);
+ static void ReportMessage(Isolate* isolate,
+ MessageLocation* loc,
+ Handle<Object> message);
static void DefaultMessageReport(const MessageLocation* loc,
Handle<Object> message_obj);
=======================================
--- /branches/bleeding_edge/src/top.cc Tue Apr 5 02:21:02 2011
+++ /branches/bleeding_edge/src/top.cc Thu Apr 7 12:52:24 2011
@@ -72,6 +72,7 @@
int id = Isolate::Current()->thread_manager()->CurrentId();
thread_id_ = (id == 0) ? ThreadManager::kInvalidId : id;
external_caught_exception_ = false;
+ in_exception_reporting_ = false;
failed_access_check_callback_ = NULL;
save_context_ = NULL;
catcher_ = NULL;
@@ -794,55 +795,38 @@
void Isolate::ReportPendingMessages() {
ASSERT(has_pending_exception());
+ PropagatePendingExceptionToExternalTryCatch();
+
// 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 = IsExternallyCaught();
- thread_local_top()->external_caught_exception_ = external_caught;
- HandleScope scope(this);
- if (thread_local_top()->pending_exception_ ==
- Failure::OutOfMemoryException()) {
+ HandleScope scope;
+ if (thread_local_top_.pending_exception_ ==
Failure::OutOfMemoryException()) {
context()->mark_out_of_memory();
- } else if (thread_local_top()->pending_exception_ ==
- heap_.termination_exception()) {
- if (external_caught) {
- try_catch_handler()->can_continue_ = false;
- try_catch_handler()->exception_ = heap_.null_value();
- }
+ } else if (thread_local_top_.pending_exception_ ==
+ heap()->termination_exception()) {
+ // Do nothing: if needed, the exception has been already propagated to
+ // v8::TryCatch.
} else {
- // At this point all non-object (failure) exceptions have
- // been dealt with so this shouldn't fail.
- Object* pending_exception_object =
pending_exception()->ToObjectUnchecked();
- Handle<Object> exception(pending_exception_object);
- thread_local_top()->external_caught_exception_ = false;
- if (external_caught) {
- try_catch_handler()->can_continue_ = true;
- try_catch_handler()->exception_ =
thread_local_top()->pending_exception_;
- if (!thread_local_top()->pending_message_obj_->IsTheHole()) {
- try_catch_handler()->message_ =
- thread_local_top()->pending_message_obj_;
- }
- }
- if (thread_local_top()->has_pending_message_) {
- thread_local_top()->has_pending_message_ = false;
- if (thread_local_top()->pending_message_ != NULL) {
-
MessageHandler::ReportMessage(thread_local_top()->pending_message_);
- } else if (!thread_local_top()->pending_message_obj_->IsTheHole()) {
- Handle<Object>
message_obj(thread_local_top()->pending_message_obj_);
- if (thread_local_top()->pending_message_script_ != NULL) {
- Handle<Script>
script(thread_local_top()->pending_message_script_);
- int start_pos = thread_local_top()->pending_message_start_pos_;
- int end_pos = thread_local_top()->pending_message_end_pos_;
+ if (thread_local_top_.has_pending_message_) {
+ thread_local_top_.has_pending_message_ = false;
+ if (thread_local_top_.pending_message_ != NULL) {
+ MessageHandler::ReportMessage(thread_local_top_.pending_message_);
+ } else if (!thread_local_top_.pending_message_obj_->IsTheHole()) {
+ HandleScope scope;
+ Handle<Object> message_obj(thread_local_top_.pending_message_obj_);
+ if (thread_local_top_.pending_message_script_ != NULL) {
+ Handle<Script> script(thread_local_top_.pending_message_script_);
+ int start_pos = thread_local_top_.pending_message_start_pos_;
+ int end_pos = thread_local_top_.pending_message_end_pos_;
MessageLocation location(script, start_pos, end_pos);
- MessageHandler::ReportMessage(&location, message_obj);
+ MessageHandler::ReportMessage(this, &location, message_obj);
} else {
- MessageHandler::ReportMessage(NULL, message_obj);
+ MessageHandler::ReportMessage(this, NULL, message_obj);
}
}
}
- thread_local_top()->external_caught_exception_ = external_caught;
- set_pending_exception(*exception);
}
clear_pending_message();
}
@@ -854,6 +838,9 @@
bool Isolate::OptionalRescheduleException(bool is_bottom_call) {
+ ASSERT(has_pending_exception());
+ PropagatePendingExceptionToExternalTryCatch();
+
// Allways reschedule out of memory exceptions.
if (!is_out_of_memory()) {
bool is_termination_exception =
@@ -966,7 +953,7 @@
memcpy(reinterpret_cast<char*>(thread_local_top()), from,
sizeof(ThreadLocalTop));
// This might be just paranoia, but it seems to be needed in case a
- // thread_local_ is restored on a separate OS thread.
+ // thread_local_top_ is restored on a separate OS thread.
#ifdef USE_SIMULATOR
#ifdef V8_TARGET_ARCH_ARM
thread_local_top()->simulator_ = Simulator::current(this);
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Thu Apr 7 02:51:25 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc Thu Apr 7 12:52:24 2011
@@ -50,18 +50,26 @@
}
using ::v8::AccessorInfo;
+using ::v8::Arguments;
using ::v8::Context;
using ::v8::Extension;
using ::v8::Function;
+using ::v8::FunctionTemplate;
+using ::v8::Handle;
using ::v8::HandleScope;
using ::v8::Local;
+using ::v8::Message;
+using ::v8::MessageCallback;
using ::v8::Object;
using ::v8::ObjectTemplate;
using ::v8::Persistent;
using ::v8::Script;
+using ::v8::StackTrace;
using ::v8::String;
-using ::v8::Value;
+using ::v8::TryCatch;
+using ::v8::Undefined;
using ::v8::V8;
+using ::v8::Value;
namespace i = ::i;
@@ -8572,6 +8580,128 @@
ExpectTrue("obj.x === 42");
ExpectTrue("!obj.propertyIsEnumerable('x')");
}
+
+
+static Handle<Value> ThrowingGetter(Local<String> name,
+ const AccessorInfo& info) {
+ ApiTestFuzzer::Fuzz();
+ ThrowException(Handle<Value>());
+ return Undefined();
+}
+
+
+THREADED_TEST(VariousGetPropertiesAndThrowingCallbacks) {
+ HandleScope scope;
+ LocalContext context;
+
+ Local<FunctionTemplate> templ = FunctionTemplate::New();
+ Local<ObjectTemplate> instance_templ = templ->InstanceTemplate();
+ instance_templ->SetAccessor(v8_str("f"), ThrowingGetter);
+
+ Local<Object> instance = templ->GetFunction()->NewInstance();
+
+ Local<Object> another = Object::New();
+ another->SetPrototype(instance);
+
+ Local<Object> with_js_getter = CompileRun(
+ "o = {};\n"
+ "o.__defineGetter__('f', function() { throw undefined; });\n"
+ "o\n").As<Object>();
+ CHECK(!with_js_getter.IsEmpty());
+
+ TryCatch try_catch;
+
+ Local<Value> result = instance->GetRealNamedProperty(v8_str("f"));
+ CHECK(try_catch.HasCaught());
+ try_catch.Reset();
+ CHECK(result.IsEmpty());
+
+ result = another->GetRealNamedProperty(v8_str("f"));
+ CHECK(try_catch.HasCaught());
+ try_catch.Reset();
+ CHECK(result.IsEmpty());
+
+ result = another->GetRealNamedPropertyInPrototypeChain(v8_str("f"));
+ CHECK(try_catch.HasCaught());
+ try_catch.Reset();
+ CHECK(result.IsEmpty());
+
+ result = another->Get(v8_str("f"));
+ CHECK(try_catch.HasCaught());
+ try_catch.Reset();
+ CHECK(result.IsEmpty());
+
+ result = with_js_getter->GetRealNamedProperty(v8_str("f"));
+ CHECK(try_catch.HasCaught());
+ try_catch.Reset();
+ CHECK(result.IsEmpty());
+
+ result = with_js_getter->Get(v8_str("f"));
+ CHECK(try_catch.HasCaught());
+ try_catch.Reset();
+ CHECK(result.IsEmpty());
+}
+
+
+static Handle<Value> ThrowingCallbackWithTryCatch(const Arguments& args) {
+ TryCatch try_catch;
+ // Verboseness is important: it triggers message delivery which can call
into
+ // external code.
+ try_catch.SetVerbose(true);
+ CompileRun("throw 'from JS';");
+ CHECK(try_catch.HasCaught());
+ CHECK(!i::Isolate::Current()->has_pending_exception());
+ CHECK(!i::Isolate::Current()->has_scheduled_exception());
+ return Undefined();
+}
+
+
+static void WithTryCatch(Handle<Message> message, Handle<Value> data) {
+ TryCatch try_catch;
+}
+
+
+static void ThrowFromJS(Handle<Message> message, Handle<Value> data) {
+ CompileRun("throw 'ThrowInJS';");
+}
+
+
+static void ThrowViaApi(Handle<Message> message, Handle<Value> data) {
+ ThrowException(v8_str("ThrowViaApi"));
+}
+
+
+static void WebKitLike(Handle<Message> message, Handle<Value> data) {
+ Handle<String> errorMessageString = message->Get();
+ CHECK(!errorMessageString.IsEmpty());
+ message->GetStackTrace();
+ message->GetScriptResourceName();
+}
+
+THREADED_TEST(ExceptionsDoNotPropagatePastTryCatch) {
+ HandleScope scope;
+ LocalContext context;
+
+ Local<Function> func =
+ FunctionTemplate::New(ThrowingCallbackWithTryCatch)->GetFunction();
+ context->Global()->Set(v8_str("func"), func);
+
+ MessageCallback callbacks[] =
+ { NULL, WebKitLike, ThrowViaApi, ThrowFromJS, WithTryCatch };
+ for (unsigned i = 0; i < sizeof(callbacks)/sizeof(callbacks[0]); i++) {
+ MessageCallback callback = callbacks[i];
+ if (callback != NULL) {
+ V8::AddMessageListener(callback);
+ }
+ ExpectFalse(
+ "var thrown = false;\n"
+ "try { func(); } catch(e) { thrown = true; }\n"
+ "thrown\n");
+ if (callback != NULL) {
+ V8::RemoveMessageListeners(callback);
+ }
+ }
+}
static v8::Handle<Value> ParentGetter(Local<String> name,
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
