Bug 1592431 - Part 3: Support .script on suspended generator frames. r=jimb

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

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

js/src/debugger/Frame.cpp

@@ -1038,7 +1038,6 @@ Result<Completion> DebuggerFrame::eval(JSContext* cx, HandleDebuggerFrame frame,
   return DebuggerGenericEval(cx, chars, bindings, options, dbg, nullptr, &iter);
 }
 
-/* static */
 bool DebuggerFrame::isOnStack() const { return !!getPrivate(); }
 
 bool DebuggerFrame::isOnStackMaybeForwarded() const {
@@ -1077,16 +1076,6 @@ void DebuggerFrame::setOnPopHandler(JSContext* cx, OnPopHandler* handler) {
   }
 }
 
-bool DebuggerFrame::requireOnStack(JSContext* cx) {
-  if (!isOnStack()) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_DEBUG_NOT_ON_STACK, "Debugger.Frame");
-    return false;
-  }
-
-  return true;
-}
-
 FrameIter::Data* DebuggerFrame::frameIterData() const {
   return static_cast<FrameIter::Data*>(getPrivate());
 }
@@ -1168,7 +1157,7 @@ void DebuggerFrame::trace(JSTracer* trc) {
 
 /* static */
 DebuggerFrame* DebuggerFrame::check(JSContext* cx, HandleValue thisv,
-                                    bool checkOnStack) {
+                                    MinState minState) {
   JSObject* thisobj = RequireObject(cx, thisv);
   if (!thisobj) {
     return nullptr;
@@ -1194,10 +1183,24 @@ DebuggerFrame* DebuggerFrame::check(JSContext* cx, HandleValue thisv,
     return nullptr;
   }
 
-  if (checkOnStack) {
-    if (!frame->requireOnStack(cx)) {
-      return nullptr;
-    }
+  switch (minState) {
+    case MinState::OnStack:
+      if (!frame->isOnStack()) {
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                  JSMSG_DEBUG_NOT_ON_STACK, "Debugger.Frame");
+        return nullptr;
+      }
+      break;
+    case MinState::OnStackOrSuspended:
+      if (!frame->isOnStack() && !frame->hasGenerator()) {
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                  JSMSG_DEBUG_NOT_ON_STACK_OR_SUSPENDED,
+                                  "Debugger.Frame");
+        return nullptr;
+      }
+      break;
+    case MinState::OnStackOrSuspendedOrTerminated:
+      break;
   }
 
   return frame;
@@ -1245,17 +1248,23 @@ bool DebuggerFrame::CallData::ToNative(JSContext* cx, unsigned argc,
                                        Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
 
-  // These methods do not require liveness.
-  bool checkOnStack = MyMethod != &CallData::liveGetter &&
-                      MyMethod != &CallData::onStackGetter &&
-                      MyMethod != &CallData::terminatedGetter &&
-                      MyMethod != &CallData::onStepGetter &&
-                      MyMethod != &CallData::onStepSetter &&
-                      MyMethod != &CallData::onPopGetter &&
-                      MyMethod != &CallData::onPopSetter;
+  MinState minState = MinState::OnStack;
+  if (MyMethod == &CallData::getScript) {
+    minState = MinState::OnStackOrSuspended;
+  } else if (
+      // These methods do not require any frame metadata.
+      MyMethod == &CallData::liveGetter ||
+      MyMethod == &CallData::onStackGetter ||
+      MyMethod == &CallData::terminatedGetter ||
+      MyMethod == &CallData::onStepGetter ||
+      MyMethod == &CallData::onStepSetter ||
+      MyMethod == &CallData::onPopGetter ||
+      MyMethod == &CallData::onPopSetter) {
+    minState = MinState::OnStackOrSuspendedOrTerminated;
+  }
 
-  RootedDebuggerFrame frame(
-      cx, DebuggerFrame::check(cx, args.thisv(), checkOnStack));
+  RootedDebuggerFrame frame(cx,
+                            DebuggerFrame::check(cx, args.thisv(), minState));
   if (!frame) {
     return false;
   }
@@ -1392,7 +1401,8 @@ static bool DebuggerArguments_getArg(JSContext* cx, unsigned argc, Value* vp) {
 
   RootedValue framev(cx, argsobj->as<NativeObject>().getReservedSlot(
                              JSSLOT_DEBUGARGUMENTS_FRAME));
-  RootedDebuggerFrame thisobj(cx, DebuggerFrame::check(cx, framev, true));
+  RootedDebuggerFrame thisobj(
+      cx, DebuggerFrame::check(cx, framev, DebuggerFrame::MinState::OnStack));
   if (!thisobj) {
     return false;
   }
@@ -1496,26 +1506,29 @@ bool DebuggerFrame::CallData::argumentsGetter() {
 }
 
 bool DebuggerFrame::CallData::getScript() {
-  FrameIter iter(*frame->frameIterData());
-  AbstractFramePtr framePtr = iter.abstractFramePtr();
-  Debugger* debug = Debugger::fromChildJSObject(frame);
-
   RootedDebuggerScript scriptObject(cx);
-  if (framePtr.isWasmDebugFrame()) {
-    RootedWasmInstanceObject instance(cx, framePtr.wasmInstance()->object());
-    scriptObject = debug->wrapWasmScript(cx, instance);
-    if (!scriptObject) {
-      return false;
+
+  Debugger* debug = Debugger::fromChildJSObject(frame);
+  if (frame->isOnStack()) {
+    FrameIter iter(*frame->frameIterData());
+    AbstractFramePtr framePtr = iter.abstractFramePtr();
+
+    if (framePtr.isWasmDebugFrame()) {
+      RootedWasmInstanceObject instance(cx, framePtr.wasmInstance()->object());
+      scriptObject = debug->wrapWasmScript(cx, instance);
+    } else {
+      RootedScript script(cx, framePtr.script());
+      scriptObject = debug->wrapScript(cx, script);
     }
   } else {
-    RootedScript script(cx, framePtr.script());
+    MOZ_ASSERT(frame->hasGenerator());
+    RootedScript script(cx, frame->generatorInfo()->generatorScript());
     scriptObject = debug->wrapScript(cx, script);
-    if (!scriptObject) {
-      return false;
-    }
+  }
+  if (!scriptObject) {
+    return false;
   }
 
-  MOZ_ASSERT(scriptObject);
   args.rval().setObject(*scriptObject);
   return true;
 }

js/src/debugger/Frame.h

@@ -182,9 +182,19 @@ class DebuggerFrame : public NativeObject {
       mozilla::Range<const char16_t> chars, HandleObject bindings,
       const EvalOptions& options);
 
-  MOZ_MUST_USE bool requireOnStack(JSContext* cx);
+  enum class MinState {
+    // The frame is guaranteed to have FrameIter::Data.
+    OnStack,
+
+    // The frame is guaranteed to have FrameIter::Data or GeneratorInfo.
+    OnStackOrSuspended,
+
+    // The frame may have FrameIter::Data, GeneratorInfo, or neither.
+    OnStackOrSuspendedOrTerminated,
+  };
+
   static MOZ_MUST_USE DebuggerFrame* check(JSContext* cx, HandleValue thisv,
-                                           bool checkOnStack);
+                                           MinState minState);
 
   bool isOnStack() const;
 

js/src/doc/Debugger/Debugger.Frame.md

@@ -235,7 +235,7 @@ instance), or `null` on frames that do not represent calls to debuggee
 code. On frames whose `callee` property is not null, this is equal to
 `callee.script`.
 
-Accessing this property will throw if `.onStack == false`.
+Accessing this property will throw if `.terminated == true`.
 
 ### `offset`
 The offset of the bytecode instruction currently being executed in

js/src/jit-test/tests/debug/Frame-script-04.js

@@ -0,0 +1,35 @@
+// Frame.prototype.script for generator frames.
+
+load(libdir + "asserts.js");
+
+var g = newGlobal({ newCompartment: true });
+var dbg = new Debugger(g);
+g.eval(`
+function* f() {}
+`);
+
+let frame;
+let script;
+dbg.onEnterFrame = function(f) {
+  frame = f;
+  script = frame.script;
+};
+
+const it = g.f();
+
+assertEq(frame instanceof Debugger.Frame, true);
+assertEq(script instanceof Debugger.Script, true);
+assertEq(frame.script, script);
+
+const lastFrame = frame;
+const lastScript = script;
+frame = null;
+script = null;
+
+it.next();
+
+assertEq(frame, lastFrame);
+assertEq(script, lastScript);
+
+// The frame has finished evaluating, so the script is no longer accessible.
+assertThrowsInstanceOf(() => frame.script, Error);

js/src/jit-test/tests/debug/Frame-script-05.js

@@ -0,0 +1,37 @@
+// Frame.prototype.script for async function frames.
+
+load(libdir + "asserts.js");
+
+var g = newGlobal({ newCompartment: true });
+var dbg = new Debugger(g);
+g.eval(`
+async function f() {
+  await Promise.resolve()
+}
+`);
+
+let frame;
+let script;
+dbg.onEnterFrame = function(f) {
+  frame = f;
+  script = frame.script;
+};
+
+const promise = g.f();
+
+assertEq(frame instanceof Debugger.Frame, true);
+assertEq(script instanceof Debugger.Script, true);
+assertEq(frame.script, script);
+
+const lastFrame = frame;
+const lastScript = script;
+frame = null;
+script = null;
+
+promise.then(() => {
+  assertEq(frame, lastFrame);
+  assertEq(script, lastScript);
+
+  // The frame has finished evaluating, so the script is no longer accessible.
+  assertThrowsInstanceOf(() => frame.script, Error);
+});

js/src/jit-test/tests/debug/Frame-script-06.js

@@ -0,0 +1,48 @@
+// Frame.prototype.script for async generator frames.
+
+load(libdir + "asserts.js");
+
+var g = newGlobal({ newCompartment: true });
+var dbg = new Debugger(g);
+g.eval(`
+async function* f() {
+  await Promise.resolve();
+}
+`);
+
+let frame;
+let script;
+dbg.onEnterFrame = function(f) {
+  frame = f;
+  script = frame.script;
+};
+
+const it = g.f();
+
+assertEq(frame instanceof Debugger.Frame, true);
+assertEq(script instanceof Debugger.Script, true);
+assertEq(frame.script, script);
+
+let lastFrame = frame;
+let lastScript = script;
+frame = null;
+script = null;
+
+let promise = it.next();
+
+assertEq(frame, lastFrame);
+assertEq(script, lastScript);
+assertEq(frame.script, script);
+
+lastFrame = frame;
+lastScript = script;
+frame = null;
+script = null;
+
+promise.then(() => {
+  assertEq(frame, lastFrame);
+  assertEq(script, lastScript);
+
+  // The frame has finished evaluating, so the script is no longer accessible.
+  assertThrowsInstanceOf(() => frame.script, Error);
+});

js/src/js.msg

@@ -494,6 +494,7 @@ MSG_DEF(JSMSG_DEBUG_NOT_DEBUGGEE,      2, JSEXN_ERR, "{0} is not a debuggee {1}"
 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_ON_STACK,      1, JSEXN_ERR, "{0} is not on stack")
+MSG_DEF(JSMSG_DEBUG_NOT_ON_STACK_OR_SUSPENDED, 1, JSEXN_ERR, "{0} is not on stack or suspended")
 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")