Bug 650161 - Add an assertion that something is only ever called from a GC callback r=terrence
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 19 Sep 2014 09:57:11 +0100
changeset 206209 cbe1887ddc956de31d41c6940b7554d14cc2a42a
parent 206208 c3173fa1200bef8d50a7a8b4874900fcf0b2a286
child 206210 9737b23a2790891c6b851646a0e731b9a9e380e8
push id27516
push userryanvm@gmail.com
push dateFri, 19 Sep 2014 17:54:48 +0000
treeherdermozilla-central@b00bdb144e06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs650161
milestone35.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 650161 - Add an assertion that something is only ever called from a GC callback r=terrence
js/public/GCAPI.h
js/src/devtools/rootAnalysis/annotations.js
js/src/jsgc.cpp
js/src/jsgcinlines.h
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -388,34 +388,70 @@ class JS_PUBLIC_API(AutoAssertOnGC)
     explicit AutoAssertOnGC(JSRuntime *rt) {}
     ~AutoAssertOnGC() {}
 
     static void VerifyIsSafeToGC(JSRuntime *rt) {}
 #endif
 };
 
 /*
- * Disable the static rooting hazard analysis in the live region, but assert if
- * any GC occurs while this guard object is live. This is most useful to help
- * the exact rooting hazard analysis in complex regions, since it cannot
- * understand dataflow.
+ * Assert if an allocation of a GC thing occurs while this class is live. This
+ * class does not disable the static rooting hazard analysis.
+ */
+class JS_PUBLIC_API(AutoAssertNoAlloc)
+{
+#ifdef JS_DEBUG
+    js::gc::GCRuntime *gc;
+
+  public:
+    AutoAssertNoAlloc() : gc(nullptr) {}
+    explicit AutoAssertNoAlloc(JSRuntime *rt);
+    void disallowAlloc(JSRuntime *rt);
+    ~AutoAssertNoAlloc();
+#else
+  public:
+    AutoAssertNoAlloc() {}
+    explicit AutoAssertNoAlloc(JSRuntime *rt) {}
+    void disallowAlloc(JSRuntime *rt) {}
+#endif
+};
+
+/*
+ * Disable the static rooting hazard analysis in the live region and assert if
+ * any allocation that could potentially trigger a GC occurs while this guard
+ * object is live. This is most useful to help the exact rooting hazard analysis
+ * in complex regions, since it cannot understand dataflow.
  *
  * Note: GC behavior is unpredictable even when deterministice and is generally
  *       non-deterministic in practice. The fact that this guard has not
  *       asserted is not a guarantee that a GC cannot happen in the guarded
  *       region. As a rule, anyone performing a GC unsafe action should
  *       understand the GC properties of all code in that region and ensure
  *       that the hazard analysis is correct for that code, rather than relying
  *       on this class.
  */
-class JS_PUBLIC_API(AutoSuppressGCAnalysis) : public AutoAssertOnGC
+class JS_PUBLIC_API(AutoSuppressGCAnalysis) : public AutoAssertNoAlloc
 {
   public:
-    AutoSuppressGCAnalysis() : AutoAssertOnGC() {}
-    explicit AutoSuppressGCAnalysis(JSRuntime *rt) : AutoAssertOnGC(rt) {}
+    AutoSuppressGCAnalysis() : AutoAssertNoAlloc() {}
+    explicit AutoSuppressGCAnalysis(JSRuntime *rt) : AutoAssertNoAlloc(rt) {}
+};
+
+/*
+ * Assert that code is only ever called from a GC callback, disable the static
+ * rooting hazard analysis and assert if any allocation that could potentially
+ * trigger a GC occurs while this guard object is live.
+ *
+ * This is useful to make the static analysis ignore code that runs in GC
+ * callbacks.
+ */
+class JS_PUBLIC_API(AutoAssertGCCallback) : public AutoSuppressGCAnalysis
+{
+  public:
+    explicit AutoAssertGCCallback(JSObject *obj);
 };
 
 /*
  * Place AutoCheckCannotGC in scopes that you believe can never GC. These
  * annotations will be verified both dynamically via AutoAssertOnGC, and
  * statically with the rooting hazard analysis (implemented by making the
  * analysis consider AutoCheckCannotGC to be a GC pointer, and therefore
  * complain if it is live across a GC call.) It is useful when dealing with
--- a/js/src/devtools/rootAnalysis/annotations.js
+++ b/js/src/devtools/rootAnalysis/annotations.js
@@ -217,16 +217,17 @@ function isRootedPointerTypeName(name)
         return /\(js::AllowGC\)1u>::RootType/.test(name);
 
     return name.startsWith('Rooted') || name.startsWith('PersistentRooted');
 }
 
 function isSuppressConstructor(name)
 {
     return name.indexOf("::AutoSuppressGC") != -1
+        || name.indexOf("::AutoAssertGCCallback") != -1
         || name.indexOf("::AutoEnterAnalysis") != -1
         || name.indexOf("::AutoSuppressGCAnalysis") != -1
         || name.indexOf("::AutoIgnoreRootingHazards") != -1;
 }
 
 // nsISupports subclasses' methods may be scriptable (or overridden
 // via binary XPCOM), and so may GC. But some fields just aren't going
 // to get overridden with something that can GC.
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -5637,22 +5637,22 @@ GCRuntime::scanZonesBeforeGC()
     return zoneStats;
 }
 
 void
 GCRuntime::collect(bool incremental, int64_t budget, JSGCInvocationKind gckind,
                    JS::gcreason::Reason reason)
 {
     /* GC shouldn't be running in parallel execution mode */
-    MOZ_ASSERT(!InParallelSection());
+    MOZ_ALWAYS_TRUE(!InParallelSection());
 
     JS_AbortIfWrongThread(rt);
 
     /* If we attempt to invoke the GC while we are running in the GC, assert. */
-    MOZ_ASSERT(!rt->isHeapBusy());
+    MOZ_ALWAYS_TRUE(!rt->isHeapBusy());
 
     /* The engine never locks across anything that could GC. */
     MOZ_ASSERT(!rt->currentThreadHasExclusiveAccess());
 
     if (rt->mainThread.suppressGC)
         return;
 
     TraceLogger *logger = TraceLoggerForMainThread(rt);
@@ -6354,18 +6354,43 @@ JS::AutoAssertOnGC::~AutoAssertOnGC()
 }
 
 /* static */ void
 JS::AutoAssertOnGC::VerifyIsSafeToGC(JSRuntime *rt)
 {
     if (rt->gc.isInsideUnsafeRegion())
         MOZ_CRASH("[AutoAssertOnGC] possible GC in GC-unsafe region");
 }
+
+JS::AutoAssertNoAlloc::AutoAssertNoAlloc(JSRuntime *rt)
+  : gc(nullptr)
+{
+    disallowAlloc(rt);
+}
+
+void JS::AutoAssertNoAlloc::disallowAlloc(JSRuntime *rt)
+{
+    JS_ASSERT(!gc);
+    gc = &rt->gc;
+    gc->disallowAlloc();
+}
+
+JS::AutoAssertNoAlloc::~AutoAssertNoAlloc()
+{
+    if (gc)
+        gc->allowAlloc();
+}
 #endif
 
+JS::AutoAssertGCCallback::AutoAssertGCCallback(JSObject *obj)
+  : AutoSuppressGCAnalysis()
+{
+    MOZ_ASSERT(obj->runtimeFromMainThread()->isHeapMajorCollecting());
+}
+
 #ifdef JSGC_HASH_TABLE_CHECKS
 void
 js::gc::CheckHashTablesAfterMovingGC(JSRuntime *rt)
 {
     /*
      * Check that internal hash tables no longer have any pointers to things
      * that have been moved.
      */
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -320,47 +320,19 @@ class ZoneCellIterUnderGC : public ZoneC
 #ifdef JSGC_GENERATIONAL
         JS_ASSERT(zone->runtimeFromAnyThread()->gc.nursery.isEmpty());
 #endif
         JS_ASSERT(zone->runtimeFromAnyThread()->isHeapBusy());
         init(zone, kind);
     }
 };
 
-/* In debug builds, assert that no allocation occurs. */
-class AutoAssertNoAlloc
-{
-#ifdef JS_DEBUG
-    GCRuntime *gc;
-
-  public:
-    AutoAssertNoAlloc() : gc(nullptr) {}
-    explicit AutoAssertNoAlloc(JSRuntime *rt) : gc(nullptr) {
-        disallowAlloc(rt);
-    }
-    void disallowAlloc(JSRuntime *rt) {
-        JS_ASSERT(!gc);
-        gc = &rt->gc;
-        gc->disallowAlloc();
-    }
-    ~AutoAssertNoAlloc() {
-        if (gc)
-            gc->allowAlloc();
-    }
-#else
-  public:
-    AutoAssertNoAlloc() {}
-    explicit AutoAssertNoAlloc(JSRuntime *) {}
-    void disallowAlloc(JSRuntime *rt) {}
-#endif
-};
-
 class ZoneCellIter : public ZoneCellIterImpl
 {
-    AutoAssertNoAlloc noAlloc;
+    JS::AutoAssertNoAlloc noAlloc;
     ArenaLists *lists;
     AllocKind kind;
 
   public:
     ZoneCellIter(JS::Zone *zone, AllocKind kind)
       : lists(&zone->allocator.arenas),
         kind(kind)
     {