Bug 829372 - Don't clobber non-pointer-sized fields in root analysis; r=sfink
authorTerrence Cole <terrence@mozilla.com>
Thu, 10 Jan 2013 17:46:06 -0800
changeset 118498 fa88bbeed9b1f8160c46ea7c64d97eb89b42746d
parent 118497 3eae4564001cd14333d8665fa147b63e094cb35c
child 118499 7d0aa0231e3778d2d4bf79c6d3244b8757eced74
push id24166
push userMs2ger@gmail.com
push dateFri, 11 Jan 2013 13:57:41 +0000
treeherdermozilla-central@63c4b0f66a0c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs829372
milestone21.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 829372 - Don't clobber non-pointer-sized fields in root analysis; r=sfink
js/src/gc/Root.h
--- a/js/src/gc/Root.h
+++ b/js/src/gc/Root.h
@@ -159,16 +159,21 @@ template <typename T> class MutableHandl
 
 JS_FRIEND_API(void) EnterAssertNoGCScope();
 JS_FRIEND_API(void) LeaveAssertNoGCScope();
 
 /* These are exposing internal state of the GC for inlining purposes. */
 JS_FRIEND_API(bool) InNoGCScope();
 JS_FRIEND_API(bool) isGCEnabled();
 
+#if defined(DEBUG) && defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
+extern void
+CheckStackRoots(JSContext *cx);
+#endif
+
 /*
  * Handle provides an implicit constructor for NullPtr so that, given:
  *   foo(Handle<JSObject*> h);
  * callers can simply write:
  *   foo(NullPtr());
  * which avoids creating a Rooted<JSObject*> just to pass NULL.
  */
 struct NullPtr
@@ -608,58 +613,49 @@ class Rooted : public RootedBase<T>
 #endif
     }
 
     void init(js::PerThreadData *ptArg) {
 #if defined(JSGC_ROOT_ANALYSIS) || defined(JSGC_USE_EXACT_ROOTING)
         PerThreadDataFriendFields *pt = PerThreadDataFriendFields::get(ptArg);
         commonInit(pt->thingGCRooters);
 #endif
+#if defined(JSGC_ROOT_ANALYSIS)
+        scanned = false;
+#endif
     }
 
   public:
     Rooted(JSRuntime *rt
            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : ptr(RootMethods<T>::initial())
-#if defined(JSGC_ROOT_ANALYSIS)
-      , scanned(false)
-#endif
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         init(rt);
     }
 
     Rooted(JSRuntime *rt, T initial
            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : ptr(initial)
-#if defined(JSGC_ROOT_ANALYSIS)
-      , scanned(false)
-#endif
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         init(rt);
     }
 
     Rooted(JSContext *cx
            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : ptr(RootMethods<T>::initial())
-#if defined(JSGC_ROOT_ANALYSIS)
-      , scanned(false)
-#endif
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         init(cx);
     }
 
     Rooted(JSContext *cx, T initial
            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : ptr(initial)
-#if defined(JSGC_ROOT_ANALYSIS)
-      , scanned(false)
-#endif
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         init(cx);
     }
 
     Rooted(js::PerThreadData *pt
            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : ptr(RootMethods<T>::initial())
@@ -675,19 +671,16 @@ class Rooted : public RootedBase<T>
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         init(pt);
     }
 
     template <typename S>
     Rooted(JSContext *cx, const Unrooted<S> &initial
            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : ptr(static_cast<S>(initial))
-#if defined(JSGC_ROOT_ANALYSIS)
-      , scanned(false)
-#endif
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         init(cx);
     }
 
     ~Rooted() {
 #if defined(JSGC_ROOT_ANALYSIS) || defined(JSGC_USE_EXACT_ROOTING)
         JS_ASSERT(*stack == this);
@@ -727,26 +720,32 @@ class Rooted : public RootedBase<T>
 
         JS_ASSERT(!RootMethods<T>::poisoned(ptr));
 #endif
     }
 
 #if defined(JSGC_ROOT_ANALYSIS) || defined(JSGC_USE_EXACT_ROOTING)
     Rooted<T> **stack, *prev;
 #endif
+
+#if defined(JSGC_ROOT_ANALYSIS)
+    /* Has the rooting analysis ever scanned this Rooted's stack location? */
+    friend void JS::CheckStackRoots(JSContext*);
+    bool scanned;
+#endif
+
+    /*
+     * |ptr| must be the last field in Rooted because the analysis treats all
+     * Rooted as Rooted<void*> during the analysis. See bug 829372.
+     */
     T ptr;
+
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
     Rooted(const Rooted &) MOZ_DELETE;
-
-#if defined(JSGC_ROOT_ANALYSIS)
-  public:
-    /* Has the rooting analysis ever scanned this Rooted's stack location? */
-    bool scanned;
-#endif
 };
 
 #if !(defined(JSGC_ROOT_ANALYSIS) || defined(JSGC_USE_EXACT_ROOTING))
 // Defined in vm/String.h.
 template <>
 class Rooted<JSStableString *>;
 #endif
 
@@ -882,21 +881,16 @@ public:
  * AssertCanGC will assert if it is called inside of an AutoAssertNoGC region.
  */
 JS_ALWAYS_INLINE void
 AssertCanGC()
 {
     JS_ASSERT_IF(isGCEnabled(), !InNoGCScope());
 }
 
-#if defined(DEBUG) && defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
-extern void
-CheckStackRoots(JSContext *cx);
-#endif
-
 JS_FRIEND_API(bool) NeedRelaxedRootChecks();
 
 } /* namespace JS */
 
 namespace js {
 
 /*
  * Hook for dynamic root analysis. Checks the native stack and poisons