Bug 789633 - Ergonomics improvements for stack rooting assertions; r=sfink

There are a handful of classes that want to mix in this assertion, but it is
somewhat annoying to do this with our current tools. The current names are also
quite long and annoying to type.

js/src/gc/Root.h

@@ -11,9 +11,11 @@
 #ifdef __cplusplus
 
 #include "mozilla/TypeTraits.h"
+#include "mozilla/GuardObjects.h"
 
 #include "js/TemplateLib.h"
-#include "js/Utility.h"
+
+#include "jspubtd.h"
 
 namespace JS {
 
@@ -440,38 +442,30 @@ class SkipRoot
     JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
+JS_FRIEND_API(void) EnterAssertNoGCScope();
+JS_FRIEND_API(void) LeaveAssertNoGCScope();
+JS_FRIEND_API(bool) InNoGCScope();
+
 /*
- * This typedef is to annotate parameters that we have manually verified do not
- * need rooting, as opposed to parameters that have not yet been considered.
+ * The scoped guard object AutoAssertNoGC forces the GC to assert if a GC is
+ * attempted while the guard object is live.  If you have a GC-unsafe operation
+ * to perform, use this guard object to protect your opertion.
  */
-typedef JSObject *RawObject;
+class AutoAssertNoGC
+{
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
-#ifdef DEBUG
-JS_FRIEND_API(bool) IsRootingUnnecessaryForContext(JSContext *cx);
-JS_FRIEND_API(void) SetRootingUnnecessaryForContext(JSContext *cx, bool value);
-JS_FRIEND_API(bool) RelaxRootChecksForContext(JSContext *cx);
-#endif
-
-class AssertRootingUnnecessary {
-    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
-#ifdef DEBUG
-    JSContext *cx;
-    bool prev;
-#endif
 public:
-    AssertRootingUnnecessary(JSContext *cx JS_GUARD_OBJECT_NOTIFIER_PARAM)
-    {
-        JS_GUARD_OBJECT_NOTIFIER_INIT;
+    AutoAssertNoGC(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM) {
+        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
 #ifdef DEBUG
-        this->cx = cx;
-        prev = IsRootingUnnecessaryForContext(cx);
-        SetRootingUnnecessaryForContext(cx, true);
+        EnterAssertNoGCScope();
 #endif
     }
 
-    ~AssertRootingUnnecessary() {
+    ~AutoAssertNoGC() {
 #ifdef DEBUG
-        SetRootingUnnecessaryForContext(cx, prev);
+        LeaveAssertNoGCScope();
 #endif
     }
 };
@@ -481,6 +475,8 @@ extern void
 CheckStackRoots(JSContext *cx);
 #endif
 
+JS_FRIEND_API(bool) NeedRelaxedRootChecks();
+
 /*
  * Hook for dynamic root analysis. Checks the native stack and poisons
  * references to GC things which have not been rooted.
@@ -488,9 +484,9 @@ CheckStackRoots(JSContext *cx);
 inline void MaybeCheckStackRoots(JSContext *cx, bool relax = true)
 {
 #ifdef DEBUG
-    JS_ASSERT(!IsRootingUnnecessaryForContext(cx));
+    JS_ASSERT(!InNoGCScope());
 # if defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
-    if (relax && RelaxRootChecksForContext(cx))
+    if (relax && NeedRelaxedRootChecks())
         return;
     CheckStackRoots(cx);
 # endif

js/src/jsapi.cpp

@@ -740,8 +740,41 @@ static JSBool js_NewRuntimeWasCalled = JS_FALSE;
  */
 namespace JS {
 mozilla::ThreadLocal<JSRuntime *> TlsRuntime;
+
+#ifdef DEBUG
+JS_FRIEND_API(void)
+EnterAssertNoGCScope()
+{
+    ++TlsRuntime.get()->gcAssertNoGCDepth;
 }
 
+JS_FRIEND_API(void)
+LeaveAssertNoGCScope()
+{
+    --TlsRuntime.get()->gcAssertNoGCDepth;
+    JS_ASSERT(TlsRuntime.get()->gcAssertNoGCDepth >= 0);
+}
+
+JS_FRIEND_API(bool)
+InNoGCScope()
+{
+    return TlsRuntime.get()->gcAssertNoGCDepth > 0;
+}
+
+JS_FRIEND_API(bool)
+NeedRelaxedRootChecks()
+{
+    return TlsRuntime.get()->gcRelaxRootChecks;
+}
+#else
+JS_FRIEND_API(void) EnterAssertNoGCScope() {}
+JS_FRIEND_API(void) LeaveAssertNoGCScope() {}
+JS_FRIEND_API(bool) InNoGCScope() { return false; }
+JS_FRIEND_API(bool) NeedRelaxedRootChecks() { return false; }
+#endif
+
+} /* namespace JS */
+
 static const JSSecurityCallbacks NullSecurityCallbacks = { };
 
 JSRuntime::JSRuntime()
@@ -817,6 +850,10 @@ JSRuntime::JSRuntime()
     gcSliceBudget(SliceBudget::Unlimited),
     gcIncrementalEnabled(true),
     gcExactScanningEnabled(true),
+#ifdef DEBUG
+    gcRelaxRootChecks(false),
+    gcAssertNoGCDepth(0),
+#endif
     gcPoke(false),
     heapState(Idle),
 #ifdef JS_GC_ZEAL
@@ -3451,7 +3488,7 @@ JS_NewObject(JSContext *cx, JSClass *jsclasp, JSObject *protoArg, JSObject *pare
     JS_ASSERT(!(clasp->flags & JSCLASS_IS_GLOBAL));
 
     JSObject *obj = NewObjectWithClassProto(cx, clasp, proto, parent);
-    AssertRootingUnnecessary safe(cx);
+    AutoAssertNoGC nogc;
     if (obj) {
         if (clasp->ext.equality)
             MarkTypeObjectFlags(cx, obj, OBJECT_FLAG_SPECIAL_EQUALITY);
@@ -3479,7 +3516,7 @@ JS_NewObjectWithGivenProto(JSContext *cx, JSClass *jsclasp, JSObject *protoArg,
     JS_ASSERT(!(clasp->flags & JSCLASS_IS_GLOBAL));
 
     JSObject *obj = NewObjectWithGivenProto(cx, clasp, proto, parent);
-    AssertRootingUnnecessary safe(cx);
+    AutoAssertNoGC nogc;
     if (obj)
         MarkTypeObjectUnknownProperties(cx, obj->type());
     return obj;
@@ -4679,7 +4716,7 @@ JS_PUBLIC_API(JSBool)
 JS_NextProperty(JSContext *cx, JSObject *iterobjArg, jsid *idp)
 {
     RootedObject iterobj(cx, iterobjArg);
-    AssertRootingUnnecessary safe(cx);
+    AutoAssertNoGC nogc;
     int32_t i;
     Shape *shape;
     JSIdArray *ida;

js/src/jscntxt.cpp

@@ -1174,9 +1174,6 @@ JSContext::JSContext(JSRuntime *rt)
     localeCallbacks(NULL),
     resolvingList(NULL),
     generatingError(false),
-#ifdef DEBUG
-    rootingUnnecessary(false),
-#endif
     compartment(NULL),
     enterCompartmentDepth_(0),
     savedFrameChains_(),
@@ -1238,30 +1235,6 @@ JSContext::~JSContext()
     JS_ASSERT(!resolvingList);
 }
 
-#ifdef DEBUG
-namespace JS {
-
-JS_FRIEND_API(void)
-SetRootingUnnecessaryForContext(JSContext *cx, bool value)
-{
-    cx->rootingUnnecessary = value;
-}
-
-JS_FRIEND_API(bool)
-IsRootingUnnecessaryForContext(JSContext *cx)
-{
-    return cx->rootingUnnecessary;
-}
-
-JS_FRIEND_API(bool)
-RelaxRootChecksForContext(JSContext *cx)
-{
-    return cx->runtime->relaxRootChecks;
-}
-
-} /* namespace JS */
-#endif
-
 /*
  * Since this function is only called in the context of a pending exception,
  * the caller must subsequently take an error path. If wrapping fails, it will

js/src/jscntxt.h

@@ -649,14 +649,13 @@ struct JSRuntime : js::RuntimeFriendFields
         SavedGCRoot(void *thing, JSGCTraceKind kind) : thing(thing), kind(kind) {}
     };
     js::Vector<SavedGCRoot, 0, js::SystemAllocPolicy> gcSavedRoots;
+
+    bool                gcRelaxRootChecks;
+    int                 gcAssertNoGCDepth;
 #endif
 
     bool                gcPoke;
 
-#ifdef DEBUG
-    bool                relaxRootChecks;
-#endif
-
     enum HeapState {
         Idle,       // doing nothing with the GC heap
         Tracing,    // tracing the GC heap without collecting, e.g. IterateCompartments()
@@ -1190,10 +1189,6 @@ struct JSContext : js::ContextFriendFields
     /* True if generating an error, to prevent runaway recursion. */
     bool                generatingError;
 
-#ifdef DEBUG
-    bool                rootingUnnecessary;
-#endif
-
     /* The current compartment. */
     JSCompartment       *compartment;
 

js/src/jsgc.cpp

@@ -4884,16 +4884,16 @@ JS::CheckStackRoots(JSContext *cx)
     if (rt->gcZeal_ == ZealStackRootingSafeValue && !rt->gcExactScanningEnabled)
         return;
 
-    // If this assertion fails, it means that an AssertRootingUnnecessary was
-    // placed around code that could trigger GC, and is therefore wrong. The
-    // AssertRootingUnnecessary should be removed and the code it was guarding
-    // should be modified to properly root any gcthings, and very possibly any
-    // code calling that function should also be modified if it was improperly
+    // If this assertion fails, it means that an AutoAssertNoGC was placed
+    // around code that could trigger GC, and is therefore wrong. The
+    // AutoAssertNoGC should be removed and the code it was guarding should be
+    // modified to properly root any gcthings, and very possibly any code
+    // calling that function should also be modified if it was improperly
     // assuming that GC could not happen at all within the called function.
-    // (The latter may not apply if the AssertRootingUnnecessary only protected
-    // a portion of a function, so the callers were already assuming that GC
-    // could happen.)
-    JS_ASSERT(!cx->rootingUnnecessary);
+    // (The latter may not apply if the AutoAssertNoGC only protected a portion
+    // of a function, so the callers were already assuming that GC could
+    // happen.)
+    JS_ASSERT(!InNoGCScope());
 
     // GCs can't happen when analysis/inference/compilation are active.
     if (cx->compartment->activeAnalysis)

js/src/jsinfer.cpp

@@ -5481,7 +5481,7 @@ JSObject::makeLazyType(JSContext *cx)
     RootedObject self(cx, this);
     JSProtoKey key = JSCLASS_CACHED_PROTO_KEY(getClass());
     TypeObject *type = cx->compartment->types.newTypeObject(cx, NULL, key, getProto());
-    AssertRootingUnnecessary safe(cx);
+    AutoAssertNoGC nogc;
     if (!type) {
         if (cx->typeInferenceEnabled())
             cx->compartment->types.setPendingNukeTypes(cx);

js/src/jspropertycacheinlines.h

@@ -31,7 +31,7 @@ JS_ALWAYS_INLINE void
 js::PropertyCache::test(JSContext *cx, jsbytecode *pc, JSObject *&obj,
                         JSObject *&pobj, PropertyCacheEntry *&entry, PropertyName *&name)
 {
-    AssertRootingUnnecessary assert(cx);
+    AutoAssertNoGC nogc;
 
     JS_ASSERT(this == &cx->propertyCache());
 

js/src/shell/js.cpp

@@ -3534,7 +3534,7 @@ RelaxRootChecks(JSContext *cx, unsigned argc, jsval *vp)
     }
 
 #ifdef DEBUG
-    cx->runtime->relaxRootChecks = true;
+    cx->runtime->gcRelaxRootChecks = true;
 #endif
 
     return true;