Bug 1478533 - Add a static_assert for WeakMap. r=jonco, mccr8

If an object is used as a WeakMap key, its TraceKind should be added
into CC graph, so adding a static_assert to make sure this.

js/public/TraceKind.h

@@ -68,6 +68,21 @@ enum class TraceKind
 };
 const static uintptr_t OutOfLineTraceKindMask = 0x07;
 
+// Returns true if the JS::TraceKind is one the cycle collector cares about.
+// Everything used as WeakMap key should be listed here, to represent the key
+// in cycle collector's graph, otherwise the key is considered to be pointed
+// from somewhere unknown, and results in leaking the subgraph which contains
+// the key.
+// See the comments in NoteWeakMapsTracer::trace for more details.
+inline constexpr bool IsCCTraceKind(JS::TraceKind aKind)
+{
+  return aKind == JS::TraceKind::Object ||
+         aKind == JS::TraceKind::Script ||
+         aKind == JS::TraceKind::LazyScript ||
+         aKind == JS::TraceKind::Scope ||
+         aKind == JS::TraceKind::RegExpShared;
+}
+
 #define ASSERT_TRACE_KIND(tk) \
     static_assert((uintptr_t(tk) & OutOfLineTraceKindMask) == OutOfLineTraceKindMask, \
         "mask bits are set")
@@ -89,7 +104,7 @@ struct MapTypeToTraceKind {
 // When this header is used outside SpiderMonkey, the class definitions are not
 // available, so the following table containing all public GC types is used.
 #define JS_FOR_EACH_TRACEKIND(D) \
- /* PrettyName       TypeName           AddToCCKind */ \
+ /* PrettyName       TypeName           IsCCTraceKind */ \
     D(BaseShape,     js::BaseShape,     true) \
     D(JitCode,       js::jit::JitCode,  true) \
     D(LazyScript,    js::LazyScript,    true) \

js/src/gc/WeakMap-inl.h

@@ -9,6 +9,7 @@
 
 #include "gc/WeakMap.h"
 
+#include "js/TraceKind.h"
 #include "vm/JSContext.h"
 
 namespace js {
@@ -28,6 +29,13 @@ template <class K, class V>
 WeakMap<K, V>::WeakMap(JSContext* cx, JSObject* memOf)
   : Base(cx->zone()), WeakMapBase(memOf, cx->zone())
 {
+    using ElemType = typename K::ElementType;
+    using NonPtrType = typename mozilla::RemovePointer<ElemType>::Type;
+    // The object's TraceKind needs to be added to CC graph if this object is
+    // used as a WeakMap key. See the comments for IsCCTraceKind for details.
+    static_assert(JS::IsCCTraceKind(NonPtrType::TraceKind),
+                  "Object's TraceKind should be added to CC graph.");
+
     zone()->gcWeakMapList().insertFront(this);
     marked = JS::IsIncrementalGCInProgress(TlsContext.get());
 }

xpcom/base/CycleCollectedJSRuntime.cpp

@@ -163,7 +163,7 @@ NoteWeakMapChildrenTracer::onChild(const JS::GCCellPtr& aThing)
     return;
   }
 
-  if (AddToCCKind(aThing.kind())) {
+  if (JS::IsCCTraceKind(aThing.kind())) {
     mCb.NoteWeakMapping(mMap, mKey, mKeyDelegate, aThing);
     mTracedAny = true;
   } else {
@@ -198,13 +198,13 @@ NoteWeakMapsTracer::trace(JSObject* aMap, JS::GCCellPtr aKey,
   // reason about the liveness of their keys, which in turn requires that
   // the key can be represented in the cycle collector graph.  All existing
   // uses of weak maps use either objects or scripts as keys, which are okay.
-  MOZ_ASSERT(AddToCCKind(aKey.kind()));
+  MOZ_ASSERT(JS::IsCCTraceKind(aKey.kind()));
 
   // As an emergency fallback for non-debug builds, if the key is not
   // representable in the cycle collector graph, we treat it as marked.  This
   // can cause leaks, but is preferable to ignoring the binding, which could
   // cause the cycle collector to free live objects.
-  if (!AddToCCKind(aKey.kind())) {
+  if (!JS::IsCCTraceKind(aKey.kind())) {
     aKey = nullptr;
   }
 
@@ -213,7 +213,7 @@ NoteWeakMapsTracer::trace(JSObject* aMap, JS::GCCellPtr aKey,
     kdelegate = js::GetWeakmapKeyDelegate(&aKey.as<JSObject>());
   }
 
-  if (AddToCCKind(aValue.kind())) {
+  if (JS::IsCCTraceKind(aValue.kind())) {
     mCb.NoteWeakMapping(aMap, aKey, kdelegate, aValue);
   } else {
     mChildTracer.mTracedAny = false;
@@ -251,7 +251,7 @@ ShouldWeakMappingEntryBeBlack(JSObject* aMap, JS::GCCellPtr aKey, JS::GCCellPtr
     return;
   }
 
-  if (!AddToCCKind(aKey.kind())) {
+  if (!JS::IsCCTraceKind(aKey.kind())) {
     aKey = nullptr;
   }
 
@@ -357,7 +357,7 @@ CheckParticipatesInCycleCollection(JS::GCCellPtr aThing, const char* aName,
     return;
   }
 
-  if (AddToCCKind(aThing.kind()) && JS::GCThingIsMarkedGray(aThing)) {
+  if (JS::IsCCTraceKind(aThing.kind()) && JS::GCThingIsMarkedGray(aThing)) {
     *cycleCollectionEnabled = true;
   }
 }
@@ -421,12 +421,12 @@ TraversalTracer::onChild(const JS::GCCellPtr& aThing)
 
   /*
    * This function needs to be careful to avoid stack overflow. Normally, when
-   * AddToCCKind is true, the recursion terminates immediately as we just add
+   * IsCCTraceKind is true, the recursion terminates immediately as we just add
    * |thing| to the CC graph. So overflow is only possible when there are long
-   * or cyclic chains of non-AddToCCKind GC things. Places where this can occur
+   * or cyclic chains of non-IsCCTraceKind GC things. Places where this can occur
    * use special APIs to handle such chains iteratively.
    */
-  if (AddToCCKind(aThing.kind())) {
+  if (JS::IsCCTraceKind(aThing.kind())) {
     if (MOZ_UNLIKELY(mCb.WantDebugInfo())) {
       char buffer[200];
       getTracingEdgeName(buffer, sizeof(buffer));

xpcom/base/CycleCollectedJSRuntime.h

@@ -17,6 +17,7 @@
 #include "mozilla/SegmentedVector.h"
 #include "jsapi.h"
 #include "jsfriendapi.h"
+#include "js/TraceKind.h"
 
 #include "nsCycleCollectionParticipant.h"
 #include "nsDataHashtable.h"
@@ -412,21 +413,6 @@ private:
 
 void TraceScriptHolder(nsISupports* aHolder, JSTracer* aTracer);
 
-// Returns true if the JS::TraceKind is one the cycle collector cares about.
-// Everything used as WeakMap key should be listed here, to represent the key
-// in cycle collector's graph, otherwise the key is considered to be pointed
-// from somewhere unknown, and results in leaking the subgraph which contains
-// the key.
-// See the comments in NoteWeakMapsTracer::trace for more details.
-inline bool AddToCCKind(JS::TraceKind aKind)
-{
-  return aKind == JS::TraceKind::Object ||
-         aKind == JS::TraceKind::Script ||
-         aKind == JS::TraceKind::LazyScript ||
-         aKind == JS::TraceKind::Scope ||
-         aKind == JS::TraceKind::RegExpShared;
-}
-
 } // namespace mozilla
 
 #endif // mozilla_CycleCollectedJSRuntime_h

xpcom/base/nsCycleCollector.cpp

@@ -2062,14 +2062,14 @@ nsCycleCollector_createLogger()
 static bool
 GCThingIsGrayCCThing(JS::GCCellPtr thing)
 {
-    return AddToCCKind(thing.kind()) &&
+    return JS::IsCCTraceKind(thing.kind()) &&
            JS::GCThingIsMarkedGray(thing);
 }
 
 static bool
 ValueIsGrayCCThing(const JS::Value& value)
 {
-    return AddToCCKind(value.traceKind()) &&
+    return JS::IsCCTraceKind(value.traceKind()) &&
            JS::GCThingIsMarkedGray(value.toGCCellPtr());
 }
 

xpcom/base/nsCycleCollectorTraceJSHelpers.cpp

@@ -28,7 +28,7 @@ nsCycleCollectionParticipant::NoteJSChild(JS::GCCellPtr aGCThing,
   nsCycleCollectionTraversalCallback* cb =
     static_cast<nsCycleCollectionTraversalCallback*>(aClosure);
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*cb, aName);
-  if (mozilla::AddToCCKind(aGCThing.kind())) {
+  if (JS::IsCCTraceKind(aGCThing.kind())) {
     cb->NoteJSChild(aGCThing);
   }
 }