From ffb72f810e834441ef0f198eea300199ff0ce5d6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 11 Jul 2018 18:58:01 +0200 Subject: [PATCH] deps: cherry-pick 09b53ee from upstream V8 Original commit message: [api] Make running scripts in AddMessageListener callback work in debug mode The existance of an `AllowJavascriptExecutionDebugOnly` scope in `Isolate::ReportPendingMessages()` indicates that the API supports running arbitrary JS code in a `AddMessageListener` callback. Currently, this can fail in debug mode: The `!isolate->external_caught_exception()` condition is checked when entering API methods inside such a handler. However, if there is a verbose `TryCatch` active when the exception occurs, this check fails, and when calling `ToString()` on the exception object leaves a pending exception itself, the flag is re-set to `true`. Fix this problem by clearing the flag and the pending exception if there was one during `ToString()`. This matches the code a few lines up in `messages.cc`, so the exception state is now consistent during the callback. This currently makes a Node.js test fail in debug mode (`parallel/test-error-reporting`). Bug: node:7144 Bug: node:17016 Change-Id: I060d00fea3e9a497f4df34c6ff8d6e29ebe96321 Reviewed-on: https://chromium-review.googlesource.com/718096 Commit-Queue: Yang Guo Reviewed-by: Yang Guo Cr-Commit-Position: refs/heads/master@{#49466} Refs: https://github.com/v8/v8/commit/09b53eef4ca65a48e74fe0b199b2ee892a4321ed PR-URL: https://github.com/nodejs/node/pull/21767 Refs: https://github.com/v8/v8/commit/09b53eef4ca65a48e74fe0b199b2ee892a4321ed Reviewed-By: Ruben Bridgewater Reviewed-By: Myles Borins --- deps/v8/AUTHORS | 2 +- deps/v8/include/v8-version.h | 2 +- deps/v8/src/messages.cc | 3 +++ deps/v8/test/cctest/test-api.cc | 36 +++++++++++++++++++++++++++++++-- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/deps/v8/AUTHORS b/deps/v8/AUTHORS index c54cad02b2d15e..3bc767945cfdee 100644 --- a/deps/v8/AUTHORS +++ b/deps/v8/AUTHORS @@ -42,7 +42,7 @@ Alexis Campailla Andreas Anyuru Andrew Paprocki Andrei Kashcha -Anna Henningsen +Anna Henningsen Bangfu Tao Ben Noordhuis Benjamin Tan diff --git a/deps/v8/include/v8-version.h b/deps/v8/include/v8-version.h index ddecda5abe18c5..abbcc76c406b4d 100644 --- a/deps/v8/include/v8-version.h +++ b/deps/v8/include/v8-version.h @@ -11,7 +11,7 @@ #define V8_MAJOR_VERSION 6 #define V8_MINOR_VERSION 2 #define V8_BUILD_NUMBER 414 -#define V8_PATCH_LEVEL 61 +#define V8_PATCH_LEVEL 62 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) diff --git a/deps/v8/src/messages.cc b/deps/v8/src/messages.cc index 780198dac6b832..eb869ffcc8fc9c 100644 --- a/deps/v8/src/messages.cc +++ b/deps/v8/src/messages.cc @@ -114,6 +114,9 @@ void MessageHandler::ReportMessage(Isolate* isolate, const MessageLocation* loc, } if (!maybe_stringified.ToHandle(&stringified)) { + DCHECK(isolate->has_pending_exception()); + isolate->clear_pending_exception(); + isolate->set_external_caught_exception(false); stringified = isolate->factory()->NewStringFromAsciiChecked("exception"); } diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index b0e070a68a3046..2a5d48037fa47d 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -5747,23 +5747,55 @@ TEST(CustomErrorMessage) { static void check_custom_rethrowing_message(v8::Local message, v8::Local data) { + CHECK(data->IsExternal()); + int* callcount = static_cast(data.As()->Value()); + ++*callcount; + const char* uncaught_error = "Uncaught exception"; CHECK(message->Get() ->Equals(CcTest::isolate()->GetCurrentContext(), v8_str(uncaught_error)) .FromJust()); + // Test that compiling code inside a message handler works. + CHECK(CompileRunChecked(CcTest::isolate(), "(function(a) { return a; })(42)") + ->Equals(CcTest::isolate()->GetCurrentContext(), + v8::Integer::NewFromUnsigned(CcTest::isolate(), 42)) + .FromJust()); } TEST(CustomErrorRethrowsOnToString) { + int callcount = 0; LocalContext context; - v8::HandleScope scope(context->GetIsolate()); - context->GetIsolate()->AddMessageListener(check_custom_rethrowing_message); + v8::Isolate* isolate = context->GetIsolate(); + v8::HandleScope scope(isolate); + context->GetIsolate()->AddMessageListener( + check_custom_rethrowing_message, v8::External::New(isolate, &callcount)); + + CompileRun( + "var e = { toString: function() { throw e; } };" + "try { throw e; } finally {}"); + + CHECK_EQ(callcount, 1); + context->GetIsolate()->RemoveMessageListeners( + check_custom_rethrowing_message); +} + +TEST(CustomErrorRethrowsOnToStringInsideVerboseTryCatch) { + int callcount = 0; + LocalContext context; + v8::Isolate* isolate = context->GetIsolate(); + v8::HandleScope scope(isolate); + v8::TryCatch try_catch(isolate); + try_catch.SetVerbose(true); + context->GetIsolate()->AddMessageListener( + check_custom_rethrowing_message, v8::External::New(isolate, &callcount)); CompileRun( "var e = { toString: function() { throw e; } };" "try { throw e; } finally {}"); + CHECK_EQ(callcount, 1); context->GetIsolate()->RemoveMessageListeners( check_custom_rethrowing_message); }