diff --git a/js/src/debugger/Debugger.cpp b/js/src/debugger/Debugger.cpp index 32a0f64f757f..e761acb3f3d6 100644 --- a/js/src/debugger/Debugger.cpp +++ b/js/src/debugger/Debugger.cpp @@ -3550,7 +3550,6 @@ void Debugger::traceCrossCompartmentEdges(JSTracer* trc) { void DebugAPI::traceCrossCompartmentEdges(JSTracer* trc) { JSRuntime* rt = trc->runtime(); gc::State state = rt->gc.state(); - MOZ_ASSERT(state == gc::State::MarkRoots || state == gc::State::Compact); for (Debugger* dbg : rt->debuggerList()) { Zone* zone = MaybeForwarded(dbg->object.get())->zone(); diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp index d6b4b16893c6..5bc4152cb851 100644 --- a/js/src/gc/GC.cpp +++ b/js/src/gc/GC.cpp @@ -2474,19 +2474,19 @@ void GCRuntime::updateRuntimePointersToRelocatedCells(AutoGCSession& session) { rt->geckoProfiler().fixupStringsMapAfterMovingGC(); + // Mark roots to update them. + traceRuntimeForMajorGC(&trc, session); - // Mark roots to update them. { gcstats::AutoPhase ap2(stats(), gcstats::PhaseKind::MARK_ROOTS); DebugAPI::traceAllForMovingGC(&trc); DebugAPI::traceCrossCompartmentEdges(&trc); - // Mark all gray roots, making sure we call the trace callback to get the - // current set. - if (JSTraceDataOp op = grayRootTracer.op) { - (*op)(&trc, grayRootTracer.data); - } + // Mark all gray roots. We call the trace callback to get the current set. + traceEmbeddingGrayRoots(&trc); + Compartment::traceIncomingCrossCompartmentEdgesForZoneGC( + &trc, Compartment::GrayEdges); } // Sweep everything to fix up weak pointers. @@ -4172,9 +4172,9 @@ void GCRuntime::markGrayRoots(gcstats::PhaseKind phase) { } } else { MOZ_ASSERT(!isIncremental); - if (JSTraceDataOp op = grayRootTracer.op) { - (*op)(&marker, grayRootTracer.data); - } + traceEmbeddingGrayRoots(&marker); + Compartment::traceIncomingCrossCompartmentEdgesForZoneGC( + &marker, Compartment::GrayEdges); } } diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 55565c248063..52478bf7b33a 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -3631,16 +3631,16 @@ bool UnmarkGrayTracer::onChild(const JS::GCCellPtr& thing) { TenuredCell& tenured = cell->asTenured(); - // If the cell is in a zone that we're currently marking gray, then it's - // possible that it is currently white but will end up gray. To handle this - // case, push any cells in zones that are currently being marked onto the - // mark stack and they will eventually get marked black. + // If the cell is in a zone that we're currently marking, then it's possible + // that it is currently white but will end up gray. To handle this case, push + // any cells in zones that are currently being marked onto the mark stack and + // they will eventually get marked black. Zone* zone = tenured.zone(); - if (zone->isGCMarkingBlackAndGray()) { + if (zone->isGCMarking()) { if (!cell->isMarkedBlack()) { Cell* tmp = cell; - TraceManuallyBarrieredGenericPointerEdge(zone->barrierTracer(), &tmp, - "read barrier"); + JSTracer* trc = &runtime()->gc.marker; + TraceManuallyBarrieredGenericPointerEdge(trc, &tmp, "read barrier"); MOZ_ASSERT(tmp == cell); unmarkedAny = true; } diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index f2e5e421e341..b9e9ed0086d9 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -276,7 +276,15 @@ void js::gc::GCRuntime::traceRuntimeForMajorGC(JSTracer* trc, traceRuntimeAtoms(trc, session.checkAtomsAccess()); } traceKeptAtoms(trc); - Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(trc); + + { + // Trace incoming cross compartment edges from uncollected compartments, + // skipping gray edges which are traced later. + gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::MARK_CCWS); + Compartment::traceIncomingCrossCompartmentEdgesForZoneGC( + trc, Compartment::NonGrayEdges); + } + traceRuntimeCommon(trc, MarkRuntime); } @@ -524,9 +532,9 @@ void js::gc::GCRuntime::bufferGrayRoots() { } BufferGrayRootsTracer grayBufferer(rt); - if (JSTraceDataOp op = grayRootTracer.op) { - (*op)(&grayBufferer, grayRootTracer.data); - } + traceEmbeddingGrayRoots(&grayBufferer); + Compartment::traceIncomingCrossCompartmentEdgesForZoneGC( + &grayBufferer, Compartment::GrayEdges); // Propagate the failure flag from the marker to the runtime. if (grayBufferer.failed()) { diff --git a/js/src/jit-test/tests/gc/bug-1578462.js b/js/src/jit-test/tests/gc/bug-1578462.js new file mode 100644 index 000000000000..a8d2dd155ae8 --- /dev/null +++ b/js/src/jit-test/tests/gc/bug-1578462.js @@ -0,0 +1,52 @@ +// |jit-test| skip-if: !('gcstate' in this) + +// Check that incoming CCW edges are marked gray in a zone GC if the +// source is gray. + +// Set up a new compartment with a gray object wrapper to this +// compartment. +function createOtherCompartment() { + let t = {}; + addMarkObservers([t]); + let g = newGlobal({newCompartment: true}); + g.t = t; + g.eval(`grayRoot().push(t);`); + g.t = null; + t = null; + return g; +} + +gczeal(0); + +let g = createOtherCompartment(); + +// The target should be gray in a full GC... +gc(); +assertEq(getMarks()[0], "gray"); + +// and subsequently gray in a zone GC of only this compartment. +gc(this); +assertEq(getMarks()[0], "gray"); + +// If a barrier marks the gray wrapper black after the start of the +// GC, the target ends up black. +schedulegc(this); +startgc(1); +assertEq(getMarks()[0], "unmarked"); +g.eval(`grayRoot()`); // Barrier marks gray roots black. +assertEq(getMarks()[0], "black"); +finishgc(); + +// A full collection resets the wrapper to gray. +gc(); +assertEq(getMarks()[0], "gray"); + +// If a barrier marks the gray wrapper black after the target has +// already been marked gray, the target ends up black. +gczeal(25); // Yield during gray marking. +schedulegc(this); +startgc(1); +assertEq(getMarks()[0], "gray"); +g.eval(`grayRoot()`); // Barrier marks gray roots black. +assertEq(getMarks()[0], "black"); +finishgc(); diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 80b93612485c..cb47e25960eb 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -6323,6 +6323,44 @@ static bool RecomputeWrappers(JSContext* cx, unsigned argc, Value* vp) { return true; } +static bool DumpObjectWrappers(JSContext* cx, unsigned argc, Value* vp) { + CallArgs args = CallArgsFromVp(argc, vp); + + bool printedHeader = false; + for (ZonesIter zone(cx->runtime(), WithAtoms); !zone.done(); zone.next()) { + bool printedZoneInfo = false; + for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) { + bool printedCompartmentInfo = false; + for (Compartment::ObjectWrapperEnum e(comp); !e.empty(); e.popFront()) { + JSObject* wrapper = e.front().value().unbarrieredGet(); + JSObject* wrapped = e.front().key(); + if (!printedHeader) { + fprintf(stderr, "Cross-compartment object wrappers:\n"); + printedHeader = true; + } + if (!printedZoneInfo) { + fprintf(stderr, " Zone %p:\n", zone.get()); + printedZoneInfo = true; + } + if (!printedCompartmentInfo) { + fprintf(stderr, " Compartment %p:\n", comp.get()); + printedCompartmentInfo = true; + } + fprintf(stderr, + " Object wrapper %p -> %p in zone %p compartment %p\n", + wrapper, wrapped, wrapped->zone(), wrapped->compartment()); + } + } + } + + if (!printedHeader) { + fprintf(stderr, "No cross-compartment object wrappers.\n"); + } + + args.rval().setUndefined(); + return true; +} + static bool GetMaxArgs(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); args.rval().setInt32(ARGS_LENGTH_MAX); @@ -8836,6 +8874,10 @@ JS_FN_HELP("parseBin", BinParse, 1, 0, " and can be used to filter source or target compartments: the unwrapped\n" " object's compartment is used as CompartmentFilter.\n"), + JS_FN_HELP("dumpObjectWrappers", DumpObjectWrappers, 2, 0, +"dumpObjectWrappers()", +" Print information about cross-compartment object wrappers.\n"), + JS_FN_HELP("wrapWithProto", WrapWithProto, 2, 0, "wrapWithProto(obj)", " Wrap an object into a noop wrapper with prototype semantics."), diff --git a/js/src/vm/Compartment.cpp b/js/src/vm/Compartment.cpp index 980d671652f1..7f545e647a69 100644 --- a/js/src/vm/Compartment.cpp +++ b/js/src/vm/Compartment.cpp @@ -419,7 +419,19 @@ bool Compartment::wrap(JSContext* cx, MutableHandle> vec) { return true; } -void Compartment::traceWrapperTargetsInCollectedZones(JSTracer* trc) { +static inline bool ShouldTraceWrapper(JSObject* wrapper, + Compartment::EdgeSelector whichEdges) { + if (whichEdges == Compartment::AllEdges) { + return true; + } + + bool isGray = wrapper->isMarkedGray(); + return (whichEdges == Compartment::NonGrayEdges && !isGray) || + (whichEdges == Compartment::GrayEdges && isGray); +} + +void Compartment::traceWrapperTargetsInCollectedZones(JSTracer* trc, + EdgeSelector whichEdges) { // Trace cross compartment wrapper private pointers into collected zones to // either mark or update them. Wrapped object pointers are updated by // sweepCrossCompartmentObjectWrappers(). @@ -430,33 +442,39 @@ void Compartment::traceWrapperTargetsInCollectedZones(JSTracer* trc) { for (WrappedObjectCompartmentEnum c(this); !c.empty(); c.popFront()) { Zone* zone = c.front()->zone(); - if (!zone->isCollecting()) { + if (!zone->isCollectingFromAnyThread()) { continue; } for (ObjectWrapperEnum e(this, c); !e.empty(); e.popFront()) { JSObject* obj = e.front().value().unbarrieredGet(); ProxyObject* wrapper = &obj->as(); - ProxyObject::traceEdgeToTarget(trc, wrapper); + if (ShouldTraceWrapper(wrapper, whichEdges)) { + ProxyObject::traceEdgeToTarget(trc, wrapper); + } } } } /* static */ -void Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(JSTracer* trc) { - gcstats::AutoPhase ap(trc->runtime()->gc.stats(), - gcstats::PhaseKind::MARK_CCWS); +void Compartment::traceIncomingCrossCompartmentEdgesForZoneGC( + JSTracer* trc, EdgeSelector whichEdges) { MOZ_ASSERT(JS::RuntimeHeapIsMajorCollecting()); + for (ZonesIter zone(trc->runtime(), SkipAtoms); !zone.done(); zone.next()) { - if (zone->isCollecting()) { + if (zone->isCollectingFromAnyThread()) { continue; } for (CompartmentsInZoneIter c(zone); !c.done(); c.next()) { - c->traceWrapperTargetsInCollectedZones(trc); + c->traceWrapperTargetsInCollectedZones(trc, whichEdges); } } - DebugAPI::traceCrossCompartmentEdges(trc); + + // Currently we trace all debugger edges as black. + if (whichEdges != GrayEdges) { + DebugAPI::traceCrossCompartmentEdges(trc); + } } void Compartment::sweepAfterMinorGC(JSTracer* trc) { @@ -482,7 +500,7 @@ void Compartment::fixupCrossCompartmentObjectWrappersAfterMovingGC( // Trace the wrappers in the map to update their cross-compartment edges // to wrapped values in other compartments that may have been moved. - traceWrapperTargetsInCollectedZones(trc); + traceWrapperTargetsInCollectedZones(trc, AllEdges); } void Compartment::fixupAfterMovingGC(JSTracer* trc) { diff --git a/js/src/vm/Compartment.h b/js/src/vm/Compartment.h index bdd413c99fc2..b9336aa78154 100644 --- a/js/src/vm/Compartment.h +++ b/js/src/vm/Compartment.h @@ -412,8 +412,11 @@ class JS::Compartment { * dangling (full GCs naturally follow pointers across compartments) and * when compacting to update cross-compartment pointers. */ - void traceWrapperTargetsInCollectedZones(JSTracer* trc); - static void traceIncomingCrossCompartmentEdgesForZoneGC(JSTracer* trc); + enum EdgeSelector { AllEdges, NonGrayEdges, GrayEdges }; + void traceWrapperTargetsInCollectedZones(JSTracer* trc, + EdgeSelector whichEdges); + static void traceIncomingCrossCompartmentEdgesForZoneGC( + JSTracer* trc, EdgeSelector whichEdges); void sweepRealms(JSFreeOp* fop, bool keepAtleastOne, bool destroyingRuntime); void sweepAfterMinorGC(JSTracer* trc);