[inspector] Fix catch prediction for promise rejection handlers.

Previously we'd predict exceptions thrown in [[Reject]] handlers as
always caught (by PromiseRejectReactionJob), but that's not what is
actually specified in ECMAScript. The PromiseRejectReactionJob will turn
any exception thrown into a promise rejection just like we do in the
case of PromiseFulfillReactionJob, and so the catch prediction should
match that behavior.

Fixed: chromium:1290861
Change-Id: Id992708b009666da7c6bf1b6e3cf30752ca0a227
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3423775
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78864}

src/builtins/builtins-definitions.h

@@ -1908,13 +1908,10 @@ namespace internal {
   V(PromiseConstructor)                              \
   V(PromiseConstructorLazyDeoptContinuation)         \
   V(PromiseFulfillReactionJob)                       \
+  V(PromiseRejectReactionJob)                        \
   V(PromiseRace)                                     \
   V(ResolvePromise)
 
-// The exception thrown in the following builtins are caught internally and will
-// not be propagated further or re-thrown
-#define BUILTIN_EXCEPTION_CAUGHT_PREDICTION_LIST(V) V(PromiseRejectReactionJob)
-
 #define IGNORE_BUILTIN(...)
 
 #define BUILTIN_LIST_C(V)                                         \

src/builtins/setup-builtins-internal.cc

@@ -354,13 +354,6 @@ void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
   BUILTIN_PROMISE_REJECTION_PREDICTION_LIST(SET_PROMISE_REJECTION_PREDICTION)
 #undef SET_PROMISE_REJECTION_PREDICTION
 
-// TODO(v8:11880): avoid roundtrips between cdc and code.
-#define SET_EXCEPTION_CAUGHT_PREDICTION(Name) \
-  FromCodeT(builtins->code(Builtin::k##Name)).set_is_exception_caught(true);
-
-  BUILTIN_EXCEPTION_CAUGHT_PREDICTION_LIST(SET_EXCEPTION_CAUGHT_PREDICTION)
-#undef SET_EXCEPTION_CAUGHT_PREDICTION
-
   builtins->MarkInitialized();
 }
 

src/objects/code-inl.h

@@ -634,21 +634,6 @@ inline void Code::set_is_promise_rejection(bool value) {
   container.set_kind_specific_flags(updated, kRelaxedStore);
 }
 
-inline bool Code::is_exception_caught() const {
-  DCHECK(kind() == CodeKind::BUILTIN);
-  int32_t flags =
-      code_data_container(kAcquireLoad).kind_specific_flags(kRelaxedLoad);
-  return IsExceptionCaughtField::decode(flags);
-}
-
-inline void Code::set_is_exception_caught(bool value) {
-  DCHECK(kind() == CodeKind::BUILTIN);
-  CodeDataContainer container = code_data_container(kAcquireLoad);
-  int32_t previous = container.kind_specific_flags(kRelaxedLoad);
-  int32_t updated = IsExceptionCaughtField::update(previous, value);
-  container.set_kind_specific_flags(updated, kRelaxedStore);
-}
-
 inline bool Code::is_off_heap_trampoline() const {
   const uint32_t flags = RELAXED_READ_UINT32_FIELD(*this, kFlagsOffset);
   return IsOffHeapTrampoline::decode(flags);
@@ -656,7 +641,6 @@ inline bool Code::is_off_heap_trampoline() const {
 
 inline HandlerTable::CatchPrediction Code::GetBuiltinCatchPrediction() {
   if (is_promise_rejection()) return HandlerTable::PROMISE;
-  if (is_exception_caught()) return HandlerTable::CAUGHT;
   return HandlerTable::UNCAUGHT;
 }
 

src/objects/code.h

@@ -478,12 +478,6 @@ class Code : public HeapObject {
   // Use GetBuiltinCatchPrediction to access this.
   inline void set_is_promise_rejection(bool flag);
 
-  // [is_exception_caught]: For kind BUILTIN tells whether the
-  // exception thrown by the code will be caught internally or
-  // uncaught if both this and is_promise_rejection is set.
-  // Use GetBuiltinCatchPrediction to access this.
-  inline void set_is_exception_caught(bool flag);
-
   // [is_off_heap_trampoline]: For kind BUILTIN tells whether
   // this is a trampoline to an off-heap builtin.
   inline bool is_off_heap_trampoline() const;
@@ -697,11 +691,10 @@ class Code : public HeapObject {
   V(EmbeddedObjectsClearedField, bool, 1, _)      \
   V(DeoptAlreadyCountedField, bool, 1, _)         \
   V(CanHaveWeakObjectsField, bool, 1, _)          \
-  V(IsPromiseRejectionField, bool, 1, _)          \
-  V(IsExceptionCaughtField, bool, 1, _)
+  V(IsPromiseRejectionField, bool, 1, _)
   DEFINE_BIT_FIELDS(CODE_KIND_SPECIFIC_FLAGS_BIT_FIELDS)
 #undef CODE_KIND_SPECIFIC_FLAGS_BIT_FIELDS
-  STATIC_ASSERT(CODE_KIND_SPECIFIC_FLAGS_BIT_FIELDS_Ranges::kBitsCount == 6);
+  STATIC_ASSERT(CODE_KIND_SPECIFIC_FLAGS_BIT_FIELDS_Ranges::kBitsCount == 5);
   STATIC_ASSERT(CODE_KIND_SPECIFIC_FLAGS_BIT_FIELDS_Ranges::kBitsCount <=
                 FIELD_SIZE(CodeDataContainer::kKindSpecificFlagsOffset) *
                     kBitsPerByte);
@@ -721,7 +714,6 @@ class Code : public HeapObject {
   inline CodeDataContainer GCSafeCodeDataContainer(AcquireLoadTag) const;
 
   bool is_promise_rejection() const;
-  bool is_exception_caught() const;
 
   enum BytecodeToPCPosition {
     kPcAtStartOfBytecode,

test/inspector/regress/regress-crbug-1290861-expected.txt

@@ -0,0 +1,4 @@
+Ensure that catch prediction is correct for [[Reject]] handlers.
+
+Running test: test
+Uncaught exception in rejectHandler

test/inspector/regress/regress-crbug-1290861.js

@@ -0,0 +1,39 @@
+// Copyright 2022 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.
+
+let {session, contextGroup, Protocol} = InspectorTest.start(
+    'Ensure that catch prediction is correct for [[Reject]] handlers.');
+
+contextGroup.addScript(`function throwInPromiseCaughtAndRethrown() {
+  var reject;
+  var promise = new Promise(function(res, rej) { reject = rej; }).catch(
+    function rejectHandler(e) {
+      throw e;
+    }
+  );
+  reject(new Error());
+  return promise;
+}`);
+
+Protocol.Debugger.onPaused(({params: {callFrames, data}}) => {
+  InspectorTest.log(`${data.uncaught ? 'Uncaught' : 'Caught'} exception in ${
+      callFrames[0].functionName}`);
+  Protocol.Debugger.resume();
+});
+
+InspectorTest.runAsyncTestSuite([async function test() {
+  await Promise.all([
+    Protocol.Runtime.enable(),
+    Protocol.Debugger.enable(),
+    Protocol.Debugger.setPauseOnExceptions({state: 'uncaught'}),
+  ]);
+  await Protocol.Runtime.evaluate({
+    awaitPromise: true,
+    expression: 'throwInPromiseCaughtAndRethrown()',
+  });
+  await Promise.all([
+    Protocol.Runtime.disable(),
+    Protocol.Debugger.disable(),
+  ]);
+}]);