From 36d8363c1cace489cbccfedcf3777863b12d4887 Mon Sep 17 00:00:00 2001 From: yangguo Date: Wed, 27 May 2015 23:30:08 -0700 Subject: [PATCH] Do not eagerly convert exception to string when creating a message object R=mstarzinger@chromium.org BUG=chromium:490680 LOG=Y Review URL: https://codereview.chromium.org/1157563005 Cr-Commit-Position: refs/heads/master@{#28670} --- src/debug.cc | 2 +- src/isolate.cc | 11 ------ src/messages.cc | 55 +++++++++++++++++++--------- src/messages.h | 2 +- test/cctest/test-api.cc | 20 ++++++++++ test/mjsunit/regress/regress-crbug-490680.js | 18 +++++++++ 6 files changed, 77 insertions(+), 31 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-490680.js diff --git a/src/debug.cc b/src/debug.cc index b5e81c5..b63e99d 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -629,7 +629,7 @@ bool Debug::CompileDebuggerScript(Isolate* isolate, int index) { DCHECK(!isolate->has_pending_exception()); MessageLocation computed_location; isolate->ComputeLocation(&computed_location); - Handle message = MessageHandler::MakeMessageObject( + Handle message = MessageHandler::MakeMessageObject( isolate, MessageTemplate::kDebuggerLoading, &computed_location, isolate->factory()->undefined_value(), Handle()); DCHECK(!isolate->has_pending_exception()); diff --git a/src/isolate.cc b/src/isolate.cc index 0b24786..dc5b536 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -1377,17 +1377,6 @@ Handle Isolate::CreateMessage(Handle exception, location = &potential_computed_location; } - // If the exception argument is a custom object, turn it into a string - // before throwing as uncaught exception. Note that the pending - // exception object to be set later must not be turned into a string. - if (exception->IsJSObject() && !IsErrorObject(exception)) { - MaybeHandle maybe_exception = - Execution::ToDetailString(this, exception); - if (!maybe_exception.ToHandle(&exception)) { - exception = - factory()->InternalizeOneByteString(STATIC_CHAR_VECTOR("exception")); - } - } return MessageHandler::MakeMessageObject( this, MessageTemplate::kUncaughtException, location, exception, stack_trace_object); diff --git a/src/messages.cc b/src/messages.cc index c21f96c..daf6c82 100644 --- a/src/messages.cc +++ b/src/messages.cc @@ -59,9 +59,8 @@ Handle MessageHandler::MakeMessageObject( } -void MessageHandler::ReportMessage(Isolate* isolate, - MessageLocation* loc, - Handle message) { +void MessageHandler::ReportMessage(Isolate* isolate, MessageLocation* loc, + Handle 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. @@ -71,14 +70,29 @@ void MessageHandler::ReportMessage(Isolate* isolate, if (isolate->has_pending_exception()) { exception_object = isolate->pending_exception(); } - Handle exception_handle(exception_object, isolate); + Handle exception(exception_object, isolate); Isolate::ExceptionScope exception_scope(isolate); isolate->clear_pending_exception(); isolate->set_external_caught_exception(false); + // Turn the exception on the message into a string if it is an object. + if (message->argument()->IsJSObject()) { + HandleScope scope(isolate); + Handle argument(message->argument(), isolate); + Handle args[] = {argument}; + MaybeHandle maybe_stringified = Execution::TryCall( + isolate->to_detail_string_fun(), isolate->js_builtins_object(), + arraysize(args), args); + Handle stringified; + if (!maybe_stringified.ToHandle(&stringified)) { + stringified = isolate->factory()->NewStringFromAsciiChecked("exception"); + } + message->set_argument(*stringified); + } + v8::Local api_message_obj = v8::Utils::MessageToLocal(message); - v8::Local api_exception_obj = v8::Utils::ToLocal(exception_handle); + v8::Local api_exception_obj = v8::Utils::ToLocal(exception); v8::NeanderArray global_listeners(isolate->factory()->message_listeners()); int global_length = global_listeners.length(); @@ -280,22 +294,27 @@ Handle MessageTemplate::FormatMessage(Isolate* isolate, int template_index, Handle arg) { Factory* factory = isolate->factory(); - Handle fmt_str = factory->InternalizeOneByteString( - STATIC_CHAR_VECTOR("$noSideEffectToString")); - Handle fun = Handle::cast( - Object::GetProperty(isolate->js_builtins_object(), fmt_str) - .ToHandleChecked()); - - MaybeHandle maybe_result = - Execution::TryCall(fun, isolate->js_builtins_object(), 1, &arg); - Handle result; - if (!maybe_result.ToHandle(&result) || !result->IsString()) { - return factory->InternalizeOneByteString(STATIC_CHAR_VECTOR("")); + Handle result_string; + if (arg->IsString()) { + result_string = Handle::cast(arg); + } else { + Handle fmt_str = factory->InternalizeOneByteString( + STATIC_CHAR_VECTOR("$noSideEffectToString")); + Handle fun = Handle::cast( + Object::GetProperty(isolate->js_builtins_object(), fmt_str) + .ToHandleChecked()); + + MaybeHandle maybe_result = + Execution::TryCall(fun, isolate->js_builtins_object(), 1, &arg); + Handle result; + if (!maybe_result.ToHandle(&result) || !result->IsString()) { + return factory->InternalizeOneByteString(STATIC_CHAR_VECTOR("")); + } + result_string = Handle::cast(result); } MaybeHandle maybe_result_string = MessageTemplate::FormatMessage( - template_index, Handle::cast(result), factory->empty_string(), + template_index, result_string, factory->empty_string(), factory->empty_string()); - Handle result_string; if (!maybe_result_string.ToHandle(&result_string)) { return factory->InternalizeOneByteString(STATIC_CHAR_VECTOR("")); } diff --git a/src/messages.h b/src/messages.h index d0b9d31..5527929 100644 --- a/src/messages.h +++ b/src/messages.h @@ -438,7 +438,7 @@ class MessageHandler { // Report a formatted message (needs JS allocation). static void ReportMessage(Isolate* isolate, MessageLocation* loc, - Handle message); + Handle message); static void DefaultMessageReport(Isolate* isolate, const MessageLocation* loc, Handle message_obj); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index e45b29a..669a5cf 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -4950,6 +4950,26 @@ TEST(CustomErrorMessage) { } +static void check_custom_rethrowing_message(v8::Handle message, + v8::Handle data) { + const char* uncaught_error = "Uncaught exception"; + CHECK(message->Get()->Equals(v8_str(uncaught_error))); +} + + +TEST(CustomErrorRethrowsOnToString) { + LocalContext context; + v8::HandleScope scope(context->GetIsolate()); + v8::V8::AddMessageListener(check_custom_rethrowing_message); + + CompileRun( + "var e = { toString: function() { throw e; } };" + "try { throw e; } finally {}"); + + v8::V8::RemoveMessageListeners(check_custom_rethrowing_message); +} + + static void receive_message(v8::Handle message, v8::Handle data) { message->Get(); diff --git a/test/mjsunit/regress/regress-crbug-490680.js b/test/mjsunit/regress/regress-crbug-490680.js new file mode 100644 index 0000000..735f3b7 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-490680.js @@ -0,0 +1,18 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +var sentinel = null; + +try { + throw { toString: function() { sentinel = "observed"; } }; +} catch (e) { +} + +L: try { + throw { toString: function() { sentinel = "observed"; } }; +} finally { + break L; +} + +assertNull(sentinel); -- 2.7.4