Revision: 7258
Author: [email protected]
Date: Fri Mar 18 05:33:19 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/6397011
http://code.google.com/p/v8/source/detail?r=7258
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/handles.cc
/branches/bleeding_edge/src/handles.h
/branches/bleeding_edge/src/messages.cc
/branches/bleeding_edge/src/top.cc
/branches/bleeding_edge/src/top.h
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Wed Mar 16 12:55:31 2011
+++ /branches/bleeding_edge/src/api.cc Fri Mar 18 05:33:19 2011
@@ -2391,6 +2391,9 @@
ENTER_V8;
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
+ // propagate outside.
+ TryCatch try_catch;
EXCEPTION_PREAMBLE();
i::Handle<i::Object> result = i::SetPrototype(self, value_obj);
has_pending_exception = result.is_null();
@@ -2575,6 +2578,25 @@
ON_BAILOUT("v8::Object::HasIndexedLookupInterceptor()", return false);
return Utils::OpenHandle(this)->HasIndexedInterceptor();
}
+
+
+static Local<Value> GetPropertyByLookup(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();
+ i::Handle<i::Object> result = i::GetProperty(receiver, name, lookup);
+ has_pending_exception = result.is_null();
+ EXCEPTION_BAILOUT_CHECK(Local<Value>());
+
+ return Utils::ToLocal(result);
+}
Local<Value> v8::Object::GetRealNamedPropertyInPrototypeChain(
@@ -2586,17 +2608,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(self_obj, key_obj, &lookup);
}
@@ -2607,17 +2619,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(self_obj, key_obj, &lookup);
}
=======================================
--- /branches/bleeding_edge/src/handles.cc Thu Mar 17 13:28:17 2011
+++ /branches/bleeding_edge/src/handles.cc Fri Mar 18 05:33:19 2011
@@ -331,6 +331,15 @@
Handle<Object> key) {
CALL_HEAP_FUNCTION(Runtime::GetObjectProperty(obj, key), Object);
}
+
+
+Handle<Object> GetProperty(Handle<JSObject> obj,
+ Handle<String> name,
+ LookupResult* result) {
+ PropertyAttributes attributes;
+ CALL_HEAP_FUNCTION(obj->GetProperty(*obj, result, *name, &attributes),
+ Object);
+}
Handle<Object> GetElement(Handle<Object> obj,
=======================================
--- /branches/bleeding_edge/src/handles.h Thu Mar 17 13:28:17 2011
+++ /branches/bleeding_edge/src/handles.h Fri Mar 18 05:33:19 2011
@@ -280,6 +280,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/messages.cc Tue Feb 8 11:42:14 2011
+++ /branches/bleeding_edge/src/messages.cc Fri Mar 18 05:33:19 2011
@@ -109,12 +109,23 @@
void MessageHandler::ReportMessage(MessageLocation* loc,
Handle<Object> message) {
+ // 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.
+ Top::ExceptionScope exception_scope;
+ Top::clear_pending_exception();
+ Top::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 (Top::has_scheduled_exception()) {
+ // Consider logging it somehow.
+ Top::clear_scheduled_exception();
+ }
} else {
for (int i = 0; i < global_length; i++) {
HandleScope scope;
@@ -125,6 +136,10 @@
FUNCTION_CAST<v8::MessageCallback>(callback_obj->proxy());
Handle<Object> callback_data(listener.get(1));
callback(api_message_obj, v8::Utils::ToLocal(callback_data));
+ if (Top::has_scheduled_exception()) {
+ // Consider logging it somehow.
+ Top::clear_scheduled_exception();
+ }
}
}
}
=======================================
--- /branches/bleeding_edge/src/top.cc Tue Feb 22 22:55:47 2011
+++ /branches/bleeding_edge/src/top.cc Fri Mar 18 05:33:19 2011
@@ -969,43 +969,57 @@
return true;
}
+
+
+void Top::PropagatePendingExceptionToExternalTryCatch() {
+ ASSERT(has_pending_exception());
+
+ bool external_caught = IsExternallyCaught();
+ thread_local_.external_caught_exception_ = external_caught;
+
+ if (!external_caught) return;
+
+ if (thread_local_.pending_exception_ == Failure::OutOfMemoryException())
{
+ // Do not propagate OOM exception: we should kill VM asap.
+ } else if (thread_local_.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_.pending_message_obj_->IsTheHole()) {
+ try_catch_handler()->message_ = thread_local_.pending_message_obj_;
+ }
+ }
+}
void Top::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_.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) {
- try_catch_handler()->can_continue_ = false;
- try_catch_handler()->exception_ = Heap::null_value();
- }
+ // 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_.external_caught_exception_ = false;
- if (external_caught) {
- 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_;
- }
- }
if (thread_local_.has_pending_message_) {
thread_local_.has_pending_message_ = false;
if (thread_local_.pending_message_ != NULL) {
MessageHandler::ReportMessage(thread_local_.pending_message_);
} else if (!thread_local_.pending_message_obj_->IsTheHole()) {
+ HandleScope scope;
Handle<Object> message_obj(thread_local_.pending_message_obj_);
if (thread_local_.pending_message_script_ != NULL) {
Handle<Script> script(thread_local_.pending_message_script_);
@@ -1018,8 +1032,6 @@
}
}
}
- thread_local_.external_caught_exception_ = external_caught;
- set_pending_exception(*exception);
}
clear_pending_message();
}
@@ -1031,6 +1043,9 @@
bool Top::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 =
=======================================
--- /branches/bleeding_edge/src/top.h Wed Feb 16 03:40:48 2011
+++ /branches/bleeding_edge/src/top.h Fri Mar 18 05:33:19 2011
@@ -195,22 +195,26 @@
ASSERT(has_pending_exception());
return thread_local_.pending_exception_;
}
- static bool external_caught_exception() {
- return thread_local_.external_caught_exception_;
- }
static void set_pending_exception(MaybeObject* exception) {
thread_local_.pending_exception_ = exception;
}
static void clear_pending_exception() {
thread_local_.pending_exception_ = Heap::the_hole_value();
}
-
static MaybeObject** pending_exception_address() {
return &thread_local_.pending_exception_;
}
static bool has_pending_exception() {
return !thread_local_.pending_exception_->IsTheHole();
}
+
+ static bool external_caught_exception() {
+ return thread_local_.external_caught_exception_;
+ }
+ static void set_external_caught_exception(bool value) {
+ thread_local_.external_caught_exception_ = value;
+ }
+
static void clear_pending_message() {
thread_local_.has_pending_message_ = false;
thread_local_.pending_message_ = NULL;
@@ -461,6 +465,25 @@
static const char* kStackOverflowMessage;
private:
+
+ static v8::TryCatch* catcher() {
+ return thread_local_.catcher_;
+ }
+
+ static void set_catcher(v8::TryCatch* catcher) {
+ thread_local_.catcher_ = catcher;
+ }
+
+ static void setup_external_caught() {
+ thread_local_.external_caught_exception_ =
+ has_pending_exception() &&
+ (thread_local_.catcher_ != NULL) &&
+ (try_catch_handler() == thread_local_.catcher_);
+ }
+
+ // Attempts to propagate the pending exception to the proper
v8::TryCatch.
+ static void PropagatePendingExceptionToExternalTryCatch();
+
#ifdef ENABLE_VMSTATE_TRACKING
// Set of states used when communicating with the runtime profiler.
//
@@ -525,6 +548,29 @@
friend class ThreadLocalTop;
static void FillCache();
+
+ public:
+ class ExceptionScope {
+ public:
+ ExceptionScope() :
+ // Scope currently can only be used for regular exceptions, not
+ // failures like OOM or termination exception.
+ pending_exception_(Top::pending_exception()->ToObjectUnchecked()),
+ external_caught_exception_(Top::external_caught_exception()),
+ catcher_(Top::catcher())
+ { }
+
+ ~ExceptionScope() {
+ Top::set_catcher(catcher_);
+ Top::set_external_caught_exception(external_caught_exception);
+ Top::set_pending_exception(*pending_exception_);
+ }
+
+ private:
+ Handle<Object> pending_exception_;
+ bool external_caught_exception_;
+ v8::TryCatch* catcher_;
+ };
};
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Wed Mar 16 12:55:31 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc Fri Mar 18 05:33:19 2011
@@ -51,18 +51,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;
@@ -8574,6 +8582,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::Top::has_pending_exception());
+ CHECK(!i::Top::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
