[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 <me@gus.host>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67630}

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

@@ -320,7 +320,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
   BIND(&if_exception);
   {
     // Report unhandled exceptions from microtasks.
-    CallRuntime(Runtime::kReportMessage, current_context,
+    CallRuntime(Runtime::kReportMessageFromMicrotask, current_context,
                 var_exception.value());
     RewindEnteredContext(saved_entered_context_count);
     SetCurrentContext(current_context);

src/builtins/promise-reaction-job.tq

@@ -4,11 +4,6 @@
 
 #include 'src/builtins/builtins-promise-gen.h'
 
-namespace runtime {
-  extern transitioning runtime
-  ReportMessage(implicit context: Context)(JSAny): JSAny;
-}
-
 namespace promise {
 
   transitioning
@@ -30,13 +25,8 @@ namespace promise {
         case (capability: PromiseCapability): {
           // In the general case we need to call the (user provided)
           // promiseCapability.[[Reject]] function.
-          try {
-            const reject = UnsafeCast<Callable>(capability.reject);
-            return Call(context, reject, Undefined, reason);
-          } catch (e) {
-            // Swallow the exception here.
-            return runtime::ReportMessage(e);
-          }
+          const reject = UnsafeCast<Callable>(capability.reject);
+          return Call(context, reject, Undefined, reason);
         }
       }
     } else {

src/execution/isolate.cc

@@ -1397,6 +1397,8 @@ void Isolate::InvokeApiInterruptCallbacks() {
   }
 }
 
+namespace {
+
 void ReportBootstrappingException(Handle<Object> exception,
                                   MessageLocation* location) {
   base::OS::PrintError("Exception thrown during bootstrapping\n");
@@ -1452,6 +1454,36 @@ void ReportBootstrappingException(Handle<Object> exception,
 #endif
 }
 
+}  // anonymous namespace
+
+Handle<JSMessageObject> Isolate::CreateMessageOrAbort(
+    Handle<Object> exception, MessageLocation* location) {
+  Handle<JSMessageObject> message_obj = CreateMessage(exception, location);
+
+  // If the abort-on-uncaught-exception flag is specified, and if the
+  // embedder didn't specify a custom uncaught exception callback,
+  // or if the custom callback determined that V8 should abort, then
+  // abort.
+  if (FLAG_abort_on_uncaught_exception) {
+    CatchType prediction = PredictExceptionCatcher();
+    if ((prediction == NOT_CAUGHT || prediction == CAUGHT_BY_EXTERNAL) &&
+        (!abort_on_uncaught_exception_callback_ ||
+         abort_on_uncaught_exception_callback_(
+             reinterpret_cast<v8::Isolate*>(this)))) {
+      // Prevent endless recursion.
+      FLAG_abort_on_uncaught_exception = false;
+      // This flag is intended for use by JavaScript developers, so
+      // print a user-friendly stack trace (not an internal one).
+      PrintF(stderr, "%s\n\nFROM\n",
+             MessageHandler::GetLocalizedMessage(this, message_obj).get());
+      PrintCurrentStackTrace(stderr);
+      base::OS::Abort();
+    }
+  }
+
+  return message_obj;
+}
+
 Object Isolate::Throw(Object raw_exception, MessageLocation* location) {
   DCHECK(!has_pending_exception());
 
@@ -1523,38 +1555,14 @@ Object Isolate::Throw(Object raw_exception, MessageLocation* location) {
     if (location == nullptr && ComputeLocation(&computed_location)) {
       location = &computed_location;
     }
-
     if (bootstrapper()->IsActive()) {
       // It's not safe to try to make message objects or collect stack traces
       // while the bootstrapper is active since the infrastructure may not have
       // been properly initialized.
       ReportBootstrappingException(exception, location);
     } else {
-      Handle<Object> message_obj = CreateMessage(exception, location);
+      Handle<Object> message_obj = CreateMessageOrAbort(exception, location);
       thread_local_top()->pending_message_obj_ = *message_obj;
-
-      // For any exception not caught by JavaScript, even when an external
-      // handler is present:
-      // If the abort-on-uncaught-exception flag is specified, and if the
-      // embedder didn't specify a custom uncaught exception callback,
-      // or if the custom callback determined that V8 should abort, then
-      // abort.
-      if (FLAG_abort_on_uncaught_exception) {
-        CatchType prediction = PredictExceptionCatcher();
-        if ((prediction == NOT_CAUGHT || prediction == CAUGHT_BY_EXTERNAL) &&
-            (!abort_on_uncaught_exception_callback_ ||
-             abort_on_uncaught_exception_callback_(
-                 reinterpret_cast<v8::Isolate*>(this)))) {
-          // Prevent endless recursion.
-          FLAG_abort_on_uncaught_exception = false;
-          // This flag is intended for use by JavaScript developers, so
-          // print a user-friendly stack trace (not an internal one).
-          PrintF(stderr, "%s\n\nFROM\n",
-                 MessageHandler::GetLocalizedMessage(this, message_obj).get());
-          PrintCurrentStackTrace(stderr);
-          base::OS::Abort();
-        }
-      }
     }
   }
 
@@ -2085,7 +2093,7 @@ bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
       bool is_at_number_conversion =
           elements->IsAsmJsWasmFrame(i) &&
           elements->Flags(i).value() & FrameArray::kAsmJsAtNumberConversion;
-      if (elements->IsWasmCompiledFrame(i)) {
+      if (elements->IsWasmCompiledFrame(i) || elements->IsAsmJsWasmFrame(i)) {
         // WasmCode* held alive by the {GlobalWasmCodeRef}.
         wasm::WasmCode* code =
             Managed<wasm::GlobalWasmCodeRef>::cast(elements->WasmCodeObject(i))
@@ -2208,9 +2216,28 @@ bool Isolate::IsExternalHandlerOnTop(Object exception) {
   return (entry_handler > external_handler);
 }
 
-void Isolate::ReportPendingMessagesImpl(bool report_externally) {
+std::vector<MemoryRange>* Isolate::GetCodePages() const {
+  return code_pages_.load(std::memory_order_acquire);
+}
+
+void Isolate::SetCodePages(std::vector<MemoryRange>* new_code_pages) {
+  code_pages_.store(new_code_pages, std::memory_order_release);
+}
+
+void Isolate::ReportPendingMessages() {
+  DCHECK(AllowExceptions::IsAllowed(this));
+
+  // The embedder might run script in response to an exception.
+  AllowJavascriptExecutionDebugOnly allow_script(this);
+
   Object exception_obj = pending_exception();
 
+  // Try to propagate the exception to an external v8::TryCatch handler. If
+  // propagation was unsuccessful, then we will get another chance at reporting
+  // the pending message if the exception is re-thrown.
+  bool has_been_propagated = PropagatePendingExceptionToExternalTryCatch();
+  if (!has_been_propagated) return;
+
   // Clear the pending message object early to avoid endless recursion.
   Object message_obj = thread_local_top()->pending_message_obj_;
   clear_pending_message();
@@ -2223,7 +2250,7 @@ void Isolate::ReportPendingMessagesImpl(bool report_externally) {
   // depending on whether and external v8::TryCatch or an internal JavaScript
   // handler is on top.
   bool should_report_exception;
-  if (report_externally) {
+  if (IsExternalHandlerOnTop(exception_obj)) {
     // Only report the exception if the external handler is verbose.
     should_report_exception = try_catch_handler()->is_verbose_;
   } else {
@@ -2249,93 +2276,6 @@ void Isolate::ReportPendingMessagesImpl(bool report_externally) {
   }
 }
 
-std::vector<MemoryRange>* Isolate::GetCodePages() const {
-  return code_pages_.load(std::memory_order_acquire);
-}
-
-void Isolate::SetCodePages(std::vector<MemoryRange>* new_code_pages) {
-  code_pages_.store(new_code_pages, std::memory_order_release);
-}
-
-void Isolate::ReportPendingMessages() {
-  DCHECK(AllowExceptions::IsAllowed(this));
-
-  // The embedder might run script in response to an exception.
-  AllowJavascriptExecutionDebugOnly allow_script(this);
-
-  Object exception = pending_exception();
-
-  // Try to propagate the exception to an external v8::TryCatch handler. If
-  // propagation was unsuccessful, then we will get another chance at reporting
-  // the pending message if the exception is re-thrown.
-  bool has_been_propagated = PropagatePendingExceptionToExternalTryCatch();
-  if (!has_been_propagated) return;
-
-  ReportPendingMessagesImpl(IsExternalHandlerOnTop(exception));
-}
-
-void Isolate::ReportPendingMessagesFromJavaScript() {
-  DCHECK(AllowExceptions::IsAllowed(this));
-
-  auto IsHandledByJavaScript = [=]() {
-    // In this situation, the exception is always a non-terminating exception.
-
-    // Get the top-most JS_ENTRY handler, cannot be on top if it doesn't exist.
-    Address entry_handler = Isolate::handler(thread_local_top());
-    DCHECK_NE(entry_handler, kNullAddress);
-    entry_handler = StackHandler::FromAddress(entry_handler)->next_address();
-
-    // Get the address of the external handler so we can compare the address to
-    // determine which one is closer to the top of the stack.
-    Address external_handler = thread_local_top()->try_catch_handler_address();
-    if (external_handler == kNullAddress) return true;
-
-    return (entry_handler < external_handler);
-  };
-
-  auto IsHandledExternally = [=]() {
-    Address external_handler = thread_local_top()->try_catch_handler_address();
-    if (external_handler == kNullAddress) return false;
-
-    // Get the top-most JS_ENTRY handler, cannot be on top if it doesn't exist.
-    Address entry_handler = Isolate::handler(thread_local_top());
-    DCHECK_NE(entry_handler, kNullAddress);
-    entry_handler = StackHandler::FromAddress(entry_handler)->next_address();
-    return (entry_handler > external_handler);
-  };
-
-  auto PropagateToExternalHandler = [=]() {
-    if (IsHandledByJavaScript()) {
-      thread_local_top()->external_caught_exception_ = false;
-      return false;
-    }
-
-    if (!IsHandledExternally()) {
-      thread_local_top()->external_caught_exception_ = false;
-      return true;
-    }
-
-    thread_local_top()->external_caught_exception_ = true;
-    v8::TryCatch* handler = try_catch_handler();
-    DCHECK(thread_local_top()->pending_message_obj_.IsJSMessageObject() ||
-           thread_local_top()->pending_message_obj_.IsTheHole(this));
-    handler->can_continue_ = true;
-    handler->has_terminated_ = false;
-    handler->exception_ = reinterpret_cast<void*>(pending_exception().ptr());
-    // Propagate to the external try-catch only if we got an actual message.
-    if (thread_local_top()->pending_message_obj_.IsTheHole(this)) return true;
-
-    handler->message_obj_ =
-        reinterpret_cast<void*>(thread_local_top()->pending_message_obj_.ptr());
-    return true;
-  };
-
-  // Try to propagate to an external v8::TryCatch handler.
-  if (!PropagateToExternalHandler()) return;
-
-  ReportPendingMessagesImpl(true);
-}
-
 bool Isolate::OptionalRescheduleException(bool clear_exception) {
   DCHECK(has_pending_exception());
   PropagatePendingExceptionToExternalTryCatch();

src/execution/isolate.h

@@ -825,10 +825,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   // Un-schedule an exception that was caught by a TryCatch handler.
   void CancelScheduledExceptionFromTryCatch(v8::TryCatch* handler);
   void ReportPendingMessages();
-  void ReportPendingMessagesFromJavaScript();
-
-  // Implements code shared between the two above methods
-  void ReportPendingMessagesImpl(bool report_externally);
 
   // Promote a scheduled exception to pending. Asserts has_scheduled_exception.
   Object PromoteScheduledException();
@@ -845,6 +841,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   Handle<JSMessageObject> CreateMessage(Handle<Object> exception,
                                         MessageLocation* location);
+  Handle<JSMessageObject> CreateMessageOrAbort(Handle<Object> exception,
+                                               MessageLocation* location);
 
   // Out of resource exception helpers.
   Object StackOverflow();

src/runtime/runtime-internal.cc

@@ -598,19 +598,21 @@ RUNTIME_FUNCTION(Runtime_GetTemplateObject) {
       isolate, native_context, description, shared_info, slot_id);
 }
 
-RUNTIME_FUNCTION(Runtime_ReportMessage) {
+RUNTIME_FUNCTION(Runtime_ReportMessageFromMicrotask) {
   // Helper to report messages and continue JS execution. This is intended to
-  // behave similarly to reporting exceptions which reach the top-level in
-  // Execution.cc, but allow the JS code to continue. This is useful for
-  // implementing algorithms such as RunMicrotasks in JS.
+  // behave similarly to reporting exceptions which reach the top-level, but
+  // allow the JS code to continue.
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
 
-  CONVERT_ARG_HANDLE_CHECKED(Object, message_obj, 0);
+  CONVERT_ARG_HANDLE_CHECKED(Object, exception, 0);
 
   DCHECK(!isolate->has_pending_exception());
-  isolate->set_pending_exception(*message_obj);
-  isolate->ReportPendingMessagesFromJavaScript();
+  isolate->set_pending_exception(*exception);
+  MessageLocation* no_location = nullptr;
+  Handle<JSMessageObject> message =
+      isolate->CreateMessageOrAbort(exception, no_location);
+  MessageHandler::ReportMessage(isolate, no_location, message);
   isolate->clear_pending_exception();
   return ReadOnlyRoots(isolate).undefined_value();
 }

src/runtime/runtime.h

@@ -225,7 +225,7 @@ namespace internal {
   F(NewTypeError, -1 /* [1, 4] */, 1)                \
   F(OrdinaryHasInstance, 2, 1)                       \
   F(PromoteScheduledException, 0, 1)                 \
-  F(ReportMessage, 1, 1)                             \
+  F(ReportMessageFromMicrotask, 1, 1)                \
   F(ReThrow, 1, 1)                                   \
   F(RunMicrotaskCallback, 2, 1)                      \
   F(PerformMicrotaskCheckpoint, 0, 1)                \

test/cctest/test-api.cc

@@ -19829,11 +19829,30 @@ static void MicrotaskExceptionTwo(
       v8::Exception::Error(v8_str("second")));
 }
 
+int handler_call_count = 0;
+static void MicrotaskExceptionHandler(Local<Message> message,
+                                      Local<Value> exception) {
+  CHECK(exception->IsNativeError());
+  Local<Context> context = message->GetIsolate()->GetCurrentContext();
+  Local<String> str = exception->ToString(context).ToLocalChecked();
+  switch (handler_call_count++) {
+    case 0:
+      CHECK(str->StrictEquals(v8_str("Error: first")));
+      break;
+    case 1:
+      CHECK(str->StrictEquals(v8_str("Error: second")));
+      break;
+    default:
+      UNREACHABLE();
+  }
+}
 
 TEST(RunMicrotasksIgnoresThrownExceptions) {
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
   v8::HandleScope scope(isolate);
+  isolate->AddMessageListenerWithErrorLevel(MicrotaskExceptionHandler,
+                                            v8::Isolate::kMessageAll);
   CompileRun(
       "var exception1Calls = 0;"
       "var exception2Calls = 0;");
@@ -19844,6 +19863,7 @@ TEST(RunMicrotasksIgnoresThrownExceptions) {
   TryCatch try_catch(isolate);
   CompileRun("1+1;");
   CHECK(!try_catch.HasCaught());
+  CHECK_EQ(handler_call_count, 2);
   CHECK_EQ(1,
            CompileRun("exception1Calls")->Int32Value(env.local()).FromJust());
   CHECK_EQ(1,