Bug 1144331 - Assert that gray buffering does not depend on isMarking; r=jonco, a=RyanVM

js/src/gc/Iteration.cpp

@@ -20,7 +20,7 @@ using namespace js::gc;
 void
 js::TraceRuntime(JSTracer *trc)
 {
-    MOZ_ASSERT(!IS_GC_MARKING_TRACER(trc));
+    MOZ_ASSERT(!IsMarkingTracer(trc));
 
     JSRuntime *rt = trc->runtime();
     rt->gc.evictNursery();

js/src/gc/Marking.cpp

@@ -143,7 +143,7 @@ IsThingPoisoned(T *thing)
 static GCMarker *
 AsGCMarker(JSTracer *trc)
 {
-    MOZ_ASSERT(IS_GC_MARKING_TRACER(trc));
+    MOZ_ASSERT(IsMarkingTracer(trc));
     return static_cast<GCMarker *>(trc);
 }
 
@@ -191,14 +191,13 @@ CheckMarkedThing(JSTracer *trc, T **thingp)
     MOZ_ASSERT(zone->runtimeFromAnyThread() == trc->runtime());
     MOZ_ASSERT(trc->hasTracingDetails());
 
-    bool isGcMarkingTracer = IS_GC_MARKING_TRACER(trc);
+    MOZ_ASSERT(thing->isAligned());
+    MOZ_ASSERT(MapTypeToTraceKind<T>::kind == GetGCThingTraceKind(thing));
+
+    bool isGcMarkingTracer = IsMarkingGray(trc) || IsMarkingTracer(trc);
 
     MOZ_ASSERT_IF(zone->requireGCTracer(), isGcMarkingTracer);
 
-    MOZ_ASSERT(thing->isAligned());
-
-    MOZ_ASSERT(MapTypeToTraceKind<T>::kind == GetGCThingTraceKind(thing));
-
     if (isGcMarkingTracer) {
         GCMarker *gcMarker = static_cast<GCMarker *>(trc);
         MOZ_ASSERT_IF(gcMarker->shouldCheckCompartments(),
@@ -299,9 +298,9 @@ MarkInternal(JSTracer *trc, T **thingp)
     trc->clearTracingDetails();
 }
 
-#define JS_ROOT_MARKING_ASSERT(trc)                                     \
-    MOZ_ASSERT_IF(IS_GC_MARKING_TRACER(trc),                            \
-                  trc->runtime()->gc.state() == NO_INCREMENTAL ||       \
+#define JS_ROOT_MARKING_ASSERT(trc) \
+    MOZ_ASSERT_IF(IsMarkingTracer(trc), \
+                  trc->runtime()->gc.state() == NO_INCREMENTAL || \
                   trc->runtime()->gc.state() == MARK_ROOTS);
 
 namespace js {
@@ -954,7 +953,7 @@ gc::MarkObjectSlots(JSTracer *trc, NativeObject *obj, uint32_t start, uint32_t n
 static bool
 ShouldMarkCrossCompartment(JSTracer *trc, JSObject *src, Cell *cell)
 {
-    if (!IS_GC_MARKING_TRACER(trc))
+    if (!IsMarkingTracer(trc))
         return true;
 
     uint32_t color = AsGCMarker(trc)->markColor();

js/src/gc/Tracer.cpp

@@ -663,15 +663,14 @@ GCMarker::startBufferingGrayRoots()
 
     MOZ_ASSERT(!callback);
     callback = GrayCallback;
-    MOZ_ASSERT(IS_GC_MARKING_TRACER(this));
+    MOZ_ASSERT(IsMarkingGray(this));
 }
 
 void
 GCMarker::endBufferingGrayRoots()
 {
-    MOZ_ASSERT(callback == GrayCallback);
+    MOZ_ASSERT(IsMarkingGray(this));
     callback = nullptr;
-    MOZ_ASSERT(IS_GC_MARKING_TRACER(this));
     MOZ_ASSERT(grayBufferState == GRAY_BUFFER_OK ||
                grayBufferState == GRAY_BUFFER_FAILED);
 }

js/src/gc/Tracer.h

@@ -353,12 +353,25 @@ class GCMarker : public JSTracer
 void
 SetMarkStackLimit(JSRuntime *rt, size_t limit);
 
+// Return true if this trace is happening on behalf of gray buffering during
+// the marking phase of incremental GC.
+inline bool
+IsMarkingGray(JSTracer *trc)
+{
+    return trc->callback == js::GCMarker::GrayCallback;
+}
+
+// Return true if this trace is happening on behalf of the marking phase of GC.
+inline bool
+IsMarkingTracer(JSTracer *trc)
+{
+    // If we call this on the gray-buffering tracer, then we have encountered a
+    // marking path that will be wrong when tracing with a callback marker to
+    // enqueue for deferred gray marking.
+    MOZ_ASSERT(!IsMarkingGray(trc));
+    return trc->callback == nullptr;
+}
+
 } /* namespace js */
 
-/*
- * Macro to test if a traversal is the marking phase of the GC.
- */
-#define IS_GC_MARKING_TRACER(trc) \
-    ((trc)->callback == nullptr || (trc)->callback == GCMarker::GrayCallback)
-
 #endif /* js_Tracer_h */

js/src/jsapi.cpp

@@ -1418,18 +1418,9 @@ JS_RemoveExtraGCRootsTracer(JSRuntime *rt, JSTraceDataOp traceOp, void *data)
 extern JS_PUBLIC_API(bool)
 JS_IsGCMarkingTracer(JSTracer *trc)
 {
-    return IS_GC_MARKING_TRACER(trc);
+    return js::IsMarkingTracer(trc);
 }
 
-#ifdef DEBUG
-extern JS_PUBLIC_API(bool)
-JS_IsMarkingGray(JSTracer *trc)
-{
-    MOZ_ASSERT(JS_IsGCMarkingTracer(trc));
-    return trc->callback == GCMarker::GrayCallback;
-}
-#endif
-
 JS_PUBLIC_API(void)
 JS_GC(JSRuntime *rt)
 {

js/src/jsapi.h

@@ -1728,12 +1728,6 @@ JS_RemoveFinalizeCallback(JSRuntime *rt, JSFinalizeCallback cb);
 extern JS_PUBLIC_API(bool)
 JS_IsGCMarkingTracer(JSTracer *trc);
 
-/* For assertions only. */
-#ifdef JS_DEBUG
-extern JS_PUBLIC_API(bool)
-JS_IsMarkingGray(JSTracer *trc);
-#endif
-
 /*
  * Weak pointers and garbage collection
  *

js/src/jsfun.cpp

@@ -744,7 +744,7 @@ JSFunction::trace(JSTracer *trc)
             // self-hosted function which can be cloned over again. The latter
             // is stored in the first extended slot.
             JSRuntime *rt = trc->runtime();
-            if (IS_GC_MARKING_TRACER(trc) &&
+            if (IsMarkingTracer(trc) &&
                 (rt->allowRelazificationForTesting || !compartment()->hasBeenEntered()) &&
                 !compartment()->isDebuggee() && !compartment()->isSelfHosting &&
                 u.i.s.script_->isRelazifiable() && (!isSelfHostedBuiltin() || isExtended()))

js/src/jsgc.cpp

@@ -3776,7 +3776,7 @@ GCRuntime::beginMarkPhase(JS::gcreason::Reason reason)
 
     marker.start();
     MOZ_ASSERT(!marker.callback);
-    MOZ_ASSERT(IS_GC_MARKING_TRACER(&marker));
+    MOZ_ASSERT(IsMarkingTracer(&marker));
 
     /* For non-incremental GC the following sweep discards the jit code. */
     if (isIncremental) {

js/src/jsscript.cpp

@@ -3382,7 +3382,7 @@ JSScript::markChildren(JSTracer *trc)
     // JSScript::Create(), but not yet finished initializing it with
     // fullyInitFromEmitter() or fullyInitTrivial().
 
-    MOZ_ASSERT_IF(IS_GC_MARKING_TRACER(trc) &&
+    MOZ_ASSERT_IF(IsMarkingTracer(trc) &&
                   static_cast<GCMarker *>(trc)->shouldCheckCompartments(),
                   zone()->isCollecting());
 
@@ -3420,7 +3420,7 @@ JSScript::markChildren(JSTracer *trc)
     if (maybeLazyScript())
         MarkLazyScriptUnbarriered(trc, &lazyScript, "lazyScript");
 
-    if (IS_GC_MARKING_TRACER(trc)) {
+    if (IsMarkingTracer(trc)) {
         compartment()->mark();
 
         if (code())

js/src/jsweakmap.cpp

@@ -46,7 +46,7 @@ void
 WeakMapBase::trace(JSTracer *tracer)
 {
     MOZ_ASSERT(isInList());
-    if (IS_GC_MARKING_TRACER(tracer)) {
+    if (IsMarkingTracer(tracer)) {
         // We don't trace any of the WeakMap entries at this time, just record
         // record the fact that the WeakMap has been marked. Enties are marked
         // in the iterative marking phase by markAllIteratively(), which happens

js/src/vm/RegExpObject.cpp

@@ -246,10 +246,10 @@ RegExpObject::trace(JSTracer *trc, JSObject *obj)
     // conditions, since:
     //   1. During TraceRuntime, isHeapBusy() is true, but the tracer might not
     //      be a marking tracer.
-    //   2. When a write barrier executes, IS_GC_MARKING_TRACER is true, but
+    //   2. When a write barrier executes, IsMarkingTracer is true, but
     //      isHeapBusy() will be false.
     if (trc->runtime()->isHeapBusy() &&
-        IS_GC_MARKING_TRACER(trc) &&
+        IsMarkingTracer(trc) &&
         !obj->asTenured().zone()->isPreservingCode())
     {
         obj->as<RegExpObject>().NativeObject::setPrivate(nullptr);
@@ -577,7 +577,7 @@ RegExpShared::~RegExpShared()
 void
 RegExpShared::trace(JSTracer *trc)
 {
-    if (IS_GC_MARKING_TRACER(trc))
+    if (IsMarkingTracer(trc))
         marked_ = true;
 
     if (source)

js/src/vm/Stack.cpp

@@ -329,7 +329,7 @@ InterpreterFrame::mark(JSTracer *trc)
     } else {
         gc::MarkScriptUnbarriered(trc, &exec.script, "script");
     }
-    if (IS_GC_MARKING_TRACER(trc))
+    if (IsMarkingTracer(trc))
         script()->compartment()->zone()->active = true;
     if (hasReturnValue())
         gc::MarkValueUnbarriered(trc, &rval_, "rval");