Skip to content

Commit 5957afc

Browse files
devsnekcodebytere
authored andcommitted
deps: V8: cherry-pick 767e65f945e7
Original commit message: [API] Fix microtask message reporting RunSingleMicrotask calls Runtime::ReportMessage, but the implementation of ReportMessage would unconditionally discard these exceptions. This CL removes all of the intermediate logic and directly calls MessageHandler::ReportMessage, restoring the ability of RunSingleMicrotask to report exceptions that occur in microtasks. Bug: v8:8326 Change-Id: I493de74383b2ab191d786611fb9eba9d27e7a243 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2162121 Commit-Queue: Gus Caplan <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#67630} Refs: v8/v8@767e65f PR-URL: #33859 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 98228b2 commit 5957afc

File tree

8 files changed

+91
-141
lines changed

8 files changed

+91
-141
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.22',
39+
'v8_embedder_string': '-node.23',
4040

4141
##### V8 defaults for Node.js #####
4242

deps/v8/src/builtins/builtins-microtask-queue-gen.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
327327
BIND(&if_exception);
328328
{
329329
// Report unhandled exceptions from microtasks.
330-
CallRuntime(Runtime::kReportMessage, current_context,
330+
CallRuntime(Runtime::kReportMessageFromMicrotask, current_context,
331331
var_exception.value());
332332
RewindEnteredContext(saved_entered_context_count);
333333
SetCurrentContext(current_context);

deps/v8/src/builtins/promise-reaction-job.tq

+2-12
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,6 @@
44

55
#include 'src/builtins/builtins-promise-gen.h'
66

7-
namespace runtime {
8-
extern transitioning runtime
9-
ReportMessage(implicit context: Context)(JSAny): JSAny;
10-
}
11-
127
namespace promise {
138

149
transitioning
@@ -30,13 +25,8 @@ namespace promise {
3025
case (capability: PromiseCapability): {
3126
// In the general case we need to call the (user provided)
3227
// promiseCapability.[[Reject]] function.
33-
try {
34-
const reject = UnsafeCast<Callable>(capability.reject);
35-
return Call(context, reject, Undefined, reason);
36-
} catch (e) {
37-
// Swallow the exception here.
38-
return runtime::ReportMessage(e);
39-
}
28+
const reject = UnsafeCast<Callable>(capability.reject);
29+
return Call(context, reject, Undefined, reason);
4030
}
4131
}
4232
} else {

deps/v8/src/execution/isolate.cc

+55-115
Original file line numberDiff line numberDiff line change
@@ -1412,6 +1412,8 @@ void Isolate::InvokeApiInterruptCallbacks() {
14121412
}
14131413
}
14141414

1415+
namespace {
1416+
14151417
void ReportBootstrappingException(Handle<Object> exception,
14161418
MessageLocation* location) {
14171419
base::OS::PrintError("Exception thrown during bootstrapping\n");
@@ -1467,6 +1469,36 @@ void ReportBootstrappingException(Handle<Object> exception,
14671469
#endif
14681470
}
14691471

1472+
} // anonymous namespace
1473+
1474+
Handle<JSMessageObject> Isolate::CreateMessageOrAbort(
1475+
Handle<Object> exception, MessageLocation* location) {
1476+
Handle<JSMessageObject> message_obj = CreateMessage(exception, location);
1477+
1478+
// If the abort-on-uncaught-exception flag is specified, and if the
1479+
// embedder didn't specify a custom uncaught exception callback,
1480+
// or if the custom callback determined that V8 should abort, then
1481+
// abort.
1482+
if (FLAG_abort_on_uncaught_exception) {
1483+
CatchType prediction = PredictExceptionCatcher();
1484+
if ((prediction == NOT_CAUGHT || prediction == CAUGHT_BY_EXTERNAL) &&
1485+
(!abort_on_uncaught_exception_callback_ ||
1486+
abort_on_uncaught_exception_callback_(
1487+
reinterpret_cast<v8::Isolate*>(this)))) {
1488+
// Prevent endless recursion.
1489+
FLAG_abort_on_uncaught_exception = false;
1490+
// This flag is intended for use by JavaScript developers, so
1491+
// print a user-friendly stack trace (not an internal one).
1492+
PrintF(stderr, "%s\n\nFROM\n",
1493+
MessageHandler::GetLocalizedMessage(this, message_obj).get());
1494+
PrintCurrentStackTrace(stderr);
1495+
base::OS::Abort();
1496+
}
1497+
}
1498+
1499+
return message_obj;
1500+
}
1501+
14701502
Object Isolate::Throw(Object raw_exception, MessageLocation* location) {
14711503
DCHECK(!has_pending_exception());
14721504

@@ -1538,38 +1570,14 @@ Object Isolate::Throw(Object raw_exception, MessageLocation* location) {
15381570
if (location == nullptr && ComputeLocation(&computed_location)) {
15391571
location = &computed_location;
15401572
}
1541-
15421573
if (bootstrapper()->IsActive()) {
15431574
// It's not safe to try to make message objects or collect stack traces
15441575
// while the bootstrapper is active since the infrastructure may not have
15451576
// been properly initialized.
15461577
ReportBootstrappingException(exception, location);
15471578
} else {
1548-
Handle<Object> message_obj = CreateMessage(exception, location);
1579+
Handle<Object> message_obj = CreateMessageOrAbort(exception, location);
15491580
thread_local_top()->pending_message_obj_ = *message_obj;
1550-
1551-
// For any exception not caught by JavaScript, even when an external
1552-
// handler is present:
1553-
// If the abort-on-uncaught-exception flag is specified, and if the
1554-
// embedder didn't specify a custom uncaught exception callback,
1555-
// or if the custom callback determined that V8 should abort, then
1556-
// abort.
1557-
if (FLAG_abort_on_uncaught_exception) {
1558-
CatchType prediction = PredictExceptionCatcher();
1559-
if ((prediction == NOT_CAUGHT || prediction == CAUGHT_BY_EXTERNAL) &&
1560-
(!abort_on_uncaught_exception_callback_ ||
1561-
abort_on_uncaught_exception_callback_(
1562-
reinterpret_cast<v8::Isolate*>(this)))) {
1563-
// Prevent endless recursion.
1564-
FLAG_abort_on_uncaught_exception = false;
1565-
// This flag is intended for use by JavaScript developers, so
1566-
// print a user-friendly stack trace (not an internal one).
1567-
PrintF(stderr, "%s\n\nFROM\n",
1568-
MessageHandler::GetLocalizedMessage(this, message_obj).get());
1569-
PrintCurrentStackTrace(stderr);
1570-
base::OS::Abort();
1571-
}
1572-
}
15731581
}
15741582
}
15751583

@@ -2106,7 +2114,7 @@ bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
21062114
bool is_at_number_conversion =
21072115
elements->IsAsmJsWasmFrame(i) &&
21082116
elements->Flags(i).value() & FrameArray::kAsmJsAtNumberConversion;
2109-
if (elements->IsWasmCompiledFrame(i)) {
2117+
if (elements->IsWasmCompiledFrame(i) || elements->IsAsmJsWasmFrame(i)) {
21102118
// WasmCode* held alive by the {GlobalWasmCodeRef}.
21112119
wasm::WasmCode* code =
21122120
Managed<wasm::GlobalWasmCodeRef>::cast(elements->WasmCodeObject(i))
@@ -2230,9 +2238,28 @@ bool Isolate::IsExternalHandlerOnTop(Object exception) {
22302238
return (entry_handler > external_handler);
22312239
}
22322240

2233-
void Isolate::ReportPendingMessagesImpl(bool report_externally) {
2241+
std::vector<MemoryRange>* Isolate::GetCodePages() const {
2242+
return code_pages_.load(std::memory_order_acquire);
2243+
}
2244+
2245+
void Isolate::SetCodePages(std::vector<MemoryRange>* new_code_pages) {
2246+
code_pages_.store(new_code_pages, std::memory_order_release);
2247+
}
2248+
2249+
void Isolate::ReportPendingMessages() {
2250+
DCHECK(AllowExceptions::IsAllowed(this));
2251+
2252+
// The embedder might run script in response to an exception.
2253+
AllowJavascriptExecutionDebugOnly allow_script(this);
2254+
22342255
Object exception_obj = pending_exception();
22352256

2257+
// Try to propagate the exception to an external v8::TryCatch handler. If
2258+
// propagation was unsuccessful, then we will get another chance at reporting
2259+
// the pending message if the exception is re-thrown.
2260+
bool has_been_propagated = PropagatePendingExceptionToExternalTryCatch();
2261+
if (!has_been_propagated) return;
2262+
22362263
// Clear the pending message object early to avoid endless recursion.
22372264
Object message_obj = thread_local_top()->pending_message_obj_;
22382265
clear_pending_message();
@@ -2245,7 +2272,7 @@ void Isolate::ReportPendingMessagesImpl(bool report_externally) {
22452272
// depending on whether and external v8::TryCatch or an internal JavaScript
22462273
// handler is on top.
22472274
bool should_report_exception;
2248-
if (report_externally) {
2275+
if (IsExternalHandlerOnTop(exception_obj)) {
22492276
// Only report the exception if the external handler is verbose.
22502277
should_report_exception = try_catch_handler()->is_verbose_;
22512278
} else {
@@ -2271,93 +2298,6 @@ void Isolate::ReportPendingMessagesImpl(bool report_externally) {
22712298
}
22722299
}
22732300

2274-
std::vector<MemoryRange>* Isolate::GetCodePages() const {
2275-
return code_pages_.load(std::memory_order_acquire);
2276-
}
2277-
2278-
void Isolate::SetCodePages(std::vector<MemoryRange>* new_code_pages) {
2279-
code_pages_.store(new_code_pages, std::memory_order_release);
2280-
}
2281-
2282-
void Isolate::ReportPendingMessages() {
2283-
DCHECK(AllowExceptions::IsAllowed(this));
2284-
2285-
// The embedder might run script in response to an exception.
2286-
AllowJavascriptExecutionDebugOnly allow_script(this);
2287-
2288-
Object exception = pending_exception();
2289-
2290-
// Try to propagate the exception to an external v8::TryCatch handler. If
2291-
// propagation was unsuccessful, then we will get another chance at reporting
2292-
// the pending message if the exception is re-thrown.
2293-
bool has_been_propagated = PropagatePendingExceptionToExternalTryCatch();
2294-
if (!has_been_propagated) return;
2295-
2296-
ReportPendingMessagesImpl(IsExternalHandlerOnTop(exception));
2297-
}
2298-
2299-
void Isolate::ReportPendingMessagesFromJavaScript() {
2300-
DCHECK(AllowExceptions::IsAllowed(this));
2301-
2302-
auto IsHandledByJavaScript = [=]() {
2303-
// In this situation, the exception is always a non-terminating exception.
2304-
2305-
// Get the top-most JS_ENTRY handler, cannot be on top if it doesn't exist.
2306-
Address entry_handler = Isolate::handler(thread_local_top());
2307-
DCHECK_NE(entry_handler, kNullAddress);
2308-
entry_handler = StackHandler::FromAddress(entry_handler)->next_address();
2309-
2310-
// Get the address of the external handler so we can compare the address to
2311-
// determine which one is closer to the top of the stack.
2312-
Address external_handler = thread_local_top()->try_catch_handler_address();
2313-
if (external_handler == kNullAddress) return true;
2314-
2315-
return (entry_handler < external_handler);
2316-
};
2317-
2318-
auto IsHandledExternally = [=]() {
2319-
Address external_handler = thread_local_top()->try_catch_handler_address();
2320-
if (external_handler == kNullAddress) return false;
2321-
2322-
// Get the top-most JS_ENTRY handler, cannot be on top if it doesn't exist.
2323-
Address entry_handler = Isolate::handler(thread_local_top());
2324-
DCHECK_NE(entry_handler, kNullAddress);
2325-
entry_handler = StackHandler::FromAddress(entry_handler)->next_address();
2326-
return (entry_handler > external_handler);
2327-
};
2328-
2329-
auto PropagateToExternalHandler = [=]() {
2330-
if (IsHandledByJavaScript()) {
2331-
thread_local_top()->external_caught_exception_ = false;
2332-
return false;
2333-
}
2334-
2335-
if (!IsHandledExternally()) {
2336-
thread_local_top()->external_caught_exception_ = false;
2337-
return true;
2338-
}
2339-
2340-
thread_local_top()->external_caught_exception_ = true;
2341-
v8::TryCatch* handler = try_catch_handler();
2342-
DCHECK(thread_local_top()->pending_message_obj_.IsJSMessageObject() ||
2343-
thread_local_top()->pending_message_obj_.IsTheHole(this));
2344-
handler->can_continue_ = true;
2345-
handler->has_terminated_ = false;
2346-
handler->exception_ = reinterpret_cast<void*>(pending_exception().ptr());
2347-
// Propagate to the external try-catch only if we got an actual message.
2348-
if (thread_local_top()->pending_message_obj_.IsTheHole(this)) return true;
2349-
2350-
handler->message_obj_ =
2351-
reinterpret_cast<void*>(thread_local_top()->pending_message_obj_.ptr());
2352-
return true;
2353-
};
2354-
2355-
// Try to propagate to an external v8::TryCatch handler.
2356-
if (!PropagateToExternalHandler()) return;
2357-
2358-
ReportPendingMessagesImpl(true);
2359-
}
2360-
23612301
bool Isolate::OptionalRescheduleException(bool clear_exception) {
23622302
DCHECK(has_pending_exception());
23632303
PropagatePendingExceptionToExternalTryCatch();

deps/v8/src/execution/isolate.h

+2-4
Original file line numberDiff line numberDiff line change
@@ -822,10 +822,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
822822
// Un-schedule an exception that was caught by a TryCatch handler.
823823
void CancelScheduledExceptionFromTryCatch(v8::TryCatch* handler);
824824
void ReportPendingMessages();
825-
void ReportPendingMessagesFromJavaScript();
826-
827-
// Implements code shared between the two above methods
828-
void ReportPendingMessagesImpl(bool report_externally);
829825

830826
// Promote a scheduled exception to pending. Asserts has_scheduled_exception.
831827
Object PromoteScheduledException();
@@ -842,6 +838,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
842838

843839
Handle<JSMessageObject> CreateMessage(Handle<Object> exception,
844840
MessageLocation* location);
841+
Handle<JSMessageObject> CreateMessageOrAbort(Handle<Object> exception,
842+
MessageLocation* location);
845843

846844
// Out of resource exception helpers.
847845
Object StackOverflow();

deps/v8/src/runtime/runtime-internal.cc

+9-7
Original file line numberDiff line numberDiff line change
@@ -578,19 +578,21 @@ RUNTIME_FUNCTION(Runtime_GetTemplateObject) {
578578
isolate, native_context, description, shared_info, slot_id);
579579
}
580580

581-
RUNTIME_FUNCTION(Runtime_ReportMessage) {
581+
RUNTIME_FUNCTION(Runtime_ReportMessageFromMicrotask) {
582582
// Helper to report messages and continue JS execution. This is intended to
583-
// behave similarly to reporting exceptions which reach the top-level in
584-
// Execution.cc, but allow the JS code to continue. This is useful for
585-
// implementing algorithms such as RunMicrotasks in JS.
583+
// behave similarly to reporting exceptions which reach the top-level, but
584+
// allow the JS code to continue.
586585
HandleScope scope(isolate);
587586
DCHECK_EQ(1, args.length());
588587

589-
CONVERT_ARG_HANDLE_CHECKED(Object, message_obj, 0);
588+
CONVERT_ARG_HANDLE_CHECKED(Object, exception, 0);
590589

591590
DCHECK(!isolate->has_pending_exception());
592-
isolate->set_pending_exception(*message_obj);
593-
isolate->ReportPendingMessagesFromJavaScript();
591+
isolate->set_pending_exception(*exception);
592+
MessageLocation* no_location = nullptr;
593+
Handle<JSMessageObject> message =
594+
isolate->CreateMessageOrAbort(exception, no_location);
595+
MessageHandler::ReportMessage(isolate, no_location, message);
594596
isolate->clear_pending_exception();
595597
return ReadOnlyRoots(isolate).undefined_value();
596598
}

deps/v8/src/runtime/runtime.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ namespace internal {
225225
F(NewTypeError, 2, 1) \
226226
F(OrdinaryHasInstance, 2, 1) \
227227
F(PromoteScheduledException, 0, 1) \
228-
F(ReportMessage, 1, 1) \
228+
F(ReportMessageFromMicrotask, 1, 1) \
229229
F(ReThrow, 1, 1) \
230230
F(RunMicrotaskCallback, 2, 1) \
231231
F(PerformMicrotaskCheckpoint, 0, 1) \

deps/v8/test/cctest/test-api.cc

+20
Original file line numberDiff line numberDiff line change
@@ -19845,11 +19845,30 @@ static void MicrotaskExceptionTwo(
1984519845
v8::Exception::Error(v8_str("second")));
1984619846
}
1984719847

19848+
int handler_call_count = 0;
19849+
static void MicrotaskExceptionHandler(Local<Message> message,
19850+
Local<Value> exception) {
19851+
CHECK(exception->IsNativeError());
19852+
Local<Context> context = message->GetIsolate()->GetCurrentContext();
19853+
Local<String> str = exception->ToString(context).ToLocalChecked();
19854+
switch (handler_call_count++) {
19855+
case 0:
19856+
CHECK(str->StrictEquals(v8_str("Error: first")));
19857+
break;
19858+
case 1:
19859+
CHECK(str->StrictEquals(v8_str("Error: second")));
19860+
break;
19861+
default:
19862+
UNREACHABLE();
19863+
}
19864+
}
1984819865

1984919866
TEST(RunMicrotasksIgnoresThrownExceptions) {
1985019867
LocalContext env;
1985119868
v8::Isolate* isolate = env->GetIsolate();
1985219869
v8::HandleScope scope(isolate);
19870+
isolate->AddMessageListenerWithErrorLevel(MicrotaskExceptionHandler,
19871+
v8::Isolate::kMessageAll);
1985319872
CompileRun(
1985419873
"var exception1Calls = 0;"
1985519874
"var exception2Calls = 0;");
@@ -19860,6 +19879,7 @@ TEST(RunMicrotasksIgnoresThrownExceptions) {
1986019879
TryCatch try_catch(isolate);
1986119880
CompileRun("1+1;");
1986219881
CHECK(!try_catch.HasCaught());
19882+
CHECK_EQ(handler_call_count, 2);
1986319883
CHECK_EQ(1,
1986419884
CompileRun("exception1Calls")->Int32Value(env.local()).FromJust());
1986519885
CHECK_EQ(1,

0 commit comments

Comments
 (0)