Bug 1556033 - Properly skip frame execution if the debugger overrides it. r=jandem

The debugger allows its hooks to return explicit completion values that should
take the place of any completion value of the function beig executed. It also
handles tearing down any in-progress generators and such.

In this case, we were attempting to treat the return completion from the
debugger as if it were a real inline 'return' statement, but that attempts to
evaluate the frame and run `finally` blocks and such, which we do not want
because debugger completions are more like function termination with a result.

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

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

js/src/jit-test/tests/debug/onEnterFrame-generator-08.js

@@ -0,0 +1,17 @@
+// Bug 1556033. Test behavior of onEnterFrame "return" completion
+// values during explicit .throw() calls.
+
+let g = newGlobal({newCompartment: true});
+g.eval(`function* f(x) { }`);
+let dbg = new Debugger(g);
+
+let it = g.f();
+
+dbg.onEnterFrame = () => ({ return: "exit" });
+const result = it.throw();
+assertEq(result.value, "exit");
+assertEq(result.done, true);
+
+const result2 = it.next();
+assertEq(result2.value, undefined);
+assertEq(result2.done, true);

js/src/jit-test/tests/debug/onEnterFrame-generator-09.js

@@ -0,0 +1,17 @@
+// Bug 1556033. Test behavior of onEnterFrame "return" completion
+// values during explicit .return() calls.
+
+let g = newGlobal({newCompartment: true});
+g.eval(`function* f(x) { }`);
+let dbg = new Debugger(g);
+
+let it = g.f();
+
+dbg.onEnterFrame = () => ({ return: "exit" });
+const result = it.return();
+assertEq(result.value, "exit");
+assertEq(result.done, true);
+
+const result2 = it.next();
+assertEq(result2.value, undefined);
+assertEq(result2.done, true);

js/src/jit-test/tests/debug/onEnterFrame-generator-10.js

@@ -0,0 +1,17 @@
+// Bug 1556033. Test behavior of onEnterFrame "return" completion
+// values during explicit .next() calls.
+
+let g = newGlobal({newCompartment: true});
+g.eval(`function* f(x) { }`);
+let dbg = new Debugger(g);
+
+let it = g.f();
+
+dbg.onEnterFrame = () => ({ return: "exit" });
+const result = it.next();
+assertEq(result.value, "exit");
+assertEq(result.done, true);
+
+const result2 = it.next();
+assertEq(result2.value, undefined);
+assertEq(result2.done, true);

js/src/jit/BaselineCodeGen.cpp

@@ -6028,7 +6028,25 @@ bool BaselineInterpreterCodeGen::emitGeneratorThrowOrReturnCallVM() {
 
   using Fn = bool (*)(JSContext*, BaselineFrame*,
                       Handle<AbstractGeneratorObject*>, HandleValue, uint32_t);
-  return callVM<Fn, jit::GeneratorThrowOrReturn>();
+  if (!callVM<Fn, jit::GeneratorThrowOrReturn>()) {
+    return false;
+  }
+
+  // Control only flows out of GeneratorThrowOrReturn if the debugger
+  // overrode the function resumption with an explicit return value, so here
+  // we want to do all of the cleanup on the baseline frame that _would_ have
+  // happened inside the epilogue of the baseline frame execution.
+
+  // Save the frame's return value to return from resume.
+  masm.loadValue(frame.addressOfReturnValue(), JSReturnOperand);
+
+  // Remove the baseline frame from the stack.
+  masm.moveToStackPtr(BaselineFrameReg);
+  masm.pop(BaselineFrameReg);
+
+  masm.ret();
+
+  return true;
 }
 
 template <>

js/src/jit/VMFunctions.cpp

@@ -1025,7 +1025,10 @@ bool GeneratorThrowOrReturn(JSContext* cx, BaselineFrame* frame,
 
   GeneratorResumeKind resumeKind = GeneratorResumeKind(resumeKindArg);
   if (mustReturn) {
-    resumeKind = GeneratorResumeKind::Return;
+    // This function always returns false for the traditional JS control
+    // flow for throw, return and terminate, so we can use the true case to
+    // indicate an explicit return via the debugger.
+    return true;
   }
 
   MOZ_ALWAYS_FALSE(