Backed out changeset 0d61bcf08362 (bug 1013531) for bustage.

CLOSED TREE

dom/bindings/CallbackObject.cpp

@@ -81,10 +81,7 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
   nsIGlobalObject* globalObject = nullptr;
 
   {
-    // Bug 955660: we cannot do "proper" rooting here because we need the
-    // global to get a context. Everything here is simple getters that cannot
-    // GC, so just paper over the necessary dataflow inversion.
-    JS::AutoSuppressGCAnalysis nogc;
+    JS::AutoAssertNoGC nogc;
     if (mIsMainThread) {
       // Now get the global and JSContext for this callback.
       nsGlobalWindow* win = xpc::WindowGlobalOrNull(realCallback);

js/public/GCAPI.h

@@ -369,27 +369,9 @@ extern JS_FRIEND_API(void)
 ShrinkGCBuffers(JSRuntime *rt);
 
 /*
- * Assert if a GC occurs while this class is live. This class does not disable
- * the static rooting hazard analysis.
- */
-class JS_PUBLIC_API(AutoAssertOnGC)
-{
-    JSRuntime *runtime;
-    size_t gcNumber;
-
-  public:
-    AutoAssertOnGC();
-    explicit AutoAssertOnGC(JSRuntime *rt);
-    ~AutoAssertOnGC();
-
-    static void VerifyIsSafeToGC(JSRuntime *rt);
-};
-
-/*
- * 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 any GC occured while this guard object was 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
@@ -399,28 +381,22 @@ class JS_PUBLIC_API(AutoAssertOnGC)
  *       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(AutoAssertNoGC)
 {
-  public:
-    AutoSuppressGCAnalysis() : AutoAssertOnGC() {}
-    explicit AutoSuppressGCAnalysis(JSRuntime *rt) : AutoAssertOnGC(rt) {}
-};
+#ifdef JS_DEBUG
+    JSRuntime *runtime;
+    size_t gcNumber;
 
-/*
- * 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
- * internal pointers to GC things where the GC thing itself may not be present
- * for the static analysis: e.g. acquiring inline chars from a JSString* on the
- * heap.
- */
-class JS_PUBLIC_API(AutoCheckCannotGC) : public AutoAssertOnGC
-{
   public:
-    AutoCheckCannotGC() : AutoAssertOnGC() {}
-    explicit AutoCheckCannotGC(JSRuntime *rt) : AutoAssertOnGC(rt) {}
+    AutoAssertNoGC();
+    AutoAssertNoGC(JSRuntime *rt);
+    ~AutoAssertNoGC();
+#else
+  public:
+    /* Prevent unreferenced local warnings in opt builds. */
+    AutoAssertNoGC() {}
+    explicit AutoAssertNoGC(JSRuntime *) {}
+#endif
 };
 
 class JS_PUBLIC_API(ObjectPtr)

js/src/devtools/rootAnalysis/annotations.js

@@ -138,7 +138,7 @@ var ignoreFunctions = {
     "PR_ExplodeTime" : true,
     "PR_ErrorInstallTable" : true,
     "PR_SetThreadPrivate" : true,
-    "JSObject* js::GetWeakmapKeyDelegate(JSObject*)" : true, // FIXME: mark with AutoSuppressGCAnalysis instead
+    "JSObject* js::GetWeakmapKeyDelegate(JSObject*)" : true, // FIXME: mark with AutoAssertNoGC instead
     "uint8 NS_IsMainThread()" : true,
 
     // FIXME!
@@ -161,7 +161,7 @@ var ignoreFunctions = {
 
     // Bug 948646 - the only thing AutoJSContext's constructor calls
     // is an Init() routine whose entire body is covered with an
-    // AutoSuppressGCAnalysis. AutoSafeJSContext is the same thing, just with
+    // AutoAssertNoGC. AutoSafeJSContext is the same thing, just with
     // a different value for the 'aSafe' parameter.
     "void mozilla::AutoJSContext::AutoJSContext(mozilla::detail::GuardObjectNotifier*)" : true,
     "void mozilla::AutoSafeJSContext::~AutoSafeJSContext(int32)" : true,
@@ -230,7 +230,7 @@ function isSuppressConstructor(name)
 {
     return name.indexOf("::AutoSuppressGC") != -1
         || name.indexOf("::AutoEnterAnalysis") != -1
-        || name.indexOf("::AutoSuppressGCAnalysis") != -1
+        || name.indexOf("::AutoAssertNoGC") != -1
         || name.indexOf("::AutoIgnoreRootingHazards") != -1;
 }
 

js/src/devtools/rootAnalysis/computeGCTypes.js

@@ -133,7 +133,6 @@ addGCType('js::LazyScript');
 addGCType('js::ion::IonCode');
 addGCPointer('JS::Value');
 addGCPointer('jsid');
-addGCPointer('JS::AutoCheckCannotGC');
 
 function explain(csu, indent, seen) {
     if (!seen)

js/src/gc/GCRuntime.h

@@ -537,14 +537,6 @@ class GCRuntime
     /* Strong references on scripts held for PCCount profiling API. */
     js::ScriptAndCountsVector *scriptAndCountsVector;
 
-    /*
-     * Some regions of code are hard for the static rooting hazard analysis to
-     * understand. In those cases, we trade the static analysis for a dynamic
-     * analysis. When this is non-zero, we should assert if we trigger, or
-     * might trigger, a GC.
-     */
-    int inUnsafeRegion;
-
   private:
     /* Always preserve JIT code during GCs, for testing. */
     bool                  alwaysPreserveCode;

js/src/jsapi-tests/testGCExactRooting.cpp

@@ -21,17 +21,3 @@ BEGIN_TEST(testGCExactRooting)
     return true;
 }
 END_TEST(testGCExactRooting)
-
-BEGIN_TEST(testGCSuppressions)
-{
-    JS::AutoAssertOnGC nogc;
-    JS::AutoCheckCannotGC checkgc;
-    JS::AutoSuppressGCAnalysis noanalysis;
-
-    JS::AutoAssertOnGC nogcRt(cx->runtime());
-    JS::AutoCheckCannotGC checkgcRt(cx->runtime());
-    JS::AutoSuppressGCAnalysis noanalysisRt(cx->runtime());
-
-    return true;
-}
-END_TEST(testGCSuppressions)

js/src/jsgc.cpp

@@ -188,7 +188,6 @@
 #include "jscntxt.h"
 #include "jscompartment.h"
 #include "jsobj.h"
-#include "jsprf.h"
 #include "jsscript.h"
 #include "jstypes.h"
 #include "jsutil.h"
@@ -1115,14 +1114,13 @@ GCRuntime::GCRuntime(JSRuntime *rt) :
     mallocBytes(0),
     mallocGCTriggered(false),
     scriptAndCountsVector(nullptr),
-    helperState(rt),
     alwaysPreserveCode(false),
 #ifdef DEBUG
     noGCOrAllocationCheck(0),
 #endif
     lock(nullptr),
     lockOwner(nullptr),
-    inUnsafeRegion(0)
+    helperState(rt)
 {
 }
 
@@ -4334,9 +4332,6 @@ AutoGCSession::AutoGCSession(GCRuntime *gc)
     // It's ok if threads other than the main thread have suppressGC set, as
     // they are operating on zones which will not be collected from here.
     JS_ASSERT(!gc->rt->mainThread.suppressGC);
-
-    // Assert if this is a GC unsafe region.
-    JS::AutoAssertOnGC::VerifyIsSafeToGC(gc->rt);
 }
 
 AutoGCSession::~AutoGCSession()
@@ -5626,50 +5621,32 @@ JS::GetGCNumber()
         return 0;
     return rt->gc.number;
 }
-#endif
 
-JS::AutoAssertOnGC::AutoAssertOnGC()
+JS::AutoAssertNoGC::AutoAssertNoGC()
   : runtime(nullptr), gcNumber(0)
 {
     js::PerThreadData *data = js::TlsPerThreadData.get();
     if (data) {
         /*
          * GC's from off-thread will always assert, so off-thread is implicitly
-         * AutoAssertOnGC. We still need to allow AutoAssertOnGC to be used in
+         * AutoAssertNoGC. We still need to allow AutoAssertNoGC to be used in
          * code that works from both threads, however. We also use this to
          * annotate the off thread run loops.
          */
         runtime = data->runtimeIfOnOwnerThread();
-        if (runtime) {
+        if (runtime)
             gcNumber = runtime->gc.number;
-            ++runtime->gc.inUnsafeRegion;
-        }
     }
 }
 
-JS::AutoAssertOnGC::AutoAssertOnGC(JSRuntime *rt)
+JS::AutoAssertNoGC::AutoAssertNoGC(JSRuntime *rt)
   : runtime(rt), gcNumber(rt->gc.number)
 {
-    ++rt->gc.inUnsafeRegion;
 }
 
-JS::AutoAssertOnGC::~AutoAssertOnGC()
+JS::AutoAssertNoGC::~AutoAssertNoGC()
 {
-    if (runtime) {
-        --runtime->gc.inUnsafeRegion;
-        MOZ_ASSERT(runtime->gc.inUnsafeRegion >= 0);
-
-        /*
-         * The following backstop assertion should never fire: if we bumped the
-         * gcNumber, we should have asserted because inUnsafeRegion was true.
-         */
-        MOZ_ASSERT(gcNumber == runtime->gc.number, "GC ran inside an AutoAssertOnGC scope.");
-    }
-}
-
-/* static */ void
-JS::AutoAssertOnGC::VerifyIsSafeToGC(JSRuntime *rt)
-{
-    if (rt->gc.inUnsafeRegion > 0)
-        MOZ_CRASH("[AutoAssertOnGC] possible GC in GC-unsafe region");
+    if (runtime)
+        MOZ_ASSERT(gcNumber == runtime->gc.number, "GC ran inside an AutoAssertNoGC scope.");
 }
+#endif

js/src/jsgcinlines.h

@@ -504,10 +504,6 @@ CheckAllocatorState(ThreadSafeContext *cx, AllocKind kind)
     JS_ASSERT(rt->gc.isAllocAllowed());
 #endif
 
-    // Crash if we perform a GC action when it is not safe.
-    if (allowGC && !rt->mainThread.suppressGC)
-        JS::AutoAssertOnGC::VerifyIsSafeToGC(rt);
-
     // For testing out of memory conditions
     if (!PossiblyFail()) {
         js_ReportOutOfMemory(cx);

js/src/jsobj.h

@@ -1203,7 +1203,7 @@ inline JSObject *
 GetInnerObject(JSObject *obj)
 {
     if (js::InnerObjectOp op = obj->getClass()->ext.innerObject) {
-        JS::AutoSuppressGCAnalysis nogc;
+        JS::AutoAssertNoGC nogc;
         return op(obj);
     }
     return obj;

js/src/jsweakmap.cpp

@@ -165,7 +165,7 @@ ObjectValueMap::findZoneEdges()
      * edge to ensure that the delegate zone does finish marking after the key
      * zone.
      */
-    JS::AutoSuppressGCAnalysis nogc;
+    JS::AutoAssertNoGC nogc;
     Zone *mapZone = compartment->zone();
     for (Range r = all(); !r.empty(); r.popFront()) {
         JSObject *key = r.front().key();

js/src/jsworkers.cpp

@@ -1047,7 +1047,7 @@ WorkerThread::handleGCHelperWorkload()
 void
 WorkerThread::threadLoop()
 {
-    JS::AutoSuppressGCAnalysis nogc;
+    JS::AutoAssertNoGC nogc;
     AutoLockWorkerThreadState lock;
 
     js::TlsPerThreadData.set(threadData.addr());

js/src/vm/ForkJoin.cpp

@@ -1503,15 +1503,15 @@ ForkJoinShared::executePortion(PerThreadData *perThread, ThreadPoolWorker *worke
     // WARNING: This code runs ON THE PARALLEL WORKER THREAD.
     // Be careful when accessing cx_.
 
+    // ForkJoinContext already contains an AutoAssertNoGC; however, the analysis
+    // does not propagate this type information. We duplicate the assertion
+    // here for maximum clarity.
+    JS::AutoAssertNoGC nogc(runtime());
+
     Allocator *allocator = allocators_[worker->id()];
     ForkJoinContext cx(perThread, worker, allocator, this, &records_[worker->id()]);
     AutoSetForkJoinContext autoContext(&cx);
 
-    // ForkJoinContext already contains an AutoSuppressGCAnalysis; however, the
-    // analysis does not propagate this type information. We duplicate the
-    // assertion here for maximum clarity.
-    JS::AutoSuppressGCAnalysis nogc;
-
 #ifdef DEBUG
     // Set the maximum worker and slice number for prettier spewing.
     cx.maxWorkerId = threadPool_->numWorkers();
@@ -1622,7 +1622,7 @@ ForkJoinContext::ForkJoinContext(PerThreadData *perThreadData, ThreadPoolWorker
     shared_(shared),
     worker_(worker),
     acquiredJSContext_(false),
-    nogc_()
+    nogc_(shared->runtime())
 {
     /*
      * Unsafely set the zone. This is used to track malloc counters and to

js/src/vm/ForkJoin.h

@@ -426,7 +426,7 @@ class ForkJoinContext : public ThreadSafeContext
 
     // ForkJoinContext is allocated on the stack. It would be dangerous to GC
     // with it live because of the GC pointer fields stored in the context.
-    JS::AutoSuppressGCAnalysis nogc_;
+    JS::AutoAssertNoGC nogc_;
 };
 
 // Locks a JSContext for its scope. Be very careful, because locking a

js/src/vm/SPSProfiler.cpp

@@ -143,7 +143,7 @@ SPSProfiler::markEvent(const char *event)
 {
     JS_ASSERT(enabled());
     if (eventMarker_) {
-        JS::AutoSuppressGCAnalysis nogc;
+        JS::AutoAssertNoGC nogc;
         eventMarker_(event);
     }
 }

js/src/vm/Stack.cpp

@@ -559,7 +559,7 @@ FrameIter::settleOnActivation()
         if (data_.principals_) {
             JSContext *cx = data_.cx_->asJSContext();
             if (JSSubsumesOp subsumes = cx->runtime()->securityCallbacks->subsumes) {
-                JS::AutoSuppressGCAnalysis nogc;
+                JS::AutoAssertNoGC nogc;
                 if (!subsumes(data_.principals_, activation->compartment()->principals)) {
                     ++data_.activations_;
                     continue;

js/src/vm/StructuredClone.cpp

@@ -370,7 +370,7 @@ Discard(uint64_t *buffer, size_t nbytes, const JSStructuredCloneCallbacks *cb, v
         return;
 
     // freeTransfer should not GC
-    JS::AutoSuppressGCAnalysis nogc;
+    JS::AutoAssertNoGC nogc;
 
     uint64_t numTransferables = LittleEndian::readUint64(point++);
     while (numTransferables--) {

js/xpconnect/src/nsCxPusher.cpp

@@ -183,7 +183,7 @@ AutoJSContext::AutoJSContext(bool aSafe MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
 void
 AutoJSContext::Init(bool aSafe MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
 {
-  JS::AutoSuppressGCAnalysis nogc;
+  JS::AutoAssertNoGC nogc;
   MOZ_ASSERT(!mCx, "mCx should not be initialized!");
 
   MOZ_GUARD_OBJECT_NOTIFIER_INIT;