Bug 1608355 - Mark all relevant zones when entering weak marking mode r=sfink

The patch moves the zone iteration out of enterWeakMaringMode() into the caller which knows which zones to mark.

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

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

js/src/gc/GC.cpp

@@ -4172,7 +4172,16 @@ void GCRuntime::markWeakReferences(gcstats::PhaseKind phase) {
 
   gcstats::AutoPhase ap1(stats(), phase);
 
-  marker.enterWeakMarkingMode();
+  if (marker.enterWeakMarkingMode()) {
+    for (ZoneIterT zone(this); !zone.done(); zone.next()) {
+      zone->enterWeakMarkingMode(&marker);
+    }
+#ifdef DEBUG
+    for (ZoneIterT zone(this); !zone.done(); zone.next()) {
+      zone->checkWeakMarkingMode();
+    }
+#endif
+  }
 
   // TODO bug 1167452: Make weak marking incremental
   drainMarkStack();

js/src/gc/GCMarker.h

@@ -304,7 +304,7 @@ class GCMarker : public JSTracer {
   // must be empty when this is called.
   void setMainStackColor(gc::MarkColor newColor);
 
-  void enterWeakMarkingMode();
+  bool enterWeakMarkingMode();
   void leaveWeakMarkingMode();
 
   // Do not use linear-time weak marking for the rest of this collection.

js/src/gc/Marking.cpp

@@ -2714,13 +2714,13 @@ void GCMarker::repush(JSObject* obj) {
   pushTaggedPtr(obj);
 }
 
-void GCMarker::enterWeakMarkingMode() {
+bool GCMarker::enterWeakMarkingMode() {
   MOZ_ASSERT(runtime()->gc.nursery().isEmpty());
 
   MOZ_ASSERT(weakMapAction() == ExpandWeakMaps);
   MOZ_ASSERT(state != MarkingState::WeakMarking);
   if (state == MarkingState::IterativeMarking) {
-    return;
+    return false;
   }
 
   // During weak marking mode, we maintain a table mapping weak keys to
@@ -2741,25 +2741,28 @@ void GCMarker::enterWeakMarkingMode() {
   while (processMarkQueue() == QueueYielded) {
   };
 
-  for (SweepGroupZonesIter zone(runtime()); !zone.done(); zone.next()) {
-    for (WeakMapBase* m : zone->gcWeakMapList()) {
-      if (m->mapColor) {
-        mozilla::Unused << m->markEntries(this);
-      }
+  return true;
+}
+
+void JS::Zone::enterWeakMarkingMode(GCMarker* marker) {
+  MOZ_ASSERT(marker->isWeakMarking());
+  for (WeakMapBase* m : gcWeakMapList()) {
+    if (m->mapColor) {
+      mozilla::Unused << m->markEntries(marker);
     }
   }
+}
 
 #ifdef DEBUG
-  for (SweepGroupZonesIter zone(runtime()); !zone.done(); zone.next()) {
-    for (auto r = zone->gcWeakKeys().all(); !r.empty(); r.popFront()) {
-      for (auto markable : r.front().value) {
-        MOZ_ASSERT(markable.weakmap->mapColor,
-                   "unmarked weakmaps in weak keys table");
-      }
+void JS::Zone::checkWeakMarkingMode() {
+  for (auto r = gcWeakKeys().all(); !r.empty(); r.popFront()) {
+    for (auto markable : r.front().value) {
+      MOZ_ASSERT(markable.weakmap->mapColor,
+                 "unmarked weakmaps in weak keys table");
     }
   }
-#endif
 }
+#endif
 
 void GCMarker::leaveWeakMarkingMode() {
   MOZ_ASSERT(state == MarkingState::WeakMarking ||

js/src/gc/Verifier.cpp

@@ -496,6 +496,9 @@ void js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session) {
   JSRuntime* runtime = gc->rt;
   GCMarker* gcmarker = &gc->marker;
 
+  MOZ_ASSERT(gc->nursery().isEmpty());
+  MOZ_ASSERT(!gcmarker->isWeakMarking());
+
   gc->waitBackgroundSweepEnd();
 
   /* Wait for off-thread parsing which can allocate. */
@@ -545,6 +548,7 @@ void js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session) {
     AutoEnterOOMUnsafeRegion oomUnsafe;
     for (gc::WeakKeyTable::Range r = zone->gcWeakKeys().all(); !r.empty();
          r.popFront()) {
+      MOZ_ASSERT(r.front().key->asTenured().zone() == zone);
       if (!savedWeakKeys.put(r.front().key, std::move(r.front().value))) {
         oomUnsafe.crash("saving weak keys table for validator");
       }
@@ -671,6 +675,9 @@ void js::gc::MarkingValidator::validate() {
     return;
   }
 
+  MOZ_ASSERT(gc->nursery().isEmpty());
+  MOZ_ASSERT(!gc->marker.isWeakMarking());
+
   gc->waitBackgroundSweepEnd();
 
   for (SweepGroupZonesIter zone(gc); !zone.done(); zone.next()) {

js/src/gc/WeakMap.h

@@ -130,6 +130,8 @@ class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
   // Whether this object has been marked during garbage collection and which
   // color it was marked.
   gc::CellColor mapColor;
+
+  friend class JS::Zone;
 };
 
 namespace detail {

js/src/gc/Zone.h

@@ -458,6 +458,11 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
   js::gc::WeakKeyTable& gcWeakKeys() { return gcWeakKeys_.ref(); }
   js::gc::WeakKeyTable& gcNurseryWeakKeys() { return gcNurseryWeakKeys_.ref(); }
 
+  // Perform all pending weakmap entry marking for this zone after
+  // transitioning to weak marking mode.
+  void enterWeakMarkingMode(js::GCMarker* marker);
+  void checkWeakMarkingMode();
+
  private:
   void sweepWeakKeysAfterMinorGC();
 

js/src/jit-test/tests/gc/bug-1608355.js

@@ -0,0 +1,7 @@
+try {
+    enableShellAllocationMetadataBuilder();
+    gczeal(11);
+    gczeal(22);
+    grayRoot() = 0;
+} catch (e) {}
+newGlobal();