Bug 767141 - Implement AssertRootingUnnecessary guard. r=bhackett
authorSteve Fink <sfink@mozilla.com>
Thu, 21 Jun 2012 14:15:12 -0700
changeset 98033 0d87f9f5d3c6053a3ded26d1a316897557e5aa04
parent 98032 cd6d52bdf2d81a79d83b5a664229f30951a0b23b
child 98034 54bb503183ebfcc0c98faa89c097a128eabd155c
push id23017
push userryanvm@gmail.com
push dateSat, 30 Jun 2012 19:29:24 +0000
treeherdermozilla-central@4c2ddc60f360 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs767141
milestone16.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 767141 - Implement AssertRootingUnnecessary guard. r=bhackett This RAII guard is used to mark lexical regions that are believed to never trigger GC. It is mostly intended for documentation, but is also dynamically checked.
js/src/gc/Root.h
js/src/jscntxt.cpp
js/src/jscntxt.h
js/src/jsgc.cpp
--- a/js/src/gc/Root.h
+++ b/js/src/gc/Root.h
@@ -288,24 +288,55 @@ class SkipRoot
         JS_GUARD_OBJECT_NOTIFIER_INIT;
     }
 
 #endif /* DEBUG && JSGC_ROOT_ANALYSIS */
 
     JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
+#ifdef DEBUG
+JS_FRIEND_API(bool) IsRootingUnnecessaryForContext(JSContext *cx);
+JS_FRIEND_API(void) SetRootingUnnecessaryForContext(JSContext *cx, bool value);
+#endif
+
+class AssertRootingUnnecessary {
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
+    JSContext *cx;
+    bool prev;
+public:
+    AssertRootingUnnecessary(JSContext *cx JS_GUARD_OBJECT_NOTIFIER_PARAM)
+        : cx(cx)
+    {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
+#ifdef DEBUG
+        prev = IsRootingUnnecessaryForContext(cx);
+        SetRootingUnnecessaryForContext(cx, true);
+#endif
+    }
+
+    ~AssertRootingUnnecessary() {
+#ifdef DEBUG
+        SetRootingUnnecessaryForContext(cx, prev);
+#endif
+    }
+};
+
 /*
  * Hook for dynamic root analysis. Checks the native stack and poisons
  * references to GC things which have not been rooted.
  */
-#if  defined(DEBUG) && defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
-void CheckStackRoots(JSContext *cx);
-inline void MaybeCheckStackRoots(JSContext *cx) { CheckStackRoots(cx); }
-#else
-inline void MaybeCheckStackRoots(JSContext *cx) {}
+inline void MaybeCheckStackRoots(JSContext *cx)
+{
+#ifdef DEBUG
+    JS_ASSERT(!IsRootingUnnecessaryForContext(cx));
+# if defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
+    void CheckStackRoots(JSContext *cx);
+    CheckStackRoots(cx);
+# endif
 #endif
+}
 
 }  /* namespace JS */
 
 #endif  /* __cplusplus */
 
 #endif  /* jsgc_root_h___ */
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -973,16 +973,19 @@ JSContext::JSContext(JSRuntime *rt)
     hasVersionOverride(false),
     throwing(false),
     exception(UndefinedValue()),
     runOptions(0),
     reportGranularity(JS_DEFAULT_JITREPORT_GRANULARITY),
     localeCallbacks(NULL),
     resolvingList(NULL),
     generatingError(false),
+#ifdef DEBUG
+    rootingUnnecessary(false),
+#endif
     compartment(NULL),
     stack(thisDuringConstruction()),  /* depends on cx->thread_ */
     parseMapPool_(NULL),
     globalObject(NULL),
     sharpObjectMap(thisDuringConstruction()),
     argumentFormatMap(NULL),
     lastMessage(NULL),
     errorReporter(NULL),
@@ -1033,16 +1036,32 @@ JSContext::~JSContext()
         JSArgumentFormatMap *temp = map;
         map = map->next;
         Foreground::free_(temp);
     }
 
     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;
+}
+} /* namespace JS */
+#endif
+
 void
 JSContext::resetCompartment()
 {
     RootedObject scopeobj(this);
     if (stack.hasfp()) {
         scopeobj = fp()->scopeChain();
     } else {
         scopeobj = globalObject;
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -1067,16 +1067,20 @@ struct JSContext : js::ContextFriendFiel
     /* Locale specific callbacks for string conversion. */
     JSLocaleCallbacks   *localeCallbacks;
 
     js::AutoResolving   *resolvingList;
 
     /* True if generating an error, to prevent runaway recursion. */
     bool                generatingError;
 
+#ifdef DEBUG
+    bool                rootingUnnecessary;
+#endif
+
     /* GC heap compartment. */
     JSCompartment       *compartment;
 
     inline void setCompartment(JSCompartment *compartment);
 
     /* Current execution stack. */
     js::ContextStack    stack;
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -4235,16 +4235,29 @@ JS::CheckStackRoots(JSContext *cx)
 {
     JSRuntime *rt = cx->runtime;
 
     if (rt->gcZeal_ != ZealStackRootingSafeValue && rt->gcZeal_ != ZealStackRootingValue)
         return;
     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
+    // 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);
+
+        return;
+
     AutoCopyFreeListToArenas copy(rt);
 
     JSTracer checker;
     JS_TracerInit(&checker, rt, EmptyMarkCallback);
 
     ConservativeGCData *cgcd = &rt->conservativeGC;
     cgcd->recordStackTop();