Bug 903548 - GC: What do we do for UnmarkGray on a Nursery GCThing? r=billm

js/public/GCAPI.h

@@ -256,7 +256,7 @@ class ObjectPtr
  * Unsets the gray bit for anything reachable from |thing|. |kind| should not be
  * JSTRACE_SHAPE. |thing| should be non-null.
  */
-extern JS_FRIEND_API(void)
+extern JS_FRIEND_API(bool)
 UnmarkGrayGCThingRecursively(void *thing, JSGCTraceKind kind);
 
 /*

js/public/HeapAPI.h

@@ -128,6 +128,16 @@ GetObjectZone(JSObject *obj)
 static JS_ALWAYS_INLINE bool
 GCThingIsMarkedGray(void *thing)
 {
+#ifdef JSGC_GENERATIONAL
+    /*
+     * GC things residing in the nursery cannot be gray: they have no mark bits.
+     * All live objects in the nursery are moved to tenured at the beginning of
+     * each GC slice, so the gray marker never sees nursery things.
+     */
+    JS::shadow::Runtime *rt = js::gc::GetGCThingRuntime(thing);
+    if (uintptr_t(thing) >= rt->gcNurseryStart_ && uintptr_t(thing) < rt->gcNurseryEnd_)
+        return false;
+#endif
     uintptr_t *word, mask;
     js::gc::GetGCThingMarkWordAndMask(thing, js::gc::GRAY, &word, &mask);
     return *word & mask;

js/src/gc/Marking.cpp

@@ -651,10 +651,15 @@ ShouldMarkCrossCompartment(JSTracer *trc, JSObject *src, Cell *cell)
     if (!IS_GC_MARKING_TRACER(trc))
         return true;
 
-    JS::Zone *zone = cell->tenuredZone();
     uint32_t color = AsGCMarker(trc)->getMarkColor();
-
     JS_ASSERT(color == BLACK || color == GRAY);
+
+    if (IsInsideNursery(trc->runtime, cell)) {
+        JS_ASSERT(color == BLACK);
+        return false;
+    }
+
+    JS::Zone *zone = cell->tenuredZone();
     if (color == BLACK) {
         /*
          * Having black->gray edges violates our promise to the cycle
@@ -1578,14 +1583,14 @@ struct UnmarkGrayTracer : public JSTracer
      * up any color mismatches involving weakmaps when it runs.
      */
     UnmarkGrayTracer(JSRuntime *rt)
-      : tracingShape(false), previousShape(NULL)
+      : tracingShape(false), previousShape(NULL), unmarkedAny(false)
     {
         JS_TracerInit(this, rt, UnmarkGrayChildren);
         eagerlyTraceWeakMaps = DoNotTraceWeakMaps;
     }
 
     UnmarkGrayTracer(JSTracer *trc, bool tracingShape)
-      : tracingShape(tracingShape), previousShape(NULL)
+      : tracingShape(tracingShape), previousShape(NULL), unmarkedAny(false)
     {
         JS_TracerInit(this, trc->runtime, UnmarkGrayChildren);
         eagerlyTraceWeakMaps = DoNotTraceWeakMaps;
@@ -1596,6 +1601,9 @@ struct UnmarkGrayTracer : public JSTracer
 
     /* If tracingShape, shape child or NULL. Otherwise, NULL. */
     void *previousShape;
+
+    /* Whether we unmarked anything. */
+    bool unmarkedAny;
 };
 
 /*
@@ -1642,10 +1650,14 @@ UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind)
         return;
     }
 
-    if (!JS::GCThingIsMarkedGray(thing))
+    UnmarkGrayTracer *tracer = static_cast<UnmarkGrayTracer *>(trc);
-        return;
+    if (!IsInsideNursery(trc->runtime, thing)) {
+        if (!JS::GCThingIsMarkedGray(thing))
+            return;
 
-    UnmarkGrayGCThing(thing);
+        UnmarkGrayGCThing(thing);
+        tracer->unmarkedAny = true;
+    }
 
     /*
      * Trace children of |thing|. If |thing| and its parent are both shapes,
@@ -1654,12 +1666,12 @@ UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind)
      * depth during shape tracing. It is safe to do because a shape can only
      * have one child that is a shape.
      */
-    UnmarkGrayTracer *tracer = static_cast<UnmarkGrayTracer *>(trc);
     UnmarkGrayTracer childTracer(tracer, kind == JSTRACE_SHAPE);
 
     if (kind != JSTRACE_SHAPE) {
         JS_TraceChildren(&childTracer, thing, kind);
         JS_ASSERT(!childTracer.previousShape);
+        tracer->unmarkedAny |= childTracer.unmarkedAny;
         return;
     }
 
@@ -1675,19 +1687,27 @@ UnmarkGrayChildren(JSTracer *trc, void **thingp, JSGCTraceKind kind)
         thing = childTracer.previousShape;
         childTracer.previousShape = NULL;
     } while (thing);
+    tracer->unmarkedAny |= childTracer.unmarkedAny;
 }
 
-JS_FRIEND_API(void)
+JS_FRIEND_API(bool)
 JS::UnmarkGrayGCThingRecursively(void *thing, JSGCTraceKind kind)
 {
     JS_ASSERT(kind != JSTRACE_SHAPE);
 
-    if (!JS::GCThingIsMarkedGray(thing))
-        return;
-
-    UnmarkGrayGCThing(thing);
-
     JSRuntime *rt = static_cast<Cell *>(thing)->runtimeFromMainThread();
+
+    bool unmarkedArg = false;
+    if (!IsInsideNursery(rt, thing)) {
+        if (!JS::GCThingIsMarkedGray(thing))
+            return false;
+
+        UnmarkGrayGCThing(thing);
+        unmarkedArg = true;
+    }
+
     UnmarkGrayTracer trc(rt);
     JS_TraceChildren(&trc, thing, kind);
+
+    return unmarkedArg || trc.unmarkedAny;
 }

js/src/jsfriendapi.cpp

@@ -591,10 +591,11 @@ js::GCThingTraceKind(void *thing)
 JS_FRIEND_API(void)
 js::VisitGrayWrapperTargets(Zone *zone, GCThingCallback callback, void *closure)
 {
+    JSRuntime *rt = zone->runtimeFromMainThread();
     for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) {
         for (JSCompartment::WrapperEnum e(comp); !e.empty(); e.popFront()) {
             gc::Cell *thing = e.front().key.wrapped;
-            if (thing->isMarked(gc::GRAY))
+            if (!IsInsideNursery(rt, thing) && thing->isMarked(gc::GRAY))
                 callback(closure, thing);
         }
     }

xpcom/base/CycleCollectedJSRuntime.cpp

@@ -258,8 +258,9 @@ private:
     if (delegateMightNeedMarking && kkind == JSTRACE_OBJECT) {
       JSObject* kdelegate = js::GetWeakmapKeyDelegate((JSObject*)k);
       if (kdelegate && !xpc_IsGrayGCThing(kdelegate)) {
-        JS::UnmarkGrayGCThingRecursively(k, JSTRACE_OBJECT);
+        if (JS::UnmarkGrayGCThingRecursively(k, JSTRACE_OBJECT)) {
-        tracer->mAnyMarked = true;
+          tracer->mAnyMarked = true;
+        }
       }
     }
 
@@ -267,8 +268,9 @@ private:
         (!k || !xpc_IsGrayGCThing(k)) &&
         (!m || !xpc_IsGrayGCThing(m)) &&
         vkind != JSTRACE_SHAPE) {
-      JS::UnmarkGrayGCThingRecursively(v, vkind);
+      if (JS::UnmarkGrayGCThingRecursively(v, vkind)) {
-      tracer->mAnyMarked = true;
+        tracer->mAnyMarked = true;
+      }
     }
   }