Bug 1776376 - Disallow setting a breakpoint for JSOp::SetAliasedVar ".generator". r=mgaudet

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

js/public/friend/ErrorNumbers.msg

@@ -521,6 +521,7 @@ MSG_DEF(JSMSG_SC_SHMEM_POLICY,         0, JSEXN_TYPEERR, "Policy object must for
 MSG_DEF(JSMSG_ASSIGN_FUNCTION_OR_NULL, 1, JSEXN_TYPEERR, "value assigned to {0} must be a function or null")
 MSG_DEF(JSMSG_DEBUG_BAD_LINE,          0, JSEXN_TYPEERR, "invalid line number")
 MSG_DEF(JSMSG_DEBUG_BAD_OFFSET,        0, JSEXN_TYPEERR, "invalid script offset")
+MSG_DEF(JSMSG_DEBUG_BREAKPOINT_NOT_ALLOWED, 0, JSEXN_ERR, "breakpoint is not allowed for this opcode")
 MSG_DEF(JSMSG_DEBUG_BAD_REFERENT,      2, JSEXN_TYPEERR, "{0} does not refer to {1}")
 MSG_DEF(JSMSG_DEBUG_BAD_RESUMPTION,    0, JSEXN_TYPEERR, "debugger resumption value must be undefined, {throw: val}, {return: val}, or null")
 MSG_DEF(JSMSG_DEBUG_RESUMPTION_CONFLICT, 0, JSEXN_TYPEERR, "debugger hook returned a resumption, but an earlier hook already did")

js/src/debugger/Script.cpp

@@ -31,6 +31,7 @@
 #include "js/Wrapper.h"               // for UncheckedUnwrap
 #include "vm/ArrayObject.h"           // for ArrayObject
 #include "vm/BytecodeUtil.h"          // for GET_JUMP_OFFSET
+#include "vm/EnvironmentObject.h"     // for EnvironmentCoordinateNameSlow
 #include "vm/GlobalObject.h"          // for GlobalObject
 #include "vm/JSContext.h"             // for JSContext, ReportValueError
 #include "vm/JSFunction.h"            // for JSFunction
@@ -588,6 +589,28 @@ static bool EnsureScriptOffsetIsValid(JSContext* cx, JSScript* script,
   return false;
 }
 
+static bool EnsureBreakpointIsAllowed(JSContext* cx, JSScript* script,
+                                      size_t offset) {
+  // Disallow breakpoint for `JSOp::SetAliasedVar` after `JSOp::Generator`.
+  // Those 2 instructions are supposed to be atomic, and nothing should happen
+  // in between them.
+  //
+  // Hitting a breakpoint there breaks the assumption around the existence of
+  // the frame's `GeneratorInfo`.
+  // (see `DebugAPI::slowPathOnNewGenerator` and `DebuggerFrame::create`)
+  jsbytecode* pc = script->offsetToPC(offset);
+  if (JSOp(*pc) == JSOp::SetAliasedVar) {
+    PropertyName* name = EnvironmentCoordinateNameSlow(script, pc);
+    if (name == cx->names().dotGenerator) {
+      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                JSMSG_DEBUG_BREAKPOINT_NOT_ALLOWED);
+      return false;
+    }
+  }
+
+  return true;
+}
+
 template <bool OnlyOffsets>
 class DebuggerScript::GetPossibleBreakpointsMatcher {
   JSContext* cx_;
@@ -1955,6 +1978,10 @@ struct DebuggerScript::SetBreakpointMatcher {
       return false;
     }
 
+    if (!EnsureBreakpointIsAllowed(cx_, script, offset_)) {
+      return false;
+    }
+
     // Ensure observability *before* setting the breakpoint. If the script is
     // not already a debuggee, trying to ensure observability after setting
     // the breakpoint (and thus marking the script as a debuggee) will skip

js/src/jit-test/tests/debug/breakpoint-dot-generator.js

@@ -0,0 +1,34 @@
+const g = newGlobal({ newCompartment: true });
+g.eval(`
+function* func() {
+}
+`);
+const d = new Debugger();
+const dg = d.addDebuggee(g);
+const script = dg.makeDebuggeeValue(g.func).script;
+
+// The following test assumes the above `func` function has the following
+// bytecode sequences:
+//
+// 00000:  Generator                       # GENERATOR
+// 00001:  SetAliasedVar ".generator"      # GENERATOR
+// 00006:  InitialYield 0                  # RVAL GENERATOR RESUMEKIND
+
+// Setting a breakpoint at `SetAliasedVar ".generator"` should be disallow.
+let caught = false;
+try {
+  script.setBreakpoint(1, {});
+} catch (e) {
+  caught = true;
+  assertEq(e.message.includes("not allowed"), true);
+}
+
+assertEq(caught, true);
+
+// Setting breakpoints to other opcodes should be allowed.
+script.setBreakpoint(0, {});
+script.setBreakpoint(6, {});
+
+// Offset 1 shouldn't be exposed.
+assertEq(script.getPossibleBreakpoints().some(p => p.offset == 1), false);
+assertEq(script.getAllColumnOffsets().some(p => p.offset == 1), false);