[debug] Default to UNCAUGHT in catch prediction

V8's catch prediction mechanism tries to predict whether a thrown
exception will be caught, just by looking at the current call stack.

At the time when catch prediction was first introduced, only a few
builtins (mostly related to Promise and Generator) could end up being
fed into the catch prediction mechanism. This is no longer the case now
that builtins are used in new ways, e.g. Array.p.forEach's continuation
builtins.

This CL removes the need to explicitly mark all builtins visible to the
StackFrameIterator as CAUGHT/UNCAUGHT/PROMISE, and instead defaults to
treating unmarked builtins as UNCAUGHT.

BUG=v8:6536

Change-Id: Ibdc106a91b2b0ffb93099433077642cad02c71e2
Reviewed-on: https://chromium-review.googlesource.com/555518
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46357}

src/builtins/builtins-definitions.h

@@ -1101,20 +1101,10 @@ namespace internal {
   V(ResolveNativePromise)                            \
   V(ResolvePromise)
 
-// The exception thrown in the following builtins are caught
-// internally and should not trigger the catch prediction heuristic.
+// 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(PromiseHandleReject)
 
-// The exception thrown in the following builtins are not caught
-// internally and should trigger the catch prediction heuristic.
-#define BUILTIN_EXCEPTION_UNCAUGHT_PREDICTION_LIST(V) \
-  V(ArrayForEachLoopLazyDeoptContinuation)            \
-  V(MapConstructor)                                   \
-  V(SetConstructor)                                   \
-  V(GeneratorPrototypeNext)                           \
-  V(GeneratorPrototypeReturn)                         \
-  V(GeneratorPrototypeThrow)
-
 #define IGNORE_BUILTIN(...)
 
 #define BUILTIN_LIST_ALL(V) BUILTIN_LIST(V, V, V, V, V, V, V, V)

src/builtins/setup-builtins-internal.cc

@@ -191,7 +191,6 @@ void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
       ->set_is_promise_rejection(true);
 
   BUILTIN_PROMISE_REJECTION_PREDICTION_LIST(SET_PROMISE_REJECTION_PREDICTION)
-  BUILTIN_EXCEPTION_UNCAUGHT_PREDICTION_LIST(SET_PROMISE_REJECTION_PREDICTION)
 #undef SET_PROMISE_REJECTION_PREDICTION
 
 #define SET_EXCEPTION_CAUGHT_PREDICTION(Name)        \
@@ -199,7 +198,6 @@ void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
       ->set_is_exception_caught(true);
 
   BUILTIN_EXCEPTION_CAUGHT_PREDICTION_LIST(SET_EXCEPTION_CAUGHT_PREDICTION)
-  BUILTIN_EXCEPTION_UNCAUGHT_PREDICTION_LIST(SET_EXCEPTION_CAUGHT_PREDICTION)
 #undef SET_EXCEPTION_CAUGHT_PREDICTION
 
 #define SET_CODE_NON_TAGGED_PARAMS(Name)             \

src/isolate.cc

@@ -1454,12 +1454,13 @@ Isolate::CatchType Isolate::PredictExceptionCatcher() {
 
       case StackFrame::STUB: {
         Handle<Code> code(frame->LookupCode());
-        if (code->kind() == Code::BUILTIN && code->is_turbofanned() &&
-            code->handler_table()->length()) {
-          CatchType prediction = ToCatchType(code->GetBuiltinCatchPrediction());
-          if (prediction == NOT_CAUGHT) break;
-          return prediction;
+        if (!code->IsCode() || code->kind() != Code::BUILTIN ||
+            !code->handler_table()->length() || !code->is_turbofanned()) {
+          break;
         }
+
+        CatchType prediction = ToCatchType(code->GetBuiltinCatchPrediction());
+        if (prediction != NOT_CAUGHT) return prediction;
       } break;
 
       default:

src/objects-inl.h

@@ -3899,23 +3899,9 @@ inline void Code::set_is_exception_caught(bool value) {
 }
 
 inline HandlerTable::CatchPrediction Code::GetBuiltinCatchPrediction() {
-  // An exception is uncaught if both is_promise_rejection and
-  // is_exception_caught bits are set.
-  if (is_promise_rejection() && is_exception_caught()) {
+  if (is_promise_rejection()) return HandlerTable::PROMISE;
+  if (is_exception_caught()) return HandlerTable::CAUGHT;
   return HandlerTable::UNCAUGHT;
-  }
-
-  if (is_promise_rejection()) {
-    return HandlerTable::PROMISE;
-  }
-
-  // This the exception throw in PromiseHandle which doesn't
-  // cause a promise rejection.
-  if (is_exception_caught()) {
-    return HandlerTable::CAUGHT;
-  }
-
-  UNREACHABLE();
 }
 
 

test/cctest/test-debug.cc

@@ -6667,9 +6667,7 @@ TEST(BuiltinsExceptionPrediction) {
   for (int i = 0; i < i::Builtins::builtin_count; i++) {
     Code* builtin = builtins->builtin(static_cast<i::Builtins::Name>(i));
 
-    if (i::HandlerTable::cast(builtin->handler_table())->length() == 0)
-      continue;
-
+    if (builtin->kind() != Code::BUILTIN) continue;
     if (whitelist.find(i) != whitelist.end()) continue;
 
     auto prediction = builtin->GetBuiltinCatchPrediction();