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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7258 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -2391,6 +2391,9 @@ bool v8::Object::SetPrototype(Handle<Value> value) {
   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();
@@ -2577,6 +2580,25 @@ bool v8::Object::HasIndexedLookupInterceptor() {
 }
 
 
+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(
       Handle<String> key) {
   ON_BAILOUT("v8::Object::GetRealNamedPropertyInPrototypeChain()",
@@ -2586,17 +2608,7 @@ Local<Value> v8::Object::GetRealNamedPropertyInPrototypeChain(
   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 @@ Local<Value> v8::Object::GetRealNamedProperty(Handle<String> key) {
   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);
 }
 
 

src/handles.cc

@@ -333,6 +333,15 @@ Handle<Object> GetProperty(Handle<Object> obj,
 }
 
 
+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,
                           uint32_t index) {
   CALL_HEAP_FUNCTION(Runtime::GetElement(obj, index), Object);

src/handles.h

@@ -280,6 +280,11 @@ Handle<Object> GetProperty(Handle<JSObject> obj,
 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);
 

src/messages.cc

@@ -109,12 +109,23 @@ Handle<JSMessageObject> MessageHandler::MakeMessageObject(
 
 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 @@ void MessageHandler::ReportMessage(MessageLocation* loc,
           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();
+      }
     }
   }
 }

src/top.cc

@@ -971,41 +971,55 @@ bool Top::IsExternallyCaught() {
 }
 
 
+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 @@ void Top::ReportPendingMessages() {
         }
       }
     }
-    thread_local_.external_caught_exception_ = external_caught;
-    set_pending_exception(*exception);
   }
   clear_pending_message();
 }
@@ -1031,6 +1043,9 @@ void Top::TraceException(bool flag) {
 
 
 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 =

src/top.h

@@ -195,22 +195,26 @@ class Top {
     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 @@ class Top {
   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 @@ class Top {
   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_;
+  };
 };
 
 

test/cctest/test-api.cc

@@ -51,18 +51,26 @@ static bool IsNaN(double x) {
 }
 
 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;
 
@@ -8576,6 +8584,128 @@ THREADED_TEST(NamedPropertyHandlerGetterAttributes) {
 }
 
 
+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,
                                       const AccessorInfo& info) {
   ApiTestFuzzer::Fuzz();