[JSC] Async functions and generators should properly handle broken pr…

…omises

https://bugs.webkit.org/show_bug.cgi?id=266502
<rdar://problem/119734587>

Reviewed by Justin Michaud.

Before this change, abrupt completions of PromiseResolve [1] that arised during "constructor" lookup
were not handled properly in async functions and generators, resulting in exception propagation up
the call stack rather than rejecting a promise. That affected `await`, `yield`, and `return` called
with a broken promise (i.e. with throwing "constructor" getter).

Most likely, this is a regression from implementing async / await tick reduction proposal [2].

This patch guards "constructor" lookup with exception handling, ensuring that all call sites supply
onRejected() callback that is semantically equivalent to throwing an exception at that point, as per
spec. Invoking onRejected() synchronously, without extra microtask, is also required to match the
standard, V8, and SpiderMonkey.

Also, this change implements a proposal [3] to fix AsyncGenerator.prototype.return() called on a
broken promise, aligning JSC with V8.

[1]: https://tc39.es/ecma262/#sec-promise-resolve (step 1.a)
[2]: tc39/ecma262#1250
[3]: tc39/ecma262#2683

* JSTests/stress/async-function-broken-promise.js: Added.
* JSTests/test262/expectations.yaml: Mark 4 tests as passing.
* Source/JavaScriptCore/builtins/PromiseOperations.js:
(linkTimeConstant.resolveWithoutPromiseForAsyncAwait):

Canonical link: https://commits.webkit.org/272291@main

JSTests/stress/async-function-broken-promise.js

@@ -0,0 +1,107 @@
+const log = msg => { logsActual.push(msg); };
+const logsActual = [];
+const logsExpected = `
+CAUGHT broken promise #2
+CAUGHT broken promise #4
+CAUGHT broken promise #6
+CAUGHT broken promise #8
+Promise.all() called
+tick #1
+REJECTED broken promise #1
+tick #2
+tick #3
+REJECTED broken promise #3
+tick #4
+tick #5
+REJECTED broken promise #5
+tick #6
+tick #7
+REJECTED broken promise #7
+tick #8
+Promise.all() resolved
+`.trim();
+
+function shouldBeRejected(promise) {
+  return new Promise((resolve, reject) => {
+    promise.then(reject, err => {
+      log(`REJECTED ${err.message}`);
+      resolve();
+    });
+  });
+}
+
+function makeTick(id) {
+  Promise.resolve().then(() => {
+    log(`tick #${id}`);
+  });
+}
+
+function makeBrokenPromise(id) {
+  const brokenPromise = Promise.resolve(id);
+  Object.defineProperty(brokenPromise, "constructor", {
+    get() { throw new Error(`broken promise #${id}`); },
+  });
+  return brokenPromise;
+}
+
+async function brokenPromiseInAwaitOfAsyncFunction() {
+  makeTick(1);
+  await makeBrokenPromise(1);
+}
+
+async function brokenPromiseCaughtInAwaitOfAsyncFunction() {
+  makeTick(2);
+  try { await makeBrokenPromise(2); }
+  catch (err) { log(`CAUGHT ${err.message}`); }
+}
+
+async function* brokenPromiseInAwaitOfAsyncGeneratorFunction() {
+  makeTick(3);
+  await makeBrokenPromise(3);
+}
+
+async function* brokenPromiseCaughtInAwaitOfAsyncGeneratorFunction() {
+  makeTick(4);
+  try { await makeBrokenPromise(4); }
+  catch (err) { log(`CAUGHT ${err.message}`); }
+}
+
+async function* brokenPromiseInYieldOfAsyncGeneratorFunction() {
+  makeTick(5);
+  yield makeBrokenPromise(5);
+}
+
+async function* brokenPromiseCaughtInYieldOfAsyncGeneratorFunction() {
+  makeTick(6);
+  try { yield makeBrokenPromise(6); }
+  catch (err) { log(`CAUGHT ${err.message}`); }
+}
+
+async function* brokenPromiseInReturnOfAsyncGeneratorFunction() {
+  makeTick(7);
+  return makeBrokenPromise(7);
+}
+
+async function* brokenPromiseCaughtInReturnOfAsyncGeneratorFunction() {
+  makeTick(8);
+  try { return makeBrokenPromise(8); }
+  catch (err) { log(`CAUGHT ${err.message}`); }
+}
+
+Promise.all([
+  shouldBeRejected(brokenPromiseInAwaitOfAsyncFunction()),
+  brokenPromiseCaughtInAwaitOfAsyncFunction(),
+  shouldBeRejected(brokenPromiseInAwaitOfAsyncGeneratorFunction().next()),
+  brokenPromiseCaughtInAwaitOfAsyncGeneratorFunction().next(),
+  shouldBeRejected(brokenPromiseInYieldOfAsyncGeneratorFunction().next()),
+  brokenPromiseCaughtInYieldOfAsyncGeneratorFunction().next(),
+  shouldBeRejected(brokenPromiseInReturnOfAsyncGeneratorFunction().next()),
+  brokenPromiseCaughtInReturnOfAsyncGeneratorFunction().next(),
+  log("Promise.all() called"),
+]).then(() => {
+  log("Promise.all() resolved");
+});
+
+drainMicrotasks();
+if (logsActual.join("\n") !== logsExpected)
+  throw new Error(`Bad logs!\n\n${logsActual.join("\n")}`)

JSTests/test262/expectations.yaml

@@ -7,12 +7,6 @@ test/built-ins/Array/fromAsync/this-constructor-operations.js:
 test/built-ins/Array/fromAsync/this-constructor.js:
   default: 'Test262:AsyncTestFailure:Test262Error: Test262Error: constructor is called once Expected SameValue(«2», «1») to be true'
   strict mode: 'Test262:AsyncTestFailure:Test262Error: Test262Error: constructor is called once Expected SameValue(«2», «1») to be true'
-test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:
-  default: 'Error: broken promise'
-  strict mode: 'Error: broken promise'
-test/built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js:
-  default: 'Test262:AsyncTestFailure:Error: broken promise'
-  strict mode: 'Test262:AsyncTestFailure:Error: broken promise'
 test/built-ins/Function/internals/Construct/derived-return-val-realm.js:
   default: 'Test262Error: Expected a TypeError but got a different error constructor with the same name'
   strict mode: 'Test262Error: Expected a TypeError but got a different error constructor with the same name'

Source/JavaScriptCore/builtins/PromiseOperations.js

@@ -382,7 +382,13 @@ function resolveWithoutPromiseForAsyncAwait(resolution, onFulfilled, onRejected,
     "use strict";
 
     if (@isPromise(resolution)) {
-        var constructor = resolution.constructor;
+        try {
+            var { constructor } = resolution;
+        } catch (error) {
+            onRejected(error, context);
+            return;
+        }
+
         if (constructor === @Promise || constructor === @InternalPromise)
             return @performPromiseThen(resolution, onFulfilled, onRejected, @undefined, context);
     }