Skip to content

Commit

Permalink
deps: cherry-pick 09b53ee from upstream V8
Browse files Browse the repository at this point in the history
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 <[email protected]>
    Reviewed-by: Yang Guo <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#49466}

Refs: v8/v8@09b53ee

PR-URL: #21767
Refs: v8/v8@09b53ee
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
  • Loading branch information
addaleax authored and rvagg committed Aug 16, 2018
1 parent 6f545d1 commit ffb72f8
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 4 deletions.
2 changes: 1 addition & 1 deletion deps/v8/AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Alexis Campailla <[email protected]>
Andreas Anyuru <[email protected]>
Andrew Paprocki <[email protected]>
Andrei Kashcha <[email protected]>
Anna Henningsen <[email protected]>
Anna Henningsen <[email protected]>
Bangfu Tao <[email protected]>
Ben Noordhuis <[email protected]>
Benjamin Tan <[email protected]>
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/include/v8-version.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/messages.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
36 changes: 34 additions & 2 deletions deps/v8/test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5747,23 +5747,55 @@ TEST(CustomErrorMessage) {

static void check_custom_rethrowing_message(v8::Local<v8::Message> message,
v8::Local<v8::Value> data) {
CHECK(data->IsExternal());
int* callcount = static_cast<int*>(data.As<v8::External>()->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);
}
Expand Down

0 comments on commit ffb72f8

Please sign in to comment.