tor-browser

commit 32ae96f0b44e91670cc8da828f5ca71815c2b8ad
parent d24adae695d5634debc7e073627be3ace5924f20
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date:   Fri,  7 Nov 2025 16:31:16 +0000

Bug 1997896 - Update source zone atom marking bitmaps when gray unmarking regardless of the state of the target zone r=sfink

This fixes a problem with gray unmarking where we return early based on
|zone|'s GC state, potentially skipping updating |sourceZone|'s atom marking
bitmap. The problem is that we still need to do this regardless of the target
zone's state.

I was also able to make a testcase that reproduced gray marking failures which
is fixed by this patch.

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

Diffstat:
M
js/src/gc/Marking.cpp
 | 
19
++++++++++---------
A
js/src/jit-test/tests/gc/bug-1997896.js
 | 
52
++++++++++++++++++++++++++++++++++++++++++++++++++++

2 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp
@@ -2905,6 +2905,16 @@ void UnmarkGrayTracer::onChild(JS::GCCellPtr thing, const char* name) {
   TenuredCell& tenured = cell->asTenured();
   Zone* zone = tenured.zoneFromAnyThread();
 
+  // As well as updating the mark bits, we may need to update the color in the
+  // atom marking bitmap to record that |sourceZone| now has a black edge to
+  // |thing|.
+  if (zone->isAtomsZone() && sourceZone) {
+    MOZ_ASSERT(tenured.is<JS::Symbol>());
+    GCRuntime* gc = &runtime()->gc;
+    JS::Symbol* symbol = tenured.as<JS::Symbol>();
+    gc->atomMarking.maybeUnmarkGrayAtomically(sourceZone, symbol);
+  }
+
   // If the cell is in a zone whose mark bits are being cleared, then it will
   // end up being marked black by GC marking.
   if (zone->isGCPreparing()) {
@@ -2932,15 +2942,6 @@ void UnmarkGrayTracer::onChild(JS::GCCellPtr thing, const char* name) {
     }
   }
 
-  // As well as updating the mark bits, we may need to update the color in the
-  // atom marking bitmap to record that |zone| now has a black edge to |thing|.
-  if (zone->isAtomsZone() && sourceZone) {
-    MOZ_ASSERT(tenured.is<JS::Symbol>());
-    GCRuntime* gc = &runtime()->gc;
-    JS::Symbol* symbol = tenured.as<JS::Symbol>();
-    gc->atomMarking.maybeUnmarkGrayAtomically(sourceZone, symbol);
-  }
-
   unmarkedAny = true;
 }
 
diff --git a/js/src/jit-test/tests/gc/bug-1997896.js b/js/src/jit-test/tests/gc/bug-1997896.js
@@ -0,0 +1,52 @@
+function checkMarks(expected) {
+  assertEq(getMarks().join(", "), expected.join(", "));
+}
+
+gczeal('CheckGrayMarking');
+
+let g1 = newGlobal({newCompartment: true});
+let g2 = newGlobal({newCompartment: true});
+let makeAndReturnObject = `
+  var o = {};
+  addMarkObservers([o]);
+  grayRoot()[0] = o;
+  o;
+`;
+let o1 = g1.eval(makeAndReturnObject);
+let o2 = g2.eval(makeAndReturnObject);
+let s = Symbol();
+addMarkObservers([s]);
+let i = getAtomMarkIndex(s);
+o1.s = s;
+o2.s = s;
+o1 = undefined;
+o2 = undefined;
+s = undefined;
+g1.o = undefined;
+g2.o = undefined;
+
+gc();
+checkMarks(['gray', 'gray', 'gray']);
+assertEq(getAtomMarkColor(g1, i), 'gray');
+assertEq(getAtomMarkColor(g2, i), 'gray');
+
+o1 = g1.eval('grayRoot()[0]');
+checkMarks(['black', 'gray', 'black']);
+assertEq(getAtomMarkColor(g1, i), 'black');
+assertEq(getAtomMarkColor(g2, i), 'gray');
+
+o2 = g2.eval('grayRoot()[0]');
+checkMarks(['black', 'black', 'black']);
+assertEq(getAtomMarkColor(g1, i), 'black');
+assertEq(getAtomMarkColor(g2, i), 'black');
+
+o1 = undefined;
+schedulezone(this);
+schedulezone(g1);
+schedulezone('atoms');
+startgc();
+assertEq(gcstate(), 'NotActive');
+
+checkMarks(['gray', 'black', 'black']);
+assertEq(getAtomMarkColor(g1, i), 'gray');
+assertEq(getAtomMarkColor(g2, i), 'black');