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 374628 5a927aeb8fb3d765371c8b24204187f57ed54a8e
parent 374627 8dd5417e8cb520d5709004a8c6b99443766da014
child 374629 772de2146034c098b70024da6a7c74b941fc2ead
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, mccr8
bugs1338623
milestone54.0a1
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