commit 98bcabadc0b781dbb6be2c0171c1951c3b1d34e0
parent 9c37b4ea73a7f757aad3cd27b48c0517d5ad7f4b
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date: Sat, 1 Nov 2025 11:58:11 +0000
Bug 1994871 - Part 4: Add tests for gray symbols r=sfink
Differential Revision: https://phabricator.services.mozilla.com/D269956
Diffstat:
5 files changed, 172 insertions(+), 32 deletions(-)
diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp
@@ -148,6 +148,7 @@
#include "wasm/WasmValue.h"
#include "debugger/DebugAPI-inl.h"
+#include "gc/AtomMarking-inl.h"
#include "jit/JitHints-inl.h"
#include "vm/Compartment-inl.h"
#include "vm/EnvironmentObject-inl.h"
@@ -3206,6 +3207,73 @@ static bool IsAtomMarked(JSContext* cx, unsigned argc, Value* vp) {
return true;
}
+static bool GetAtomMarkIndex(JSContext* cx, unsigned argc, Value* vp) {
+ CallArgs args = CallArgsFromVp(argc, vp);
+ RootedObject callee(cx, &args.callee());
+
+ if (args.length() != 1 || !args[0].isGCThing() ||
+ !args[0].toGCThing()->zone()->isAtomsZone()) {
+ ReportUsageErrorASCII(cx, callee,
+ "Expected an atom as the single argument");
+ return false;
+ }
+
+ gc::TenuredCell* atom = &args[0].toGCThing()->asTenured();
+ if ((atom->is<JSString>() &&
+ atom->as<JSString>()->isPermanentAndMayBeShared()) ||
+ (atom->is<JS::Symbol>() &&
+ atom->as<JS::Symbol>()->isPermanentAndMayBeShared())) {
+ ReportUsageErrorASCII(
+ cx, callee, "Atom marking bitmap is not used for permanent atoms");
+ return false;
+ }
+
+ if (atom->is<JSString>() && atom->as<JSString>()->asAtom().isPinned()) {
+ ReportUsageErrorASCII(cx, callee,
+ "Atom marking bitmap is not used for pinned atoms");
+ return false;
+ }
+
+ size_t index = gc::AtomMarkingRuntime::getAtomBit(atom);
+ MOZ_RELEASE_ASSERT(index <= INT32_MAX);
+ args.rval().setInt32(index);
+ return true;
+}
+
+static bool GetAtomMarkColor(JSContext* cx, unsigned argc, Value* vp) {
+ CallArgs args = CallArgsFromVp(argc, vp);
+ RootedObject callee(cx, &args.callee());
+
+ if (args.length() != 2) {
+ ReportUsageErrorASCII(cx, callee, "Expected two arguments");
+ return false;
+ }
+
+ if (!args[0].isObject()) {
+ ReportUsageErrorASCII(cx, callee,
+ "Expected an object as the first argument");
+ return false;
+ }
+ Zone* zone = UncheckedUnwrap(&args[0].toObject())->zone();
+
+ if (!args[1].isInt32() || args[1].toInt32() < 0) {
+ ReportUsageErrorASCII(cx, callee,
+ "Expected a positive integer as the second argument");
+ return false;
+ }
+ size_t index = args[1].toInt32();
+
+ gc::GCRuntime* gc = &cx->runtime()->gc;
+ gc::CellColor color = gc->atomMarking.getAtomMarkColorForIndex(zone, index);
+ RootedString name(cx, JS_NewStringCopyZ(cx, gc::CellColorName(color)));
+ if (!name) {
+ return false;
+ }
+
+ args.rval().setString(name);
+ return true;
+}
+
static WeakMapObject* MaybeWeakMapObject(const Value& value) {
if (!value.isObject()) {
return nullptr;
@@ -10387,6 +10455,14 @@ gc::ZealModeHelpText),
"isAtomMarked(obj, atom)",
" Return whether |atom| is marked relative to the zone containing |obj|."),
+ JS_FN_HELP("getAtomMarkIndex", GetAtomMarkIndex, 1, 0,
+"getAtomMarkIndex(atom)",
+" Return the atom marking bitmap's index for |atom|."),
+
+ JS_FN_HELP("getAtomMarkColor", GetAtomMarkColor, 2, 0,
+"getAtomMarkColor(obj, index)",
+" Return the atom marking bitmap's mark color for |index| relative to the zone containing |obj|."),
+
JS_FN_HELP("setMallocMaxDirtyPageModifier", SetMallocMaxDirtyPageModifier, 1, 0,
"setMallocMaxDirtyPageModifier(value)",
" Change the maximum size of jemalloc's page cache. The value should be between\n"
diff --git a/js/src/jit-test/tests/gc/bug-1994871-2.js b/js/src/jit-test/tests/gc/bug-1994871-2.js
@@ -0,0 +1,16 @@
+// Check a gray symbol keeps a weak map entry alive.
+
+let wm = new WeakMap();
+s = Symbol();
+wm.set(s, {});
+gc();
+assertEq(nondeterministicGetWeakMapSize(wm), 1);
+
+grayRoot().push(s);
+s = undefined;
+gc();
+assertEq(nondeterministicGetWeakMapSize(wm), 1);
+
+grayRoot().length = 0;
+gc();
+assertEq(nondeterministicGetWeakMapSize(wm), 0);
diff --git a/js/src/jit-test/tests/gc/bug-1994871.js b/js/src/jit-test/tests/gc/bug-1994871.js
@@ -0,0 +1,40 @@
+// Check that atom marking bitmap state and mark color is as expected and
+// AtomMarkingRuntime::refineZoneBitmapsForCollectedZones works with gray
+// symbols.
+
+gczeal(0);
+
+// We'll track the state of a symbol in two zones, |this| and |other|.
+let s = Symbol();
+addMarkObservers([s]);
+let other = newGlobal({newCompartment: true});
+
+// Iniitally we expect the symbol to be marked black in |this| and not marked
+// in |other|.
+gc();
+let index = getAtomMarkIndex(s);
+assertEq(getAtomMarkColor(this, index), 'black');
+assertEq(getAtomMarkColor(other, index), 'white');
+assertEq(getMarks()[0], 'black');
+
+// Referencing the symbol in |other| marks it in that zone.
+other.s = s;
+assertEq(getAtomMarkColor(other, index), 'black');
+
+// Replace existing references with a gray reference from |this| and remove
+// the reference from |other|. We now expect the mark color to be gray in
+// *both* zones.
+grayRoot().push(s);
+other.s = undefined;
+s = undefined;
+gc();
+assertEq(getAtomMarkColor(this, index), 'gray');
+assertEq(getAtomMarkColor(other, index), 'gray');
+assertEq(getMarks()[0], 'gray');
+
+// Removing the final reference results in the mark color going to white.
+grayRoot().length = 0;
+gc();
+assertEq(getAtomMarkColor(this, index), 'white');
+assertEq(getAtomMarkColor(other, index), 'white');
+assertEq(getMarks()[0], 'dead');
diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp
@@ -211,6 +211,7 @@
#include "wasm/WasmFeatures.h"
#include "wasm/WasmJS.h"
+#include "gc/WeakMap-inl.h"
#include "vm/Compartment-inl.h"
#include "vm/ErrorObject-inl.h"
#include "vm/Interpreter-inl.h"
@@ -8749,7 +8750,7 @@ static MarkBitObservers* EnsureMarkBitObservers(JSContext* cx) {
ShellContext* sc = GetShellContext(cx);
if (!sc->markObservers) {
auto* observers =
- cx->new_<MarkBitObservers>(cx->runtime(), NonshrinkingGCObjectVector());
+ cx->new_<MarkBitObservers>(cx->runtime(), NonShrinkingValueVector());
if (!observers) {
return nullptr;
}
@@ -8797,19 +8798,17 @@ static bool AddMarkObservers(JSContext* cx, unsigned argc, Value* vp) {
}
RootedValue value(cx);
- RootedObject object(cx);
for (uint32_t i = 0; i < length; i++) {
if (!JS_GetElement(cx, observersArg, i, &value)) {
return false;
}
- if (!value.isObject()) {
- JS_ReportErrorASCII(cx, "argument must be an Array of objects");
+ if (!CanBeHeldWeakly(value)) {
+ JS_ReportErrorASCII(cx, "Can only observe objects and symbols");
return false;
}
- object = &value.toObject();
- if (gc::IsInsideNursery(object)) {
+ if (gc::IsInsideNursery(value.toGCThing())) {
// WeakCaches are not swept during a minor GC. To prevent
// nursery-allocated contents from having the mark bits be deceptively
// black until the second GC, they would need to be marked weakly (cf
@@ -8818,7 +8817,7 @@ static bool AddMarkObservers(JSContext* cx, unsigned argc, Value* vp) {
cx->runtime()->gc.evictNursery();
}
- if (!markObservers->get().append(object)) {
+ if (!markObservers->get().append(value)) {
ReportOutOfMemory(cx);
return false;
}
@@ -8828,6 +8827,30 @@ static bool AddMarkObservers(JSContext* cx, unsigned argc, Value* vp) {
return true;
}
+static const char* ObserveMarkColor(const Value& value) {
+ if (value.isUndefined()) {
+ return "dead";
+ }
+
+ gc::Cell* cell = value.toGCThing();
+ Zone* zone = cell->zone();
+ if (zone->isGCPreparing()) {
+ // The mark bits are not valid during unmarking.
+ return "unmarked";
+ }
+
+ gc::TenuredCell* tc = &cell->asTenured();
+ if (tc->isMarkedGray()) {
+ return "gray";
+ }
+
+ if (tc->isMarkedBlack()) {
+ return "black";
+ }
+
+ return "unmarked";
+}
+
static bool GetMarks(JSContext* cx, unsigned argc, Value* vp) {
CallArgs args = CallArgsFromVp(argc, vp);
@@ -8844,27 +8867,10 @@ static bool GetMarks(JSContext* cx, unsigned argc, Value* vp) {
}
for (uint32_t i = 0; i < length; i++) {
- const char* color;
- JSObject* obj = observers->get()[i];
- if (!obj) {
- color = "dead";
- } else if (obj->zone()->isGCPreparing()) {
- color = "unmarked";
- } else {
- gc::TenuredCell* cell = &obj->asTenured();
- if (cell->isMarkedGray()) {
- color = "gray";
- } else if (cell->isMarkedBlack()) {
- color = "black";
- } else {
- color = "unmarked";
- }
- }
+ Value value = observers->get()[i];
+ const char* color = ObserveMarkColor(value);
JSString* s = JS_NewStringCopyZ(cx, color);
- if (!s) {
- return false;
- }
- if (!NewbornArrayPush(cx, ret, StringValue(s))) {
+ if (!s || !NewbornArrayPush(cx, ret, StringValue(s))) {
return false;
}
}
diff --git a/js/src/shell/jsshell.h b/js/src/shell/jsshell.h
@@ -162,18 +162,20 @@ extern UniqueChars processWideModuleLoadPath;
bool CreateAlias(JSContext* cx, const char* dstName,
JS::HandleObject namespaceObj, const char* srcName);
-class NonshrinkingGCObjectVector
- : public GCVector<HeapPtr<JSObject*>, 0, SystemAllocPolicy> {
+class NonShrinkingValueVector
+ : public GCVector<HeapPtr<Value>, 0, SystemAllocPolicy> {
+ using Base = GCVector<HeapPtr<Value>, 0, SystemAllocPolicy>;
+
public:
bool traceWeak(JSTracer* trc) {
- for (HeapPtr<JSObject*>& obj : *this) {
- TraceWeakEdge(trc, &obj, "NonshrinkingGCObjectVector element");
+ for (HeapPtr<Value>& value : *this) {
+ TraceWeakEdge(trc, &value, "NonShrinkingValueVector element");
}
return true;
}
};
-using MarkBitObservers = JS::WeakCache<NonshrinkingGCObjectVector>;
+using MarkBitObservers = JS::WeakCache<NonShrinkingValueVector>;
#ifdef SINGLESTEP_PROFILING
using StackChars = Vector<char16_t, 0, SystemAllocPolicy>;
