Bug 1477084 - Fix assertion with Debugger forcing return from an async generator before its initial yield. r=jimb

Differential Revision: https://phabricator.services.mozilla.com/D14789

--HG--
extra : moz-landing-system : lando

js/src/jit-test/tests/debug/bug-1477084.js

@@ -0,0 +1,30 @@
+// Don't assert trying to force return before the initial yield of an async function.
+
+var g = newGlobal();
+g.parent = this;
+g.parentExc = new Error("pants");
+g.eval(`
+  var dbg = new Debugger;
+  var pw = dbg.addDebuggee(parent);
+  var hits = 0;
+  dbg.onExceptionUnwind = function (frame) {
+    dbg.onExceptionUnwind = undefined;
+    return {return: undefined};
+  };
+  dbg.uncaughtExceptionHook = exc => {
+    hits++;
+    assertEq(exc instanceof TypeError, true);
+    assertEq(/force return.*before the initial yield/.test(exc.message), true);
+    return {throw: pw.makeDebuggeeValue(parentExc)};
+  };
+`);
+
+async function* method({ x: callbackfn = unresolvableReference }) {}
+try {
+  method();
+} catch (exc) {
+  g.dbg.enabled = false;
+  assertEq(exc, g.parentExc);
+}
+
+assertEq(g.hits, 1);

js/src/js.msg

@@ -487,6 +487,7 @@ MSG_DEF(JSMSG_DEBUG_PROTO,             2, JSEXN_TYPEERR, "{0}.prototype is not a
 MSG_DEF(JSMSG_DEBUG_WRONG_OWNER,       1, JSEXN_TYPEERR, "{0} belongs to a different Debugger")
 MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT,     1, JSEXN_ERR, "variable `{0}' has been optimized out")
 MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT_FUN, 0, JSEXN_ERR, "function is optimized out")
+MSG_DEF(JSMSG_DEBUG_FORCED_RETURN_DISALLOWED, 0, JSEXN_TYPEERR, "can't force return from a generator or async function before the initial yield")
 MSG_DEF(JSMSG_DEBUG_RESUMPTION_VALUE_DISALLOWED, 0, JSEXN_TYPEERR, "resumption values are disallowed in this hook")
 MSG_DEF(JSMSG_DEBUG_VARIABLE_NOT_FOUND,0, JSEXN_TYPEERR, "variable not found in environment")
 MSG_DEF(JSMSG_DEBUG_WRAPPER_IN_WAY,    3, JSEXN_TYPEERR, "{0} is {1}{2}a global object, but a direct reference is required")

js/src/vm/Debugger.cpp

@@ -1561,6 +1561,7 @@ static bool CheckResumptionValue(JSContext* cx, AbstractFramePtr frame,
   if (maybeThisv.isSome()) {
     const HandleValue& thisv = maybeThisv.ref();
     if (resumeMode == ResumeMode::Return && vp.isPrimitive()) {
+      // Forcing return from a class constructor. There are rules.
       if (vp.isUndefined()) {
         if (thisv.isMagic(JS_UNINITIALIZED_LEXICAL)) {
           return ThrowUninitializedThis(cx, frame);
@@ -1574,45 +1575,53 @@ static bool CheckResumptionValue(JSContext* cx, AbstractFramePtr frame,
       }
     }
   }
-  return true;
-}
 
-static void AdjustGeneratorResumptionValue(JSContext* cx,
+  // Are we forcing return from a generator?
-                                           AbstractFramePtr frame,
-                                           ResumeMode& resumeMode,
-                                           MutableHandleValue vp) {
   if (resumeMode == ResumeMode::Return && frame && frame.isFunctionFrame() &&
       frame.callee()->isGenerator()) {
-    // Treat `{return: <value>}` like a `return` statement. For generators,
+    bool beforeInitialYield = true;
-    // that means doing the work below. It's only what the debuggee would
+    {
-    // do for an ordinary `return` statement--using a few bytecode
+      AutoRealm ar(cx, frame.callee());
-    // instructions--and it's simpler to do the work manually than to count
-    // on that bytecode sequence existing in the debuggee, somehow jump to
-    // it, and then avoid re-entering the debugger from it.
-    Rooted<GeneratorObject*> genObj(cx, GetGeneratorObjectForFrame(cx, frame));
-    if (genObj) {
-      // 1.  `return <value>` creates and returns a new object,
-      //     `{value: <value>, done: true}`.
-      if (!genObj->isBeforeInitialYield()) {
-        JSObject* pair = CreateIterResultObject(cx, vp, true);
-        if (!pair) {
-          // Out of memory in debuggee code. Arrange for this to propagate.
-          MOZ_ALWAYS_TRUE(cx->getPendingException(vp));
-          cx->clearPendingException();
-          resumeMode = ResumeMode::Throw;
-          return;
-        }
-        vp.setObject(*pair);
-      }
 
-      // 2.  The generator must be closed.
+      // Treat `{return: <value>}` like a `return` statement. For generators,
-      genObj->setClosed();
+      // that means doing the work below. It's only what the debuggee would
-    } else {
+      // do for an ordinary `return` statement--using a few bytecode
-      // We're before the initial yield. Carry on with the forced return.
+      // instructions--and it's simpler to do the work manually than to count
-      // The debuggee will see a call to a generator returning the
+      // on that bytecode sequence existing in the debuggee, somehow jump to
-      // non-generator value *vp.
+      // it, and then avoid re-entering the debugger from it.
+      Rooted<GeneratorObject*> genObj(cx,
+                                      GetGeneratorObjectForFrame(cx, frame));
+      if (genObj) {
+        // 1.  `return <value>` creates and returns a new object,
+        //     `{value: <value>, done: true}`.
+        beforeInitialYield = genObj->isBeforeInitialYield();
+        if (!beforeInitialYield) {
+          JSObject* pair = CreateIterResultObject(cx, vp, true);
+          if (!pair) {
+            // Out of memory in debuggee code. Arrange for this to propagate.
+            return false;
+          }
+          vp.setObject(*pair);
+        }
+
+        // 2.  The generator must be closed.
+        genObj->setClosed();
+      } else {
+        beforeInitialYield = true;
+      }
+    }
+
+    // Forcing return from a generator before the initial yield is not
+    // supported because some engine-internal code assumes a call to a
+    // generator will return a GeneratorObject; see bug 1477084.
+    if (beforeInitialYield) {
+      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                JSMSG_DEBUG_FORCED_RETURN_DISALLOWED);
+      return false;
     }
   }
+
+  return true;
 }
 
 ResumeMode Debugger::reportUncaughtException(Maybe<AutoRealm>& ar) {
@@ -1720,7 +1729,6 @@ ResumeMode Debugger::leaveDebugger(Maybe<AutoRealm>& ar, AbstractFramePtr frame,
     resumeMode = ResumeMode::Terminate;
     vp.setUndefined();
   }
-  AdjustGeneratorResumptionValue(cx, frame, resumeMode, vp);
 
   return resumeMode;
 }