Bug 1477084 - Fix assertion with Debugger forcing return from an async generator before its initial yield. r=jimb
☠☠ backed out by 1e6d3675ae4e ☠ ☠
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 04 Jan 2019 16:22:31 +0000
changeset 509658 227f1a73b16fba34216e1fb408eca5aeefcf8497
parent 509657 8e2d7fcc07f51574c9b48f3641cdae95ae387197
child 509659 dbea03a5c55e1deb19b8092c0cc13bf33af8e99c
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1477084
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
js/src/jit-test/tests/debug/bug-1477084.js
js/src/js.msg
js/src/vm/Debugger.cpp
new file mode 100644
--- /dev/null
+++ b/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);
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -482,16 +482,17 @@ MSG_DEF(JSMSG_DEBUG_NOT_DEBUGGEE,      2
 MSG_DEF(JSMSG_DEBUG_NOT_DEBUGGING,     0, JSEXN_ERR, "can't set breakpoint: script global is not a debuggee")
 MSG_DEF(JSMSG_DEBUG_NOT_IDLE,          0, JSEXN_ERR, "can't start debugging: a debuggee script is on the stack")
 MSG_DEF(JSMSG_DEBUG_NOT_LIVE,          1, JSEXN_ERR, "{0} is not live")
 MSG_DEF(JSMSG_DEBUG_NO_ENV_OBJECT,     0, JSEXN_TYPEERR, "declarative Environments don't have binding objects")
 MSG_DEF(JSMSG_DEBUG_PROTO,             2, JSEXN_TYPEERR, "{0}.prototype is not a valid {1} instance")
 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")
 MSG_DEF(JSMSG_DEBUGGEE_WOULD_RUN,      2, JSEXN_DEBUGGEEWOULDRUN, "debuggee `{0}:{1}' would run")
 MSG_DEF(JSMSG_NOT_CALLABLE_OR_UNDEFINED, 0, JSEXN_TYPEERR, "value is not a function or undefined")
 MSG_DEF(JSMSG_NOT_TRACKING_ALLOCATIONS, 1, JSEXN_ERR, "Cannot call {0} without setting trackingAllocationSites to true")
 MSG_DEF(JSMSG_OBJECT_METADATA_CALLBACK_ALREADY_SET, 0, JSEXN_ERR, "Cannot track object allocation, because other tools are already doing so")
 MSG_DEF(JSMSG_QUERY_INNERMOST_WITHOUT_LINE_URL, 0, JSEXN_TYPEERR, "findScripts query object with 'innermost' property must have 'line' and either 'displayURL', 'url', or 'source'")
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1556,68 +1556,77 @@ static bool GetThisValueForCheck(JSConte
 }
 
 static bool CheckResumptionValue(JSContext* cx, AbstractFramePtr frame,
                                  const Maybe<HandleValue>& maybeThisv,
                                  ResumeMode resumeMode, MutableHandleValue vp) {
   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);
         }
 
         vp.set(thisv);
       } else {
         ReportValueError(cx, JSMSG_BAD_DERIVED_RETURN, JSDVG_IGNORE_STACK, vp,
                          nullptr);
         return false;
       }
     }
   }
-  return true;
-}
-
-static void AdjustGeneratorResumptionValue(JSContext* cx,
-                                           AbstractFramePtr frame,
-                                           ResumeMode& resumeMode,
-                                           MutableHandleValue vp) {
+
+  // Are we forcing return from a generator?
   if (resumeMode == ResumeMode::Return && frame && frame.isFunctionFrame() &&
       frame.callee()->isGenerator()) {
-    // Treat `{return: <value>}` like a `return` statement. For generators,
-    // that means doing the work below. It's only what the debuggee would
-    // do for an ordinary `return` statement--using a few bytecode
-    // 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;
+    bool beforeInitialYield = true;
+    {
+      AutoRealm ar(cx, frame.callee());
+
+      // Treat `{return: <value>}` like a `return` statement. For generators,
+      // that means doing the work below. It's only what the debuggee would
+      // do for an ordinary `return` statement--using a few bytecode
+      // 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}`.
+        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);
         }
-        vp.setObject(*pair);
-      }
-
-      // 2.  The generator must be closed.
-      genObj->setClosed();
-    } else {
-      // We're before the initial yield. Carry on with the forced return.
-      // The debuggee will see a call to a generator returning the
-      // non-generator value *vp.
-    }
-  }
+
+        // 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) {
   JSContext* cx = ar->context();
 
   // Uncaught exceptions arise from Debugger code, and so we must already be
   // in an NX section.
   MOZ_ASSERT(EnterDebuggeeNoExecute::isLockedInStack(cx, *this));
@@ -1715,17 +1724,16 @@ ResumeMode Debugger::leaveDebugger(Maybe
     return reportUncaughtException(ar);
   }
 
   ar.reset();
   if (!cx->compartment()->wrap(cx, vp)) {
     resumeMode = ResumeMode::Terminate;
     vp.setUndefined();
   }
-  AdjustGeneratorResumptionValue(cx, frame, resumeMode, vp);
 
   return resumeMode;
 }
 
 ResumeMode Debugger::processParsedHandlerResult(Maybe<AutoRealm>& ar,
                                                 AbstractFramePtr frame,
                                                 jsbytecode* pc, bool success,
                                                 ResumeMode resumeMode,