Bug 1816158 - Part 1: Disallow GC while iterating global's debugger vector r=sfink

GC can mutate this vector so don't allow that while we are iterating. I think
it would be safe to use index-based iteration but it's safer to just ban it
entirely.

This fixes the crash produced by the testcase.

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

js/src/debugger/Debugger.cpp

@@ -1264,8 +1264,9 @@ bool DebugAPI::slowPathOnNewGenerator(JSContext* cx, AbstractFramePtr frame,
       MakeScopeExit([&] { Debugger::terminateDebuggerFrames(cx, frame); });
 
   bool ok = true;
+  gc::AutoSuppressGC nogc(cx);
   Debugger::forEachOnStackDebuggerFrame(
-      frame, [&](Debugger* dbg, DebuggerFrame* frameObjPtr) {
+      frame, nogc, [&](Debugger* dbg, DebuggerFrame* frameObjPtr) {
         if (!ok) {
           return;
         }
@@ -3290,7 +3291,10 @@ bool Debugger::updateExecutionObservabilityOfScripts(
 
 template <typename FrameFn>
 /* static */
-void Debugger::forEachOnStackDebuggerFrame(AbstractFramePtr frame, FrameFn fn) {
+void Debugger::forEachOnStackDebuggerFrame(AbstractFramePtr frame,
+                                           const JS::AutoRequireNoGC& nogc,
+                                           FrameFn fn) {
+  // GC is disallowed because it may mutate the vector we are iterating.
   for (Realm::DebuggerVectorEntry& entry : frame.global()->getDebuggers()) {
     Debugger* dbg = entry.dbg;
     if (FrameMap::Ptr frameEntry = dbg->frames.lookup(frame)) {
@@ -3301,13 +3305,14 @@ void Debugger::forEachOnStackDebuggerFrame(AbstractFramePtr frame, FrameFn fn) {
 
 template <typename FrameFn>
 /* static */
-void Debugger::forEachOnStackOrSuspendedDebuggerFrame(JSContext* cx,
-                                                      AbstractFramePtr frame,
+void Debugger::forEachOnStackOrSuspendedDebuggerFrame(
+    JSContext* cx, AbstractFramePtr frame, const JS::AutoRequireNoGC& nogc,
     FrameFn fn) {
   Rooted<AbstractGeneratorObject*> genObj(
       cx, frame.isGeneratorFrame() ? GetGeneratorObjectForFrame(cx, frame)
                                    : nullptr);
 
+  // GC is disallowed because it may mutate the vector we are iterating.
   for (Realm::DebuggerVectorEntry& entry : frame.global()->getDebuggers()) {
     Debugger* dbg = entry.dbg;
 
@@ -3329,7 +3334,9 @@ void Debugger::forEachOnStackOrSuspendedDebuggerFrame(JSContext* cx,
 bool Debugger::getDebuggerFrames(AbstractFramePtr frame,
                                  MutableHandle<DebuggerFrameVector> frames) {
   bool hadOOM = false;
-  forEachOnStackDebuggerFrame(frame, [&](Debugger*, DebuggerFrame* frameobj) {
+  JS::AutoAssertNoGC nogc;
+  forEachOnStackDebuggerFrame(frame, nogc,
+                              [&](Debugger*, DebuggerFrame* frameobj) {
                                 if (!hadOOM && !frames.append(frameobj)) {
                                   hadOOM = true;
                                 }
@@ -6535,8 +6542,10 @@ bool Debugger::replaceFrameGuts(JSContext* cx, AbstractFramePtr from,
 /* static */
 bool DebugAPI::inFrameMaps(AbstractFramePtr frame) {
   bool foundAny = false;
+  JS::AutoAssertNoGC nogc;
   Debugger::forEachOnStackDebuggerFrame(
-      frame, [&](Debugger*, DebuggerFrame* frameobj) { foundAny = true; });
+      frame, nogc,
+      [&](Debugger*, DebuggerFrame* frameobj) { foundAny = true; });
   return foundAny;
 }
 
@@ -6544,8 +6553,9 @@ bool DebugAPI::inFrameMaps(AbstractFramePtr frame) {
 void Debugger::suspendGeneratorDebuggerFrames(JSContext* cx,
                                               AbstractFramePtr frame) {
   JS::GCContext* gcx = cx->gcContext();
+  JS::AutoAssertNoGC nogc;
   forEachOnStackDebuggerFrame(
-      frame, [&](Debugger* dbg, DebuggerFrame* dbgFrame) {
+      frame, nogc, [&](Debugger* dbg, DebuggerFrame* dbgFrame) {
         dbg->frames.remove(frame);
 
 #if DEBUG
@@ -6564,8 +6574,9 @@ void Debugger::suspendGeneratorDebuggerFrames(JSContext* cx,
 void Debugger::terminateDebuggerFrames(JSContext* cx, AbstractFramePtr frame) {
   JS::GCContext* gcx = cx->gcContext();
 
+  JS::AutoAssertNoGC nogc;
   forEachOnStackOrSuspendedDebuggerFrame(
-      cx, frame, [&](Debugger* dbg, DebuggerFrame* dbgFrame) {
+      cx, frame, nogc, [&](Debugger* dbg, DebuggerFrame* dbgFrame) {
         Debugger::terminateDebuggerFrame(gcx, dbg, dbgFrame, frame);
       });
 

js/src/debugger/Debugger.h

@@ -937,10 +937,12 @@ class Debugger : private mozilla::LinkedListElement<Debugger> {
       IsObserving observing);
 
   template <typename FrameFn /* void (Debugger*, DebuggerFrame*) */>
-  static void forEachOnStackDebuggerFrame(AbstractFramePtr frame, FrameFn fn);
+  static void forEachOnStackDebuggerFrame(AbstractFramePtr frame,
+                                          const JS::AutoRequireNoGC& nogc,
+                                          FrameFn fn);
   template <typename FrameFn /* void (Debugger*, DebuggerFrame*) */>
-  static void forEachOnStackOrSuspendedDebuggerFrame(JSContext* cx,
-                                                     AbstractFramePtr frame,
+  static void forEachOnStackOrSuspendedDebuggerFrame(
+      JSContext* cx, AbstractFramePtr frame, const JS::AutoRequireNoGC& nogc,
       FrameFn fn);
 
   /*

js/src/gc/GC.h

@@ -204,7 +204,7 @@ static inline void MaybeVerifyBarriers(JSContext* cx, bool always = false) {}
  * This works by updating the |JSContext::suppressGC| counter which is checked
  * at the start of GC.
  */
-class MOZ_RAII JS_HAZ_GC_SUPPRESSED AutoSuppressGC {
+class MOZ_RAII JS_HAZ_GC_SUPPRESSED AutoSuppressGC : public JS::AutoRequireNoGC {
   int32_t& suppressGC_;
 
  public: