Check that each JS holder only contains GC things in a single zone draft
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 16 Aug 2019 13:14:07 +0100
changeset 2220753 f84fca9a987aeea005b6e719bdae2cc0eded0e20
parent 2220752 e97485c4751534d6b4555cfd4ea933c1630cea5c
child 2220754 82d89fa41ed1d02b0468a0a7844b68ad6ea8d0c5
push id407002
push userjcoppeard@mozilla.com
push dateFri, 16 Aug 2019 13:32:05 +0000
treeherdertry@5b7007ce4796 [default view] [failures only]
milestone70.0a1
Check that each JS holder only contains GC things in a single zone
xpcom/base/CycleCollectedJSRuntime.cpp
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -1070,30 +1070,130 @@ struct JsGcTracer : public TraceCallback
 };
 
 void mozilla::TraceScriptHolder(nsISupports* aHolder, JSTracer* aTracer) {
   nsXPCOMCycleCollectionParticipant* participant = nullptr;
   CallQueryInterface(aHolder, &participant);
   participant->Trace(aHolder, JsGcTracer(), aTracer);
 }
 
+#ifdef DEBUG
+// A tracer that checks that a holder only holds JS GC things in a single
+// JS::Zone.
+struct CheckZoneTracer : public TraceCallbacks {
+  const char* mClassName;
+  mutable JS::Zone *mZone = nullptr;
+
+  CheckZoneTracer(const char* aClassName)
+      : mClassName(aClassName) {}
+
+  void checkZone(JS::Zone* aZone, const char* aName) const {
+    if (!mZone) {
+      mZone = aZone;
+      return;
+    }
+
+    if (aZone == mZone) {
+      return;
+    }
+
+    MOZ_CRASH_UNSAFE_PRINTF(
+        "JS holder %s contains pointers to GC things in more than one zone: "
+        "found in %s",
+        mClassName, aName);
+  }
+
+  virtual void Trace(JS::Heap<JS::Value>* aPtr, const char* aName,
+                     void* aClosure) const override {
+    JS::Value value = aPtr->unbarrieredGet();
+    if (value.isObject()) {
+      checkZone(JS::GetObjectZone(&value.toObject()), aName);
+    } else if (value.isString()) {
+      checkZone(JS::GetStringZone(value.toString()), aName);
+    } else if (value.isGCThing()) {
+      checkZone(JS::GetTenuredGCThingZone(value.toGCCellPtr()), aName);
+    }
+  }
+  virtual void Trace(JS::Heap<jsid>* aPtr, const char* aName,
+                     void* aClosure) const override {
+    jsid id = aPtr->unbarrieredGet();
+    if (JSID_IS_GCTHING(id)) {
+      checkZone(JS::GetTenuredGCThingZone(JSID_TO_GCTHING(id)), aName);
+    }
+  }
+  virtual void Trace(JS::Heap<JSObject*>* aPtr, const char* aName,
+                     void* aClosure) const override {
+    JSObject* obj = aPtr->unbarrieredGet();
+    if (obj) {
+      checkZone(JS::GetObjectZone(obj), aName);
+    }
+  }
+  virtual void Trace(JSObject** aPtr, const char* aName,
+                     void* aClosure) const override {
+    JSObject* obj = *aPtr;
+    if (obj) {
+      checkZone(JS::GetObjectZone(obj), aName);
+    }
+  }
+  virtual void Trace(JS::TenuredHeap<JSObject*>* aPtr, const char* aName,
+                     void* aClosure) const override {
+    JSObject* obj = aPtr->unbarrieredGetPtr();
+    if (obj) {
+      checkZone(JS::GetObjectZone(obj), aName);
+    }
+  }
+  virtual void Trace(JS::Heap<JSString*>* aPtr, const char* aName,
+                     void* aClosure) const override {
+    JSString* str = aPtr->unbarrieredGet();
+    if (str) {
+      checkZone(JS::GetStringZone(str), aName);
+    }
+  }
+  virtual void Trace(JS::Heap<JSScript*>* aPtr, const char* aName,
+                     void* aClosure) const override {
+    JSScript* script = aPtr->unbarrieredGet();
+    if (script) {
+      checkZone(JS::GetTenuredGCThingZone(JS::GCCellPtr(script)), aName);
+    }
+  }
+  virtual void Trace(JS::Heap<JSFunction*>* aPtr, const char* aName,
+                     void* aClosure) const override {
+    JSFunction* fun = aPtr->unbarrieredGet();
+    if (fun) {
+      checkZone(JS::GetObjectZone(JS_GetFunctionObject(fun)), aName);
+    }
+  }
+};
+
+#endif
+
+static inline void CheckHolderIsSingleZone(void* aHolder,
+                                           nsCycleCollectionParticipant* aParticipant) {
+#ifdef DEBUG
+  CheckZoneTracer tracer(aParticipant->ClassName());
+  aParticipant->Trace(aHolder, tracer, nullptr);
+#endif
+}
+
 void CycleCollectedJSRuntime::TraceNativeGrayRoots(JSTracer* aTracer) {
   // NB: This is here just to preserve the existing XPConnect order. I doubt it
   // would hurt to do this after the JS holders.
   TraceAdditionalNativeGrayRoots(aTracer);
 
   for (auto iter = mJSHolders.Iter(); !iter.Done(); iter.Next()) {
     void* holder = iter.Get().mHolder;
     nsScriptObjectTracer* tracer = iter.Get().mTracer;
+    CheckHolderIsSingleZone(holder, tracer);
     tracer->Trace(holder, JsGcTracer(), aTracer);
   }
 }
 
 void CycleCollectedJSRuntime::AddJSHolder(void* aHolder,
                                           nsScriptObjectTracer* aTracer) {
+  CheckHolderIsSingleZone(aHolder, aTracer);
   mJSHolders.Put(aHolder, aTracer);
 }
 
 struct ClearJSHolder : public TraceCallbacks {
   virtual void Trace(JS::Heap<JS::Value>* aPtr, const char*,
                      void*) const override {
     aPtr->setUndefined();
   }