Bug 1338623 - Add a slower but more exact gray marking check for checking correctness r=sfink r=mccr8
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 02 Mar 2017 10:22:47 +0000
changeset 392243 5a927aeb8fb3d765371c8b24204187f57ed54a8e
parent 392242 8dd5417e8cb520d5709004a8c6b99443766da014
child 392244 772de2146034c098b70024da6a7c74b941fc2ead
push id7198
push userjlorenzo@mozilla.com
push dateTue, 18 Apr 2017 12:07:49 +0000
treeherdermozilla-beta@d57aa49c3948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, mccr8
bugs1338623
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1338623 - Add a slower but more exact gray marking check for checking correctness r=sfink r=mccr8
dom/bindings/BindingUtils.h
dom/bindings/CallbackObject.h
dom/bindings/Codegen.py
js/public/CallArgs.h
js/public/HeapAPI.h
js/public/RootingAPI.h
js/public/Value.h
js/src/gc/Barrier.cpp
js/src/gc/Barrier.h
js/src/gc/Marking.cpp
js/src/gc/Marking.h
js/src/jsapi-tests/testGCGrayMarking.cpp
js/src/jscntxtinlines.h
js/src/jscompartment.cpp
js/src/jscompartmentinlines.h
js/src/jsfun.h
js/src/jsgc.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1526,29 +1526,29 @@ struct WrapNativeHelper
 {
   static inline JSObject* Wrap(JSContext* cx, T* parent, nsWrapperCache* cache)
   {
     MOZ_ASSERT(cache);
 
     JSObject* obj;
     if ((obj = cache->GetWrapper())) {
       // GetWrapper always unmarks gray.
-      MOZ_ASSERT(!JS::ObjectIsMarkedGray(obj));
+      MOZ_ASSERT(JS::ObjectIsNotGray(obj));
       return obj;
     }
 
     // Inline this here while we have non-dom objects in wrapper caches.
     if (!CouldBeDOMBinding(parent)) {
       // WrapNativeFallback never returns a gray thing.
       obj = WrapNativeFallback<T>::Wrap(cx, parent, cache);
-      MOZ_ASSERT_IF(obj, !JS::ObjectIsMarkedGray(obj));
+      MOZ_ASSERT(JS::ObjectIsNotGray(obj));
     } else {
       // WrapObject never returns a gray thing.
       obj = parent->WrapObject(cx, nullptr);
-      MOZ_ASSERT_IF(obj, !JS::ObjectIsMarkedGray(obj));
+      MOZ_ASSERT(JS::ObjectIsNotGray(obj));
     }
 
     return obj;
   }
 };
 
 // Wrapping of our native parent, for cases when it's not a WebIDL object.  In
 // this case it must be nsISupports.
@@ -1560,22 +1560,22 @@ struct WrapNativeHelper<T, false>
     JSObject* obj;
     if (cache && (obj = cache->GetWrapper())) {
 #ifdef DEBUG
       JS::Rooted<JSObject*> rootedObj(cx, obj);
       NS_ASSERTION(WrapNativeISupports(cx, parent, cache) == rootedObj,
                    "Unexpected object in nsWrapperCache");
       obj = rootedObj;
 #endif
-      MOZ_ASSERT(!JS::ObjectIsMarkedGray(obj));
+      MOZ_ASSERT(JS::ObjectIsNotGray(obj));
       return obj;
     }
 
     obj = WrapNativeISupports(cx, parent, cache);
-    MOZ_ASSERT_IF(obj, !JS::ObjectIsMarkedGray(obj));
+    MOZ_ASSERT(JS::ObjectIsNotGray(obj));
     return obj;
   }
 };
 
 // Finding the associated global for an object.
 template<typename T>
 static inline JSObject*
 FindAssociatedGlobal(JSContext* cx, T* p, nsWrapperCache* cache,
@@ -1584,34 +1584,34 @@ FindAssociatedGlobal(JSContext* cx, T* p
   if (!p) {
     return JS::CurrentGlobalOrNull(cx);
   }
 
   JSObject* obj = WrapNativeHelper<T>::Wrap(cx, p, cache);
   if (!obj) {
     return nullptr;
   }
-  MOZ_ASSERT(!JS::ObjectIsMarkedGray(obj));
+  MOZ_ASSERT(JS::ObjectIsNotGray(obj));
 
   obj = js::GetGlobalForObjectCrossCompartment(obj);
 
   if (!useXBLScope) {
     return obj;
   }
 
   // If useXBLScope is true, it means that the canonical reflector for this
   // native object should live in the content XBL scope. Note that we never put
   // anonymous content inside an add-on scope.
   if (xpc::IsInContentXBLScope(obj)) {
     return obj;
   }
   JS::Rooted<JSObject*> rootedObj(cx, obj);
   JSObject* xblScope = xpc::GetXBLScope(cx, rootedObj);
   MOZ_ASSERT_IF(xblScope, JS_IsGlobalObject(xblScope));
-  MOZ_ASSERT_IF(xblScope, !JS::ObjectIsMarkedGray(xblScope));
+  MOZ_ASSERT(JS::ObjectIsNotGray(xblScope));
   return xblScope;
 }
 
 // Finding of the associated global for an object, when we don't want to
 // explicitly pass in things like the nsWrapperCache for it.
 template<typename T>
 static inline JSObject*
 FindAssociatedGlobal(JSContext* cx, const T& p)
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -121,17 +121,17 @@ public:
 
   /*
    * If the callback is known to be non-gray, then this method can be
    * used instead of CallbackOrNull() to avoid the overhead of
    * ExposeObjectToActiveJS().
    */
   JS::Handle<JSObject*> CallbackKnownNotGray() const
   {
-    MOZ_ASSERT(!JS::ObjectIsMarkedGray(mCallback));
+    MOZ_ASSERT(JS::ObjectIsNotGray(mCallback));
     return CallbackPreserveColor();
   }
 
   nsIGlobalObject* IncumbentGlobalOrNull() const
   {
     return mIncumbentGlobal;
   }
 
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3247,17 +3247,17 @@ class CGGetPerInterfaceObject(CGAbstract
              * traced by TraceProtoAndIfaceCache() and its contents are never
              * changed after they have been set.
              *
              * Calling address() avoids the read read barrier that does gray
              * unmarking, but it's not possible for the object to be gray here.
              */
 
             const JS::Heap<JSObject*>& entrySlot = protoAndIfaceCache.EntrySlotMustExist(${id});
-            MOZ_ASSERT_IF(entrySlot, !JS::ObjectIsMarkedGray(entrySlot));
+            MOZ_ASSERT(JS::ObjectIsNotGray(entrySlot));
             return JS::Handle<JSObject*>::fromMarkedLocation(entrySlot.address());
             """,
             id=self.id)
 
 
 class CGGetProtoObjectHandleMethod(CGGetPerInterfaceObject):
     """
     A method for getting the interface prototype object.
@@ -3764,17 +3764,17 @@ class CGWrapWithCacheMethod(CGAbstractMe
             MOZ_ASSERT(ToSupportsIsOnPrimaryInheritanceChain(aObject, aCache),
                        "nsISupports must be on our primary inheritance chain");
 
             JS::Rooted<JSObject*> global(aCx, FindAssociatedGlobal(aCx, aObject->GetParentObject()));
             if (!global) {
               return false;
             }
             MOZ_ASSERT(JS_IsGlobalObject(global));
-            MOZ_ASSERT(!JS::ObjectIsMarkedGray(global));
+            MOZ_ASSERT(JS::ObjectIsNotGray(global));
 
             // That might have ended up wrapping us already, due to the wonders
             // of XBL.  Check for that, and bail out as needed.
             aReflector.set(aCache->GetWrapper());
             if (aReflector) {
             #ifdef DEBUG
               binding_detail::AssertReflectorHasGivenProto(aCx, aReflector, aGivenProto);
             #endif // DEBUG
--- a/js/public/CallArgs.h
+++ b/js/public/CallArgs.h
@@ -285,17 +285,17 @@ class MOZ_STACK_CLASS CallArgs : public 
     static CallArgs create(unsigned argc, Value* argv, bool constructing) {
         CallArgs args;
         args.clearUsedRval();
         args.argv_ = argv;
         args.argc_ = argc;
         args.constructing_ = constructing;
 #ifdef DEBUG
         for (unsigned i = 0; i < argc; ++i)
-            MOZ_ASSERT_IF(argv[i].isGCThing(), !GCThingIsMarkedGray(GCCellPtr(argv[i])));
+            MOZ_ASSERT(ValueIsNotGray(argv[i]));
 #endif
         return args;
     }
 
   public:
     /*
      * Returns true if there are at least |required| arguments passed in. If
      * false, it reports an error message on the context.
--- a/js/public/HeapAPI.h
+++ b/js/public/HeapAPI.h
@@ -305,16 +305,21 @@ CellIsMarkedGray(const Cell* cell)
     uintptr_t* word, mask;
     js::gc::detail::GetGCThingMarkWordAndMask(uintptr_t(cell), js::gc::GRAY, &word, &mask);
     return *word & mask;
 }
 
 extern JS_PUBLIC_API(bool)
 CellIsMarkedGrayIfKnown(const Cell* cell);
 
+#ifdef DEBUG
+extern JS_PUBLIC_API(bool)
+CellIsNotGray(const Cell* cell);
+#endif
+
 } /* namespace detail */
 
 MOZ_ALWAYS_INLINE bool
 IsInsideNursery(const js::gc::Cell* cell)
 {
     if (!cell)
         return false;
     uintptr_t addr = uintptr_t(cell);
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -318,29 +318,41 @@ ObjectIsMarkedGray(JSObject* obj)
 }
 
 static MOZ_ALWAYS_INLINE bool
 ObjectIsMarkedGray(const JS::Heap<JSObject*>& obj)
 {
     return ObjectIsMarkedGray(obj.unbarrieredGet());
 }
 
-static MOZ_ALWAYS_INLINE bool
-ScriptIsMarkedGray(JSScript* script)
+// The following *IsNotGray functions are for use in assertions and take account
+// of the eventual gray marking state at the end of any ongoing incremental GC.
+#ifdef DEBUG
+inline bool
+CellIsNotGray(js::gc::Cell* maybeCell)
 {
-    auto cell = reinterpret_cast<js::gc::Cell*>(script);
-    return js::gc::detail::CellIsMarkedGrayIfKnown(cell);
+    if (!maybeCell)
+        return true;
+
+    return js::gc::detail::CellIsNotGray(maybeCell);
 }
 
-static MOZ_ALWAYS_INLINE bool
-ScriptIsMarkedGray(const Heap<JSScript*>& script)
+inline bool
+ObjectIsNotGray(JSObject* maybeObj)
 {
-    return ScriptIsMarkedGray(script.unbarrieredGet());
+    return CellIsNotGray(reinterpret_cast<js::gc::Cell*>(maybeObj));
 }
 
+inline bool
+ObjectIsNotGray(const JS::Heap<JSObject*>& obj)
+{
+    return ObjectIsNotGray(obj.unbarrieredGet());
+}
+#endif
+
 /**
  * The TenuredHeap<T> class is similar to the Heap<T> class above in that it
  * encapsulates the GC concerns of an on-heap reference to a JS object. However,
  * it has two important differences:
  *
  *  1) Pointers which are statically known to only reference "tenured" objects
  *     can avoid the extra overhead of SpiderMonkey's write barriers.
  *
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -1447,16 +1447,37 @@ DispatchTyped(F f, const JS::Value& val,
 }
 
 template <class S> struct VoidDefaultAdaptor { static void defaultValue(const S&) {} };
 template <class S> struct IdentityDefaultAdaptor { static S defaultValue(const S& v) {return v;} };
 template <class S, bool v> struct BoolDefaultAdaptor { static bool defaultValue(const S&) { return v; } };
 
 } // namespace js
 
+#ifdef DEBUG
+namespace JS {
+
+MOZ_ALWAYS_INLINE bool
+ValueIsNotGray(const Value& value)
+{
+    if (!value.isGCThing())
+        return true;
+
+    return CellIsNotGray(value.toGCThing());
+}
+
+MOZ_ALWAYS_INLINE bool
+ValueIsNotGray(const Heap<Value>& value)
+{
+    return ValueIsNotGray(value.unbarrieredGet());
+}
+
+} // namespace JS
+#endif
+
 /************************************************************************/
 
 namespace JS {
 
 extern JS_PUBLIC_DATA(const HandleValue) NullHandleValue;
 extern JS_PUBLIC_DATA(const HandleValue) UndefinedHandleValue;
 extern JS_PUBLIC_DATA(const HandleValue) TrueHandleValue;
 extern JS_PUBLIC_DATA(const HandleValue) FalseHandleValue;
--- a/js/src/gc/Barrier.cpp
+++ b/js/src/gc/Barrier.cpp
@@ -26,17 +26,17 @@ RuntimeFromActiveCooperatingThreadIsHeap
 {
     MOZ_ASSERT(CurrentThreadCanAccessRuntime(shadowZone->runtimeFromActiveCooperatingThread()));
     return JS::CurrentThreadIsHeapMajorCollecting();
 }
 
 #ifdef DEBUG
 
 bool
-IsMarkedBlack(NativeObject* obj)
+IsMarkedBlack(JSObject* obj)
 {
     // Note: we assume conservatively that Nursery things will be live.
     if (!obj->isTenured())
         return true;
 
     gc::TenuredCell& tenured = obj->asTenured();
     if (tenured.isMarked(gc::BLACK) || tenured.arena()->allocatedDuringIncremental)
         return true;
@@ -56,18 +56,17 @@ void
 HeapSlot::assertPreconditionForWriteBarrierPost(NativeObject* obj, Kind kind, uint32_t slot,
                                                 const Value& target) const
 {
     if (kind == Slot)
         MOZ_ASSERT(obj->getSlotAddressUnchecked(slot)->get() == target);
     else
         MOZ_ASSERT(static_cast<HeapSlot*>(obj->getDenseElements() + slot)->get() == target);
 
-    MOZ_ASSERT_IF(target.isGCThing() && IsMarkedBlack(obj),
-                  !JS::GCThingIsMarkedGray(JS::GCCellPtr(target)));
+    CheckEdgeIsNotBlackToGray(obj, target);
 }
 
 bool
 CurrentThreadIsIonCompiling()
 {
     return TlsContext.get()->ionCompiling;
 }
 
--- a/js/src/gc/Barrier.h
+++ b/js/src/gc/Barrier.h
@@ -237,19 +237,25 @@ CurrentThreadIsIonCompiling();
 
 bool
 CurrentThreadIsIonCompilingSafeForMinorGC();
 
 bool
 CurrentThreadIsGCSweeping();
 
 bool
-IsMarkedBlack(NativeObject* obj);
+IsMarkedBlack(JSObject* obj);
 #endif
 
+MOZ_ALWAYS_INLINE void
+CheckEdgeIsNotBlackToGray(JSObject* src, const Value& dst)
+{
+    MOZ_ASSERT_IF(IsMarkedBlack(src), JS::ValueIsNotGray(dst));
+}
+
 template <typename T>
 struct InternalBarrierMethods {};
 
 template <typename T>
 struct InternalBarrierMethods<T*>
 {
     static bool isMarkable(T* v) { return v != nullptr; }
 
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2249,17 +2249,17 @@ MarkStackIter::position() const
 }
 
 inline bool
 MarkStackIter::done() const
 {
     return position() == 0;
 }
 
-inline const MarkStack::TaggedPtr&
+inline MarkStack::TaggedPtr
 MarkStackIter::peekPtr() const
 {
     MOZ_ASSERT(!done());
     return pos_[-1];
 }
 
 inline MarkStack::Tag
 MarkStackIter::peekTag() const
@@ -2282,16 +2282,25 @@ inline void
 MarkStackIter::nextPtr()
 {
     MOZ_ASSERT(!done());
     MOZ_ASSERT(!TagIsArrayTag(peekTag()));
     pos_--;
 }
 
 inline void
+MarkStackIter::next()
+{
+    if (TagIsArrayTag(peekTag()))
+        nextArray();
+    else
+        nextPtr();
+}
+
+inline void
 MarkStackIter::nextArray()
 {
     MOZ_ASSERT(TagIsArrayTag(peekTag()));
     MOZ_ASSERT(position() >= ValueArrayWords);
     pos_ -= ValueArrayWords;
 }
 
 void
@@ -2554,16 +2563,39 @@ size_t
 GCMarker::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
     size_t size = stack.sizeOfExcludingThis(mallocSizeOf);
     for (ZonesIter zone(runtime(), WithAtoms); !zone.done(); zone.next())
         size += zone->gcGrayRoots().sizeOfExcludingThis(mallocSizeOf);
     return size;
 }
 
+#ifdef DEBUG
+Zone*
+GCMarker::stackContainsCrossZonePointerTo(const TenuredCell* target) const
+{
+    MOZ_ASSERT(!JS::CurrentThreadIsHeapCollecting());
+
+    for (MarkStackIter iter(stack); !iter.done(); iter.next()) {
+        if (iter.peekTag() != MarkStack::ObjectTag)
+            continue;
+
+        auto source = iter.peekPtr().as<JSObject>();
+        if (source->is<ProxyObject>() &&
+            source->as<ProxyObject>().target() == static_cast<const Cell*>(target) &&
+            source->zone() != target->zone())
+        {
+            return source->zone();
+        }
+    }
+
+    return nullptr;
+}
+#endif // DEBUG
+
 
 /*** Tenuring Tracer *****************************************************************************/
 
 namespace js {
 template <typename T>
 void
 TenuringTracer::traverse(T** tp)
 {
--- a/js/src/gc/Marking.h
+++ b/js/src/gc/Marking.h
@@ -188,26 +188,27 @@ class MarkStackIter
     MarkStack::TaggedPtr* pos_;
 
   public:
     explicit MarkStackIter(const MarkStack& stack);
     ~MarkStackIter();
 
     bool done() const;
     MarkStack::Tag peekTag() const;
+    MarkStack::TaggedPtr peekPtr() const;
     MarkStack::ValueArray peekValueArray() const;
+    void next();
     void nextPtr();
     void nextArray();
 
     // Mutate the current ValueArray to a SavedValueArray.
     void saveValueArray(NativeObject* obj, uintptr_t index, HeapSlot::Kind kind);
 
   private:
     size_t position() const;
-    const MarkStack::TaggedPtr& peekPtr() const;
 };
 
 struct WeakKeyTableHashPolicy {
     typedef JS::GCCellPtr Lookup;
     static HashNumber hash(const Lookup& v, const mozilla::HashCodeScrambler&) {
         return mozilla::HashGeneric(v.asCell());
     }
     static bool match(const JS::GCCellPtr& k, const Lookup& l) { return k == l; }
@@ -295,17 +296,21 @@ class GCMarker : public JSTracer
 
     MOZ_MUST_USE bool drainMarkStack(SliceBudget& budget);
 
     void setGCMode(JSGCMode mode) { stack.setGCMode(mode); }
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 #ifdef DEBUG
+
     bool shouldCheckCompartments() { return strictCompartmentChecking; }
+
+    Zone* stackContainsCrossZonePointerTo(const gc::TenuredCell* cell) const;
+
 #endif
 
     void markEphemeronValues(gc::Cell* markedCell, gc::WeakEntryVector& entry);
 
     static GCMarker* fromTracer(JSTracer* trc) {
         MOZ_ASSERT(trc->isMarkingTracer());
         return static_cast<GCMarker*>(trc);
     }
--- a/js/src/jsapi-tests/testGCGrayMarking.cpp
+++ b/js/src/jsapi-tests/testGCGrayMarking.cpp
@@ -99,28 +99,32 @@ TestMarking()
 
     JS_GC(cx);
 
     CHECK(IsMarkedBlack(sameSource));
     CHECK(IsMarkedBlack(crossSource));
     CHECK(IsMarkedBlack(sameTarget));
     CHECK(IsMarkedBlack(crossTarget));
 
+    CHECK(!JS::ObjectIsMarkedGray(sameSource));
+
     // Test GC with gray roots marks object gray.
 
     blackRoot1 = nullptr;
     blackRoot2 = nullptr;
 
     JS_GC(cx);
 
     CHECK(IsMarkedGray(sameSource));
     CHECK(IsMarkedGray(crossSource));
     CHECK(IsMarkedGray(sameTarget));
     CHECK(IsMarkedGray(crossTarget));
 
+    CHECK(JS::ObjectIsMarkedGray(sameSource));
+
     // Test ExposeToActiveJS marks gray objects black.
 
     JS::ExposeObjectToActiveJS(sameSource);
     JS::ExposeObjectToActiveJS(crossSource);
     CHECK(IsMarkedBlack(sameSource));
     CHECK(IsMarkedBlack(crossSource));
     CHECK(IsMarkedBlack(sameTarget));
     CHECK(IsMarkedBlack(crossTarget));
@@ -473,21 +477,22 @@ TestWatchpoints()
     grayRoots.grayRoot2 = nullptr;
 
     return true;
 }
 
 bool
 TestCCWs()
 {
-    RootedObject target(cx, AllocPlainObject());
+    JSObject* target = AllocPlainObject();
     CHECK(target);
 
     // Test getting a new wrapper doesn't return a gray wrapper.
 
+    RootedObject blackRoot(cx, target);
     JSObject* wrapper = GetCrossCompartmentWrapper(target);
     CHECK(wrapper);
     CHECK(!IsMarkedGray(wrapper));
 
     // Test getting an existing wrapper doesn't return a gray wrapper.
 
     grayRoots.grayRoot1 = wrapper;
     grayRoots.grayRoot2 = nullptr;
@@ -514,30 +519,66 @@ TestCCWs()
     CHECK(!IsMarkedBlack(wrapper));
     CHECK(wrapper->zone()->isGCMarkingBlack());
 
     CHECK(GetCrossCompartmentWrapper(target) == wrapper);
     CHECK(IsMarkedBlack(wrapper));
 
     JS::FinishIncrementalGC(cx, JS::gcreason::API);
 
-    target = nullptr;
+    // Test behaviour of gray CCWs marked black by a barrier during incremental
+    // GC.
+
+    // Initial state: source and target are gray.
+    blackRoot = nullptr;
+    grayRoots.grayRoot1 = wrapper;
+    grayRoots.grayRoot2 = nullptr;
+    JS_GC(cx);
+    CHECK(IsMarkedGray(wrapper));
+    CHECK(IsMarkedGray(target));
+
+    // Incremental zone GC started: the source is now unmarked.
+    JS_SetGCParameter(cx, JSGC_MODE, JSGC_MODE_INCREMENTAL);
+    JS::PrepareZoneForGC(wrapper->zone());
+    budget = js::SliceBudget(js::WorkBudget(1));
+    cx->runtime()->gc.startDebugGC(GC_NORMAL, budget);
+    CHECK(JS::IsIncrementalGCInProgress(cx));
+    CHECK(wrapper->zone()->isGCMarkingBlack());
+    CHECK(!target->zone()->wasGCStarted());
+    CHECK(!IsMarkedBlack(wrapper));
+    CHECK(!IsMarkedGray(wrapper));
+    CHECK(IsMarkedGray(target));
+
+    // Betweeen GC slices: source marked black by barrier, target is still gray.
+    // ObjectIsMarkedGray() and CheckObjectIsNotMarkedGray() should handle this
+    // case and report that target is not marked gray.
+    grayRoots.grayRoot1.get();
+    CHECK(IsMarkedBlack(wrapper));
+    CHECK(IsMarkedGray(target));
+    CHECK(!JS::ObjectIsMarkedGray(target));
+    MOZ_ASSERT(JS::ObjectIsNotGray(target));
+
+    // Final state: source and target are black.
+    JS::FinishIncrementalGC(cx, JS::gcreason::API);
+    CHECK(IsMarkedBlack(wrapper));
+    CHECK(IsMarkedBlack(target));
+
     grayRoots.grayRoot1 = nullptr;
     grayRoots.grayRoot2 = nullptr;
 
     return true;
 }
 
 JS::PersistentRootedObject global1;
 JS::PersistentRootedObject global2;
 
 struct GrayRoots
 {
-    JSObject* grayRoot1;
-    JSObject* grayRoot2;
+    JS::Heap<JSObject*> grayRoot1;
+    JS::Heap<JSObject*> grayRoot2;
 };
 
 GrayRoots grayRoots;
 
 bool
 InitGlobals()
 {
     global1.init(cx, global);
@@ -562,20 +603,18 @@ RemoveGrayRootTracer()
     grayRoots.grayRoot2 = nullptr;
     JS_SetGrayGCRootsTracer(cx, nullptr, nullptr);
 }
 
 static void
 TraceGrayRoots(JSTracer* trc, void* data)
 {
     auto grayRoots = static_cast<GrayRoots*>(data);
-    if (grayRoots->grayRoot1)
-        UnsafeTraceManuallyBarrieredEdge(trc, &grayRoots->grayRoot1, "gray root 1");
-    if (grayRoots->grayRoot2)
-        UnsafeTraceManuallyBarrieredEdge(trc, &grayRoots->grayRoot2, "gray root 2");
+    TraceEdge(trc, &grayRoots->grayRoot1, "gray root 1");
+    TraceEdge(trc, &grayRoots->grayRoot2, "gray root 2");
 }
 
 JSObject*
 AllocPlainObject()
 {
     JS::RootedObject obj(cx, JS_NewPlainObject(cx));
     EvictNursery();
 
--- a/js/src/jscntxtinlines.h
+++ b/js/src/jscntxtinlines.h
@@ -454,17 +454,17 @@ JSContext::enterCompartment(
     c->enter();
     setCompartment(c, maybeLock);
 }
 
 template <typename T>
 inline void
 JSContext::enterCompartmentOf(const T& target)
 {
-    MOZ_ASSERT(!js::gc::detail::CellIsMarkedGrayIfKnown(target));
+    MOZ_ASSERT(JS::CellIsNotGray(target));
     enterCompartment(target->compartment(), nullptr);
 }
 
 inline void
 JSContext::enterNullCompartment()
 {
     enterCompartmentDepth_++;
     setCompartment(nullptr);
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -457,17 +457,17 @@ JSCompartment::wrap(JSContext* cx, Mutab
 
     if (!obj)
         return true;
 
     AutoDisableProxyCheck adpc;
 
     // Anything we're wrapping has already escaped into script, so must have
     // been unmarked-gray at some point in the past.
-    MOZ_ASSERT(!ObjectIsMarkedGray(obj));
+    MOZ_ASSERT(JS::ObjectIsNotGray(obj));
 
     // The passed object may already be wrapped, or may fit a number of special
     // cases that we need to check for and manually correct.
     if (!getNonWrapperObjectForCurrentCompartment(cx, obj))
         return false;
 
     // If the reification above did not result in a same-compartment object,
     // get or create a new wrapper object in this compartment for it.
--- a/js/src/jscompartmentinlines.h
+++ b/js/src/jscompartmentinlines.h
@@ -111,17 +111,17 @@ JSCompartment::wrap(JSContext* cx, JS::M
      * script. Unwrap and prewrap are both steps that we take to get to the
      * identity of an incoming objects, and as such, they shuld never map
      * one identity object to another object. This means that we can safely
      * check the cache immediately, and only risk false negatives. Do this
      * in opt builds, and do both in debug builds so that we can assert
      * that we get the same answer.
      */
 #ifdef DEBUG
-    MOZ_ASSERT(!JS::ObjectIsMarkedGray(&vp.toObject()));
+    MOZ_ASSERT(JS::ValueIsNotGray(vp));
     JS::RootedObject cacheResult(cx);
 #endif
     JS::RootedValue v(cx, vp);
     if (js::WrapperMap::Ptr p = crossCompartmentWrappers.lookup(js::CrossCompartmentKey(v))) {
 #ifdef DEBUG
         cacheResult = &p->value().get().toObject();
 #else
         vp.set(p->value().get());
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -806,28 +806,26 @@ JSFunction::initializeExtended()
     toExtended()->extendedSlots[0].init(js::UndefinedValue());
     toExtended()->extendedSlots[1].init(js::UndefinedValue());
 }
 
 inline void
 JSFunction::initExtendedSlot(size_t which, const js::Value& val)
 {
     MOZ_ASSERT(which < mozilla::ArrayLength(toExtended()->extendedSlots));
-    MOZ_ASSERT_IF(js::IsMarkedBlack(this) && val.isGCThing(),
-                  !JS::GCThingIsMarkedGray(JS::GCCellPtr(val)));
+    js::CheckEdgeIsNotBlackToGray(this, val);
     MOZ_ASSERT(js::IsObjectValueInCompartment(val, compartment()));
     toExtended()->extendedSlots[which].init(val);
 }
 
 inline void
 JSFunction::setExtendedSlot(size_t which, const js::Value& val)
 {
     MOZ_ASSERT(which < mozilla::ArrayLength(toExtended()->extendedSlots));
-    MOZ_ASSERT_IF(js::IsMarkedBlack(this) && val.isGCThing(),
-                  !JS::GCThingIsMarkedGray(JS::GCCellPtr(val)));
+    js::CheckEdgeIsNotBlackToGray(this, val);
     MOZ_ASSERT(js::IsObjectValueInCompartment(val, compartment()));
     toExtended()->extendedSlots[which] = val;
 }
 
 inline const js::Value&
 JSFunction::getExtendedSlot(size_t which) const
 {
     MOZ_ASSERT(which < mozilla::ArrayLength(toExtended()->extendedSlots));
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -7793,39 +7793,83 @@ js::gc::Cell::dump(FILE* fp) const
 // For use in a debugger.
 void
 js::gc::Cell::dump() const
 {
     dump(stderr);
 }
 #endif
 
-JS_PUBLIC_API(bool)
-js::gc::detail::CellIsMarkedGrayIfKnown(const Cell* cell)
+static inline bool
+CanCheckGrayBits(const Cell* cell)
 {
     MOZ_ASSERT(cell);
     if (!cell->isTenured())
         return false;
 
+    auto tc = &cell->asTenured();
+    auto rt = tc->runtimeFromAnyThread();
+    return CurrentThreadCanAccessRuntime(rt) && rt->gc.areGrayBitsValid();
+}
+
+JS_PUBLIC_API(bool)
+js::gc::detail::CellIsMarkedGrayIfKnown(const Cell* cell)
+{
     // We ignore the gray marking state of cells and return false in the
     // following cases:
     //
     // 1) When OOM has caused us to clear the gcGrayBitsValid_ flag.
     //
     // 2) When we are in an incremental GC and examine a cell that is in a zone
     // that is not being collected. Gray targets of CCWs that are marked black
     // by a barrier will eventually be marked black in the next GC slice.
     //
     // 3) When we are not on the runtime's active thread. Helper threads might
     // call this while parsing, and they are not allowed to inspect the
     // runtime's incremental state. The objects being operated on are not able
     // to be collected and will not be marked any color.
+
+    if (!CanCheckGrayBits(cell))
+        return false;
+
     auto tc = &cell->asTenured();
     auto rt = tc->runtimeFromAnyThread();
-    if (!CurrentThreadCanAccessRuntime(rt) ||
-        !rt->gc.areGrayBitsValid() ||
-        (rt->gc.isIncrementalGCInProgress() && !tc->zone()->wasGCStarted()))
-    {
+    if (rt->gc.isIncrementalGCInProgress() && !tc->zone()->wasGCStarted())
         return false;
-    }
 
     return detail::CellIsMarkedGray(tc);
 }
+
+#ifdef DEBUG
+JS_PUBLIC_API(bool)
+js::gc::detail::CellIsNotGray(const Cell* cell)
+{
+    // Check that a cell is not marked gray.
+    //
+    // Since this is a debug-only check, take account of the eventual mark state
+    // of cells that will be marked black by the next GC slice in an incremental
+    // GC. For performance reasons we don't do this in CellIsMarkedGrayIfKnown.
+
+    // TODO: I'd like to AssertHeapIsIdle() here, but this ends up getting
+    // called while iterating the heap for memory reporting.
+    MOZ_ASSERT(!JS::CurrentThreadIsHeapCollecting());
+    MOZ_ASSERT(!JS::CurrentThreadIsHeapCycleCollecting());
+
+    if (!CanCheckGrayBits(cell))
+        return true;
+
+    auto tc = &cell->asTenured();
+    if (!detail::CellIsMarkedGray(tc))
+        return true;
+
+    auto rt = tc->runtimeFromAnyThread();
+    Zone* sourceZone = nullptr;
+    if (rt->gc.isIncrementalGCInProgress() &&
+        !tc->zone()->wasGCStarted() &&
+        (sourceZone = rt->gc.marker.stackContainsCrossZonePointerTo(tc)) &&
+        sourceZone->wasGCStarted())
+    {
+        return true;
+    }
+
+    return false;
+}
+#endif
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -96,17 +96,17 @@ WrapperFactory::WaiveXray(JSContext* cx,
     RootedObject obj(cx, objArg);
     obj = UncheckedUnwrap(obj);
     MOZ_ASSERT(!js::IsWindow(obj));
 
     JSObject* waiver = GetXrayWaiver(obj);
     if (!waiver) {
         waiver = CreateXrayWaiver(cx, obj);
     }
-    MOZ_ASSERT(!ObjectIsMarkedGray(waiver));
+    MOZ_ASSERT(JS::ObjectIsNotGray(waiver));
     return waiver;
 }
 
 /* static */ bool
 WrapperFactory::AllowWaiver(JSCompartment* target, JSCompartment* origin)
 {
     return CompartmentPrivate::Get(target)->allowWaivers &&
            AccessCheck::subsumes(target, origin);
@@ -317,17 +317,17 @@ WrapperFactory::PrepareForWrapping(JSCon
         nsXPConnect::XPConnect()->WrapNativeToJSVal(cx, wrapScope, wn->Native(), nullptr,
                                                     &NS_GET_IID(nsISupports), false, &v);
     if (NS_FAILED(rv)) {
         return;
     }
 
     obj.set(&v.toObject());
     MOZ_ASSERT(IS_WN_REFLECTOR(obj), "bad object");
-    MOZ_ASSERT(!ObjectIsMarkedGray(obj), "Should never return gray reflectors");
+    MOZ_ASSERT(JS::ObjectIsNotGray(obj), "Should never return gray reflectors");
 
     // Because the underlying native didn't have a PreCreate hook, we had
     // to a new (or possibly pre-existing) XPCWN in our compartment.
     // This could be a problem for chrome code that passes XPCOM objects
     // across compartments, because the effects of QI would disappear across
     // compartments.
     //
     // So whenever we pull an XPCWN across compartments in this manner, we