Bug 1169710 - Part 5: Fix a GC hazard and properly mark cross compartment edges in the tenure promotions log; r=sfink,jonco

js/src/devtools/rootAnalysis/annotations.js

@@ -189,6 +189,8 @@ var ignoreFunctions = {
     // static and dynamic checks for no GC in the non-test code, and in the test
     // code we fall back to only the dynamic checks.
     "void test::RingbufferDumper::OnTestPartResult(testing::TestPartResult*)" : true,
+
+    "float64 JS_GetCurrentEmbedderTime()" : true,
 };
 
 function isProtobuf(name)

js/src/gc/Nursery.cpp

@@ -496,12 +496,6 @@ js::Nursery::collect(JSRuntime* rt, JS::gcreason::Reason reason, ObjectGroupList
     sweep();
     TIME_END(sweep);
 
-    TIME_START(logPromotionsToTenured);
-    for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) {
-        zone->logPromotionsToTenured();
-    }
-    TIME_END(logPromotionsToTenured);
-
     TIME_START(clearStoreBuffer);
     rt->gc.storeBuffer.clear();
     TIME_END(clearStoreBuffer);
@@ -537,6 +531,12 @@ js::Nursery::collect(JSRuntime* rt, JS::gcreason::Reason reason, ObjectGroupList
     }
     TIME_END(pretenure);
 
+    TIME_START(logPromotionsToTenured);
+    for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) {
+        zone->logPromotionsToTenured();
+    }
+    TIME_END(logPromotionsToTenured);
+
     // We ignore gcMaxBytes when allocating for minor collection. However, if we
     // overflowed, we disable the nursery. The next time we allocate, we'll fail
     // because gcBytes >= gcMaxBytes.
@@ -555,7 +555,7 @@ js::Nursery::collect(JSRuntime* rt, JS::gcreason::Reason reason, ObjectGroupList
         static bool printedHeader = false;
         if (!printedHeader) {
             fprintf(stderr,
-                    "MinorGC: Reason               PRate  Size Time   mkVals mkClls mkSlts mkWCll mkGnrc ckTbls mkRntm mkDbgr clrNOC collct swpABO updtIn runFin frSlts clrSB  sweep logPtT resize pretnr\n");
+                    "MinorGC: Reason               PRate  Size Time   mkVals mkClls mkSlts mkWCll mkGnrc ckTbls mkRntm mkDbgr clrNOC collct swpABO updtIn runFin frSlts clrSB  sweep resize pretnr logPtT\n");
             printedHeader = true;
         }
 
@@ -582,9 +582,9 @@ js::Nursery::collect(JSRuntime* rt, JS::gcreason::Reason reason, ObjectGroupList
                 TIME_TOTAL(freeMallocedBuffers),
                 TIME_TOTAL(clearStoreBuffer),
                 TIME_TOTAL(sweep),
-                TIME_TOTAL(logPromotionsToTenured),
                 TIME_TOTAL(resize),
-                TIME_TOTAL(pretenure));
+                TIME_TOTAL(pretenure),
+                TIME_TOTAL(logPromotionsToTenured));
 #undef FMT
     }
 }

js/src/jsgc.cpp

@@ -4381,8 +4381,6 @@ JSCompartment::findOutgoingEdges(ComponentFinder<JS::Zone>& finder)
                 finder.addEdgeTo(w);
         }
     }
-
-    Debugger::findCompartmentEdges(zone(), finder);
 }
 
 void
@@ -4404,6 +4402,8 @@ Zone::findOutgoingEdges(ComponentFinder<JS::Zone>& finder)
             finder.addEdgeTo(r.front());
     }
     gcZoneGroupEdges.clear();
+
+    Debugger::findZoneEdges(this, finder);
 }
 
 bool

js/src/vm/Debugger.cpp

@@ -2344,6 +2344,24 @@ Debugger::markCrossCompartmentEdges(JSTracer* trc)
     environments.markCrossCompartmentEdges<DebuggerEnv_trace>(trc);
     scripts.markCrossCompartmentEdges<DebuggerScript_trace>(trc);
     sources.markCrossCompartmentEdges<DebuggerSource_trace>(trc);
+
+    // Because we don't have access to a `cx` inside
+    // `Debugger::logTenurePromotion`, we can't hold onto CCWs inside the log,
+    // and instead have unwrapped cross-compartment edges. We need to be sure to
+    // mark those here.
+    traceTenurePromotionsLog(trc);
+}
+
+/*
+ * Trace every entry in the promoted to tenured heap log.
+ */
+void
+Debugger::traceTenurePromotionsLog(JSTracer* trc)
+{
+    for (TenurePromotionsEntry* e = tenurePromotionsLog.getFirst(); e; e = e->getNext()) {
+        if (e->frame)
+            TraceEdge(trc, &e->frame, "Debugger::tenurePromotionsLog SavedFrame");
+    }
 }
 
 /*
@@ -2534,13 +2552,7 @@ Debugger::trace(JSTracer* trc)
             TraceEdge(trc, &s->ctorName, "allocation log constructor name");
     }
 
-    /*
-     * Mark every entry in the promoted to tenured heap log.
-     */
-    for (TenurePromotionsEntry* e = tenurePromotionsLog.getFirst(); e; e = e->getNext()) {
-        if (e->frame)
-            TraceEdge(trc, &e->frame, "tenure promotions log SavedFrame");
-    }
+    traceTenurePromotionsLog(trc);
 
     /* Trace the weak map from JSScript instances to Debugger.Script objects. */
     scripts.trace(trc);
@@ -2583,7 +2595,7 @@ Debugger::detachAllDebuggersFromGlobal(FreeOp* fop, GlobalObject* global)
 }
 
 /* static */ void
-Debugger::findCompartmentEdges(Zone* zone, js::gc::ComponentFinder<Zone>& finder)
+Debugger::findZoneEdges(Zone* zone, js::gc::ComponentFinder<Zone>& finder)
 {
     /*
      * For debugger cross compartment wrappers, add edges in the opposite
@@ -2598,7 +2610,8 @@ Debugger::findCompartmentEdges(Zone* zone, js::gc::ComponentFinder<Zone>& finder
         Zone* w = dbg->object->zone();
         if (w == zone || !w->isGCMarking())
             continue;
-        if (dbg->scripts.hasKeyInZone(zone) ||
+        if (dbg->debuggeeZones.has(zone) ||
+            dbg->scripts.hasKeyInZone(zone) ||
             dbg->sources.hasKeyInZone(zone) ||
             dbg->objects.hasKeyInZone(zone) ||
             dbg->environments.hasKeyInZone(zone))
@@ -6996,9 +7009,9 @@ Debugger::getObjectAllocationSite(JSObject& obj)
         return nullptr;
 
     MOZ_ASSERT(!metadata->is<WrapperObject>());
-    if (!SavedFrame::isSavedFrameAndNotProto(*metadata))
-        return nullptr;
-    return metadata;
+    return SavedFrame::isSavedFrameAndNotProto(*metadata)
+        ? metadata
+        : nullptr;
 }
 
 static bool

js/src/vm/Debugger.h

@@ -495,6 +495,7 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
     void trace(JSTracer* trc);
     static void finalize(FreeOp* fop, JSObject* obj);
     void markCrossCompartmentEdges(JSTracer* tracer);
+    void traceTenurePromotionsLog(JSTracer* trc);
 
     static const Class jsclass;
 
@@ -688,7 +689,7 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
     static void markAll(JSTracer* trc);
     static void sweepAll(FreeOp* fop);
     static void detachAllDebuggersFromGlobal(FreeOp* fop, GlobalObject* global);
-    static void findCompartmentEdges(JS::Zone* v, gc::ComponentFinder<JS::Zone>& finder);
+    static void findZoneEdges(JS::Zone* v, gc::ComponentFinder<JS::Zone>& finder);
 
     /*
      * JSTrapStatus Overview

js/src/vm/DebuggerMemory.cpp

@@ -363,10 +363,10 @@ DebuggerMemory::drainTenurePromotionsLog(JSContext* cx, unsigned argc, Value* vp
         if (!obj)
             return false;
 
-        // Don't pop the TenurePromotionsEntry yet. The queue's links are followed by
-        // the GC to find the TenurePromotionsEntry, but are not barriered, so we must
-        // edit them with great care. Use the queue entry in place, and then
-        // pop and delete together.
+        // Don't pop the TenurePromotionsEntry yet. The queue's links are
+        // followed by the GC to find the TenurePromotionsEntry, but are not
+        // barriered, so we must edit them with great care. Use the queue entry
+        // in place, and then pop and delete together.
         auto* entry = dbg->tenurePromotionsLog.getFirst();
 
         RootedValue frame(cx, ObjectOrNullValue(entry->frame));