Bug 1556033 - Properly skip frame execution if the debugger overrides it. r=jandem
authorLogan Smyth <loganfsmyth@gmail.com>
Mon, 18 Nov 2019 01:02:31 +0000
changeset 502377 41919e4ed34d693e9bc16755957f27f95936c8a9
parent 502376 34f0f8db631a206ab5115baa73c13bcd7c2f36ea
child 502378 75035491e69f440e398617e2ccfd933eb63e8fc9
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1556033
milestone72.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 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
js/src/jit-test/tests/debug/onEnterFrame-generator-08.js
js/src/jit-test/tests/debug/onEnterFrame-generator-09.js
js/src/jit-test/tests/debug/onEnterFrame-generator-10.js
js/src/jit/BaselineCodeGen.cpp
js/src/jit/VMFunctions.cpp
new file mode 100644
--- /dev/null
+++ b/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);
new file mode 100644
--- /dev/null
+++ b/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);
new file mode 100644
--- /dev/null
+++ b/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);
--- a/js/src/jit/BaselineCodeGen.cpp
+++ b/js/src/jit/BaselineCodeGen.cpp
@@ -6023,17 +6023,35 @@ bool BaselineCodeGen<Handler>::emit_JSOP
 
 template <>
 bool BaselineInterpreterCodeGen::emitGeneratorThrowOrReturnCallVM() {
   // Record the offset for the Baseline JIT code below.
   handler.setGeneratorThrowOrReturnCallOffset(masm.currentOffset());
 
   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 <>
 bool BaselineCompilerCodeGen::emitGeneratorThrowOrReturnCallVM() {
   // Jump to the interpreter code where we call GeneratorThrowOrReturn. This way
   // we ensure we have a sane return address (into the interpreter code instead
   // of the self-hosted code's BaselineScript) because we turn the generator
   // frame into an interpreter frame in jit::GeneratorThrowOrReturn.
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -1020,17 +1020,20 @@ bool GeneratorThrowOrReturn(JSContext* c
 
   bool mustReturn = false;
   if (!DebugAfterYield(cx, frame, pc, &mustReturn)) {
     return false;
   }
 
   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(
       js::GeneratorThrowOrReturn(cx, frame, genObj, arg, resumeKind));
   return false;
 }
 
 bool GlobalNameConflictsCheckFromIon(JSContext* cx, HandleScript script) {