[promisehook] Check for JSReceiver in runtime function

PromiseHooks can be called with a proxy which fails the cast and
crashes. This patch changes the runtime functions to
explicitly check for a JSPromise.

This has the side effect of removing the existing broken support for
catch prediction for non native promises.

Bug: v8:7398, v8:7190
Change-Id: I66dbe5f9935943a91afb7ee14919bd9248f9f7e4
Reviewed-on: https://chromium-review.googlesource.com/907677Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51182}

src/runtime/runtime-promise.cc

@@ -136,7 +136,9 @@ RUNTIME_FUNCTION(Runtime_PromiseHookInit) {
 RUNTIME_FUNCTION(Runtime_PromiseHookResolve) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, maybe_promise, 0);
+  if (!maybe_promise->IsJSPromise()) return isolate->heap()->undefined_value();
+  Handle<JSPromise> promise = Handle<JSPromise>::cast(maybe_promise);
   isolate->RunPromiseHook(PromiseHookType::kResolve, promise,
                           isolate->factory()->undefined_value());
   return isolate->heap()->undefined_value();
@@ -145,11 +147,12 @@ RUNTIME_FUNCTION(Runtime_PromiseHookResolve) {
 RUNTIME_FUNCTION(Runtime_PromiseHookBefore) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(JSObject, promise, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, maybe_promise, 0);
+  if (!maybe_promise->IsJSPromise()) return isolate->heap()->undefined_value();
+  Handle<JSPromise> promise = Handle<JSPromise>::cast(maybe_promise);
   if (isolate->debug()->is_active()) isolate->PushPromise(promise);
   if (promise->IsJSPromise()) {
-    isolate->RunPromiseHook(PromiseHookType::kBefore,
-                            Handle<JSPromise>::cast(promise),
+    isolate->RunPromiseHook(PromiseHookType::kBefore, promise,
                             isolate->factory()->undefined_value());
   }
   return isolate->heap()->undefined_value();
@@ -158,11 +161,12 @@ RUNTIME_FUNCTION(Runtime_PromiseHookBefore) {
 RUNTIME_FUNCTION(Runtime_PromiseHookAfter) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(JSObject, promise, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, maybe_promise, 0);
+  if (!maybe_promise->IsJSPromise()) return isolate->heap()->undefined_value();
+  Handle<JSPromise> promise = Handle<JSPromise>::cast(maybe_promise);
   if (isolate->debug()->is_active()) isolate->PopPromise();
   if (promise->IsJSPromise()) {
-    isolate->RunPromiseHook(PromiseHookType::kAfter,
-                            Handle<JSPromise>::cast(promise),
+    isolate->RunPromiseHook(PromiseHookType::kAfter, promise,
                             isolate->factory()->undefined_value());
   }
   return isolate->heap()->undefined_value();

test/cctest/test-api.cc

@@ -17963,6 +17963,21 @@ TEST(PromiseHook) {
   // 6) resolve hook (p1)
   CHECK_EQ(6, promise_hook_data->promise_hook_count);
 
+  promise_hook_data->Reset();
+  source =
+      "class X extends Promise {\n"
+      "  static get [Symbol.species]() {\n"
+      "    return Y;\n"
+      "  }\n"
+      "}\n"
+      "class Y {\n"
+      "  constructor(executor) {\n"
+      "    return new Proxy(new Promise(executor), {});\n"
+      "  }\n"
+      "}\n"
+      "var x = X.resolve().then(() => {});\n";
+
+  CompileRun(source);
   delete promise_hook_data;
   isolate->SetPromiseHook(nullptr);
 }

test/debugger/debug/es6/debug-promises/proxy-as-promise.js

+// Copyright 2018 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.
+
+
+// Test debug events when we listen to all exceptions and
+// there is a catch handler for the exception thrown in a Promise.
+// We expect a normal Exception debug event to be triggered.
+
+Debug = debug.Debug;
+
+var expected_events = 1;
+var log = [];
+
+
+class P extends Promise {
+    constructor(...args) {
+        super(...args);
+        return new Proxy(this, {
+            get(target, property, receiver) {
+                if (property in target) {
+                    return Reflect.get(target, property, receiver);
+                } else {
+                    return (...args) =>
+                        new Promise((resolve, reject) =>
+                            target.then(v => resolve(v[property](...args)))
+                                .catch(reject)
+                        );
+                }
+            }
+        });
+    }
+}
+
+P.resolve({doStuff(){log.push(1)}}).doStuff()
+
+function listener(event, exec_state, event_data, data) {}
+
+Debug.setBreakOnUncaughtException();
+Debug.setListener(listener);
+
+%RunMicrotasks();

test/debugger/debug/es6/debug-promises/throw-with-throw-in-reject.js

@@ -3,15 +3,13 @@
 // found in the LICENSE file.
 
 
-// Test debug events when an exception is thrown inside a Promise, which is
-// caught by a custom promise, which throws a new exception in its reject
-// handler. We expect two Exception debug events:
-//  1) when the exception is thrown in the promise q.
-//  2) when the custom reject closure in MyPromise throws an exception.
+// Test debug events when an exception is thrown inside a Promise,
+// which is caught by a custom promise, which throws a new exception
+// in its reject handler. We expect no Exception debug events.
 
 Debug = debug.Debug;
 
-var expected_events = 1;
+var expected_events = 0;
 var log = [];
 
 var p = new Promise(function(resolve, reject) {
@@ -21,11 +19,9 @@ var p = new Promise(function(resolve, reject) {
 
 function MyPromise(resolver) {
   var reject = function() {
-    log.push("throw in reject");
     throw new Error("reject");  // event
   };
   var resolve = function() { };
-  log.push("construct");
   resolver(resolve, reject);
 };
 
@@ -42,16 +38,7 @@ var q = p.then(
 function listener(event, exec_state, event_data, data) {
   try {
     if (event == Debug.DebugEvent.Exception) {
-      expected_events--;
-      assertTrue(expected_events >= 0);
-      if (expected_events == 0) {
-        assertEquals(["resolve", "construct", "end main",
-                      "throw caught"], log);
-        assertEquals("caught", event_data.exception().message);
-      } else {
-        assertUnreachable();
-      }
-      assertTrue(exec_state.frame(0).sourceLineText().indexOf('// event') > 0);
+      assertUnreachable();
     }
   } catch (e) {
     %AbortJS(e + "\n" + e.stack);
@@ -68,8 +55,8 @@ function testDone(iteration) {
     try {
       assertTrue(iteration < 10);
       if (expected_events === 0) {
-        assertEquals(["resolve", "construct", "end main",
-                      "throw caught", "throw in reject"], log);
+        assertEquals(["resolve", "end main",
+                      "throw caught"], log);
       } else {
         testDone(iteration + 1);
       }