Bug 794667 - Add an AutoAssertCanGC to help with exact rooting; r=billm
☠☠ backed out by 0b4424c89bbd ☠ ☠
authorTerrence Cole <terrence@mozilla.com>
Wed, 26 Sep 2012 18:07:44 -0700
changeset 108892 ed626654fe5638886e4a4b401bbdd6fad5ef0477
parent 108891 e2fb3f7e790df9250af28f0154e59991362ee443
child 108893 14cf58aff2ec582529f8ca0638a1370c99167e60
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersbillm
bugs794667
milestone18.0a1
Bug 794667 - Add an AutoAssertCanGC to help with exact rooting; r=billm This will allow us to annotate all methods with either an AutoAssertNoGC or AutoAssertCanGC. These will serve the purpose currently being served by MaybeCheckStackRoots, but because they are lighter can be used throughout the engine.
js/src/gc/Root.h
js/src/jsapi.h
js/src/jsgc.cpp
js/src/jsgcinlines.h
js/src/jsnum.cpp
js/src/jsnum.h
js/src/jsscope.h
js/src/jsscopeinlines.h
js/src/jsstr.cpp
js/src/jsstr.h
--- a/js/src/gc/Root.h
+++ b/js/src/gc/Root.h
@@ -60,17 +60,17 @@
  *   separate rooting analysis.
  */
 
 namespace js {
 
 template <typename T> class Rooted;
 
 template <typename T>
-struct RootMethods { };
+struct RootMethods {};
 
 template <typename T>
 class HandleBase {};
 
 template <typename T>
 class MutableHandleBase {};
 
 } /* namespace js */
@@ -573,16 +573,31 @@ public:
 
     ~AutoAssertNoGC() {
 #ifdef DEBUG
         LeaveAssertNoGCScope();
 #endif
     }
 };
 
+/*
+ * The scoped guard object AutoAssertCanGC will assert if its live region
+ * crosses the live region of an AutoAssertNoGC guard object.
+ */
+class AutoAssertCanGC
+{
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+
+  public:
+    AutoAssertCanGC(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM) {
+        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+        JS_ASSERT(!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 */
@@ -590,23 +605,21 @@ JS_FRIEND_API(bool) NeedRelaxedRootCheck
 namespace js {
 
 /*
  * Hook for dynamic root analysis. Checks the native stack and poisons
  * references to GC things which have not been rooted.
  */
 inline void MaybeCheckStackRoots(JSContext *cx, bool relax = true)
 {
-#ifdef DEBUG
-    JS_ASSERT(!InNoGCScope());
-# if defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
+    AutoAssertCanGC cangc;
+#if defined(DEBUG) && defined(JS_GC_ZEAL) && defined(JSGC_ROOT_ANALYSIS) && !defined(JS_THREADSAFE)
     if (relax && NeedRelaxedRootChecks())
         return;
     CheckStackRoots(cx);
-# endif
 #endif
 }
 
 namespace gc {
 struct Cell;
 } /* namespace gc */
 
 /* Base class for automatic read-only object rooting during compilation. */
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2643,16 +2643,17 @@ ToBooleanSlow(const JS::Value &v);
 } /* namespace js */
 
 namespace JS {
 
 /* ES5 9.3 ToNumber. */
 JS_ALWAYS_INLINE bool
 ToNumber(JSContext *cx, const Value &v, double *out)
 {
+    AutoAssertCanGC cangc;
     AssertArgumentsAreSane(cx, v);
     {
         js::SkipRoot root(cx, &v);
         js::MaybeCheckStackRoots(cx);
     }
 
     if (v.isNumber()) {
         *out = v.toNumber();
@@ -2737,64 +2738,68 @@ extern JS_PUBLIC_API(bool)
 ToUint64Slow(JSContext *cx, const JS::Value &v, uint64_t *out);
 } /* namespace js */
 
 namespace JS {
 
 JS_ALWAYS_INLINE bool
 ToUint16(JSContext *cx, const js::Value &v, uint16_t *out)
 {
+    AutoAssertCanGC cangc;
     AssertArgumentsAreSane(cx, v);
     {
         js::SkipRoot skip(cx, &v);
         js::MaybeCheckStackRoots(cx);
     }
 
     if (v.isInt32()) {
         *out = uint16_t(v.toInt32());
         return true;
     }
     return js::ToUint16Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
 ToInt32(JSContext *cx, const js::Value &v, int32_t *out)
 {
+    AutoAssertCanGC cangc;
     AssertArgumentsAreSane(cx, v);
     {
         js::SkipRoot root(cx, &v);
         js::MaybeCheckStackRoots(cx);
     }
 
     if (v.isInt32()) {
         *out = v.toInt32();
         return true;
     }
     return js::ToInt32Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
 ToUint32(JSContext *cx, const js::Value &v, uint32_t *out)
 {
+    AutoAssertCanGC cangc;
     AssertArgumentsAreSane(cx, v);
     {
         js::SkipRoot root(cx, &v);
         js::MaybeCheckStackRoots(cx);
     }
 
     if (v.isInt32()) {
         *out = uint32_t(v.toInt32());
         return true;
     }
     return js::ToUint32Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
 ToInt64(JSContext *cx, const js::Value &v, int64_t *out)
 {
+    AutoAssertCanGC cangc;
     AssertArgumentsAreSane(cx, v);
     {
         js::SkipRoot skip(cx, &v);
         js::MaybeCheckStackRoots(cx);
     }
 
     if (v.isInt32()) {
         *out = int64_t(v.toInt32());
@@ -2802,16 +2807,17 @@ ToInt64(JSContext *cx, const js::Value &
     }
 
     return js::ToInt64Slow(cx, v, out);
 }
 
 JS_ALWAYS_INLINE bool
 ToUint64(JSContext *cx, const js::Value &v, uint64_t *out)
 {
+    AutoAssertCanGC cangc;
     AssertArgumentsAreSane(cx, v);
     {
         js::SkipRoot skip(cx, &v);
         js::MaybeCheckStackRoots(cx);
     }
 
     if (v.isInt32()) {
         /* Account for sign extension of negatives into the longer 64bit space. */
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -4665,42 +4665,46 @@ Collect(JSRuntime *rt, bool incremental,
     } while (rt->gcPoke && rt->gcShouldCleanUpEverything);
 }
 
 namespace js {
 
 void
 GC(JSRuntime *rt, JSGCInvocationKind gckind, gcreason::Reason reason)
 {
+    AutoAssertCanGC cangc;
     Collect(rt, false, SliceBudget::Unlimited, gckind, reason);
 }
 
 void
 GCSlice(JSRuntime *rt, JSGCInvocationKind gckind, gcreason::Reason reason, int64_t millis)
 {
+    AutoAssertCanGC cangc;
     int64_t sliceBudget;
     if (millis)
         sliceBudget = SliceBudget::TimeBudget(millis);
     else if (rt->gcHighFrequencyGC && rt->gcDynamicMarkSlice)
         sliceBudget = rt->gcSliceBudget * IGC_MARK_SLICE_MULTIPLIER;
     else
         sliceBudget = rt->gcSliceBudget;
 
     Collect(rt, true, sliceBudget, gckind, reason);
 }
 
 void
 GCFinalSlice(JSRuntime *rt, JSGCInvocationKind gckind, gcreason::Reason reason)
 {
+    AutoAssertCanGC cangc;
     Collect(rt, true, SliceBudget::Unlimited, gckind, reason);
 }
 
 void
 GCDebugSlice(JSRuntime *rt, bool limit, int64_t objCount)
 {
+    AutoAssertCanGC cangc;
     int64_t budget = limit ? SliceBudget::WorkBudget(objCount) : SliceBudget::Unlimited;
     PrepareForDebugGC(rt);
     Collect(rt, true, budget, GC_NORMAL, gcreason::API);
 }
 
 /* Schedule a full GC unless a compartment will already be collected. */
 void
 PrepareForDebugGC(JSRuntime *rt)
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -20,38 +20,41 @@ namespace js {
 
 struct Shape;
 
 namespace gc {
 
 inline JSGCTraceKind
 GetGCThingTraceKind(const void *thing)
 {
+    JS::AutoAssertNoGC nogc;
     JS_ASSERT(thing);
     const Cell *cell = reinterpret_cast<const Cell *>(thing);
     return MapAllocToTraceKind(cell->getAllocKind());
 }
 
 /* Capacity for slotsToThingKind */
 const size_t SLOTS_TO_THING_KIND_LIMIT = 17;
 
 /* Get the best kind to use when making an object with the given slot count. */
 static inline AllocKind
 GetGCObjectKind(size_t numSlots)
 {
     extern AllocKind slotsToThingKind[];
 
+    JS::AutoAssertNoGC nogc;
     if (numSlots >= SLOTS_TO_THING_KIND_LIMIT)
         return FINALIZE_OBJECT16;
     return slotsToThingKind[numSlots];
 }
 
 static inline AllocKind
 GetGCObjectKind(Class *clasp)
 {
+    JS::AutoAssertNoGC nogc;
     if (clasp == &FunctionClass)
         return JSFunction::FinalizeKind;
     uint32_t nslots = JSCLASS_RESERVED_SLOTS(clasp);
     if (clasp->flags & JSCLASS_HAS_PRIVATE)
         nslots++;
     return GetGCObjectKind(nslots);
 }
 
@@ -62,57 +65,62 @@ GetGCArrayKind(size_t numSlots)
     extern AllocKind slotsToThingKind[];
 
     /*
      * Dense arrays can use their fixed slots to hold their elements array
      * (less two Values worth of ObjectElements header), but if more than the
      * maximum number of fixed slots is needed then the fixed slots will be
      * unused.
      */
+    JS::AutoAssertNoGC nogc;
     JS_STATIC_ASSERT(ObjectElements::VALUES_PER_HEADER == 2);
     if (numSlots > JSObject::NELEMENTS_LIMIT || numSlots + 2 >= SLOTS_TO_THING_KIND_LIMIT)
         return FINALIZE_OBJECT2;
     return slotsToThingKind[numSlots + 2];
 }
 
 static inline AllocKind
 GetGCObjectFixedSlotsKind(size_t numFixedSlots)
 {
     extern AllocKind slotsToThingKind[];
 
+    JS::AutoAssertNoGC nogc;
     JS_ASSERT(numFixedSlots < SLOTS_TO_THING_KIND_LIMIT);
     return slotsToThingKind[numFixedSlots];
 }
 
 static inline AllocKind
 GetBackgroundAllocKind(AllocKind kind)
 {
+    JS::AutoAssertNoGC nogc;
     JS_ASSERT(!IsBackgroundFinalized(kind));
     JS_ASSERT(kind <= FINALIZE_OBJECT_LAST);
     return (AllocKind) (kind + 1);
 }
 
 /*
  * Try to get the next larger size for an object, keeping BACKGROUND
  * consistent.
  */
 static inline bool
 TryIncrementAllocKind(AllocKind *kindp)
 {
+    JS::AutoAssertNoGC nogc;
     size_t next = size_t(*kindp) + 2;
     if (next >= size_t(FINALIZE_OBJECT_LIMIT))
         return false;
     *kindp = AllocKind(next);
     return true;
 }
 
 /* Get the number of fixed slots and initial capacity associated with a kind. */
 static inline size_t
 GetGCKindSlots(AllocKind thingKind)
 {
+    JS::AutoAssertNoGC nogc;
     /* Using a switch in hopes that thingKind will usually be a compile-time constant. */
     switch (thingKind) {
       case FINALIZE_OBJECT0:
       case FINALIZE_OBJECT0_BACKGROUND:
         return 0;
       case FINALIZE_OBJECT2:
       case FINALIZE_OBJECT2_BACKGROUND:
         return 2;
@@ -132,16 +140,17 @@ GetGCKindSlots(AllocKind thingKind)
         JS_NOT_REACHED("Bad object finalize kind");
         return 0;
     }
 }
 
 static inline size_t
 GetGCKindSlots(AllocKind thingKind, Class *clasp)
 {
+    JS::AutoAssertNoGC nogc;
     size_t nslots = GetGCKindSlots(thingKind);
 
     /* An object's private data uses the space taken by its last fixed slot. */
     if (clasp->flags & JSCLASS_HAS_PRIVATE) {
         JS_ASSERT(nslots > 0);
         nslots--;
     }
 
@@ -153,16 +162,18 @@ GetGCKindSlots(AllocKind thingKind, Clas
         nslots = 0;
 
     return nslots;
 }
 
 static inline void
 GCPoke(JSRuntime *rt, Value oldval)
 {
+    JS::AutoAssertNoGC nogc;
+
     /*
      * Since we're forcing a GC from JS_GC anyway, don't bother wasting cycles
      * loading oldval.  XXX remove implied force, fix jsinterp.c's "second arg
      * ignored", etc.
      */
 #if 1
     rt->gcPoke = true;
 #else
@@ -421,16 +432,17 @@ class GCCompartmentsIter {
  * trigger GC. This will ensure that GC tracing never sees junk values stored
  * in the partially initialized thing.
  */
 
 template <typename T>
 inline T *
 NewGCThing(JSContext *cx, js::gc::AllocKind kind, size_t thingSize)
 {
+    JS::AutoAssertCanGC cangc;
     JS_ASSERT(thingSize == js::gc::Arena::thingSize(kind));
     JS_ASSERT_IF(cx->compartment == cx->runtime->atomsCompartment,
                  kind == js::gc::FINALIZE_STRING || kind == js::gc::FINALIZE_SHORT_STRING);
     JS_ASSERT(!cx->runtime->isHeapBusy());
     JS_ASSERT(!cx->runtime->noGCOrAllocationCheck);
 
     /* For testing out of memory conditions */
     JS_OOM_POSSIBLY_FAIL_REPORT(cx);
@@ -457,16 +469,17 @@ NewGCThing(JSContext *cx, js::gc::AllocK
     return static_cast<T *>(t);
 }
 
 /* Alternate form which allocates a GC thing if doing so cannot trigger a GC. */
 template <typename T>
 inline T *
 TryNewGCThing(JSContext *cx, js::gc::AllocKind kind, size_t thingSize)
 {
+    JS::AutoAssertCanGC cangc;
     JS_ASSERT(thingSize == js::gc::Arena::thingSize(kind));
     JS_ASSERT_IF(cx->compartment == cx->runtime->atomsCompartment,
                  kind == js::gc::FINALIZE_STRING || kind == js::gc::FINALIZE_SHORT_STRING);
     JS_ASSERT(!cx->runtime->isHeapBusy());
     JS_ASSERT(!cx->runtime->noGCOrAllocationCheck);
 
 #ifdef JS_GC_ZEAL
     if (cx->runtime->needZealousGC())
@@ -486,61 +499,69 @@ TryNewGCThing(JSContext *cx, js::gc::All
 }
 
 } /* namespace gc */
 } /* namespace js */
 
 inline JSObject *
 js_NewGCObject(JSContext *cx, js::gc::AllocKind kind)
 {
+    JS::AutoAssertCanGC cangc;
     JS_ASSERT(kind >= js::gc::FINALIZE_OBJECT0 && kind <= js::gc::FINALIZE_OBJECT_LAST);
     return js::gc::NewGCThing<JSObject>(cx, kind, js::gc::Arena::thingSize(kind));
 }
 
 inline JSObject *
 js_TryNewGCObject(JSContext *cx, js::gc::AllocKind kind)
 {
+    JS::AutoAssertCanGC cangc;
     JS_ASSERT(kind >= js::gc::FINALIZE_OBJECT0 && kind <= js::gc::FINALIZE_OBJECT_LAST);
     return js::gc::TryNewGCThing<JSObject>(cx, kind, js::gc::Arena::thingSize(kind));
 }
 
 inline JSString *
 js_NewGCString(JSContext *cx)
 {
+    JS::AutoAssertCanGC cangc;
     return js::gc::NewGCThing<JSString>(cx, js::gc::FINALIZE_STRING, sizeof(JSString));
 }
 
 inline JSShortString *
 js_NewGCShortString(JSContext *cx)
 {
+    JS::AutoAssertCanGC cangc;
     return js::gc::NewGCThing<JSShortString>(cx, js::gc::FINALIZE_SHORT_STRING, sizeof(JSShortString));
 }
 
 inline JSExternalString *
 js_NewGCExternalString(JSContext *cx)
 {
+    JS::AutoAssertCanGC cangc;
     return js::gc::NewGCThing<JSExternalString>(cx, js::gc::FINALIZE_EXTERNAL_STRING,
                                                 sizeof(JSExternalString));
 }
 
 inline JSScript *
 js_NewGCScript(JSContext *cx)
 {
+    JS::AutoAssertCanGC cangc;
     return js::gc::NewGCThing<JSScript>(cx, js::gc::FINALIZE_SCRIPT, sizeof(JSScript));
 }
 
 inline js::Shape *
 js_NewGCShape(JSContext *cx)
 {
+    JS::AutoAssertCanGC cangc;
     return js::gc::NewGCThing<js::Shape>(cx, js::gc::FINALIZE_SHAPE, sizeof(js::Shape));
 }
 
 inline js::BaseShape *
 js_NewGCBaseShape(JSContext *cx)
 {
+    JS::AutoAssertCanGC cangc;
     return js::gc::NewGCThing<js::BaseShape>(cx, js::gc::FINALIZE_BASE_SHAPE, sizeof(js::BaseShape));
 }
 
 #if JS_HAS_XML_SUPPORT
 extern JSXML *
 js_NewGCXML(JSContext *cx);
 #endif
 
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -1339,16 +1339,17 @@ NumberValueToStringBuffer(JSContext *cx,
     size_t cstrlen = strlen(cstr);
     JS_ASSERT(!cbuf.dbuf && cstrlen < cbuf.sbufSize);
     return sb.appendInflated(cstr, cstrlen);
 }
 
 JS_PUBLIC_API(bool)
 ToNumberSlow(JSContext *cx, Value v, double *out)
 {
+    AutoAssertCanGC cangc;
 #ifdef DEBUG
     /*
      * MSVC bizarrely miscompiles this, complaining about the first brace below
      * being unmatched (!).  The error message points at both this opening brace
      * and at the corresponding SkipRoot constructor.  The error seems to derive
      * from the presence guard-object macros on the SkipRoot class/constructor,
      * which seems well in the weeds for an unmatched-brace syntax error.
      * Otherwise the problem is inscrutable, and I haven't found a workaround.
--- a/js/src/jsnum.h
+++ b/js/src/jsnum.h
@@ -120,16 +120,17 @@ const double DOUBLE_INTEGRAL_PRECISION_L
 extern bool
 GetPrefixInteger(JSContext *cx, const jschar *start, const jschar *end, int base,
                  const jschar **endp, double *dp);
 
 /* ES5 9.3 ToNumber, overwriting *vp with the appropriate number value. */
 JS_ALWAYS_INLINE bool
 ToNumber(JSContext *cx, Value *vp)
 {
+    AutoAssertCanGC cangc;
 #ifdef DEBUG
     {
         SkipRoot skip(cx, vp);
         MaybeCheckStackRoots(cx);
     }
 #endif
 
     if (vp->isNumber())
@@ -202,16 +203,17 @@ IsDefinitelyIndex(const Value &v, uint32
 
     return false;
 }
 
 /* ES5 9.4 ToInteger. */
 static inline bool
 ToInteger(JSContext *cx, const js::Value &v, double *dp)
 {
+    AutoAssertCanGC cangc;
 #ifdef DEBUG
     {
         SkipRoot skip(cx, &v);
         MaybeCheckStackRoots(cx);
     }
 #endif
 
     if (v.isInt32()) {
--- a/js/src/jsscope.h
+++ b/js/src/jsscope.h
@@ -1051,16 +1051,17 @@ struct StackShape
 #define SHAPE_STORE_PRESERVING_COLLISION(spp, shape)                          \
     (*(spp) = (js::Shape *) (uintptr_t(shape) | SHAPE_HAD_COLLISION(*(spp))))
 
 namespace js {
 
 inline Shape *
 Shape::search(JSContext *cx, Shape *start, jsid id, Shape ***pspp, bool adding)
 {
+    AutoAssertCanGC cangc;
 #ifdef DEBUG
     {
         SkipRoot skip0(cx, &start);
         SkipRoot skip1(cx, &id);
         MaybeCheckStackRoots(cx);
     }
 #endif
 
--- a/js/src/jsscopeinlines.h
+++ b/js/src/jsscopeinlines.h
@@ -261,16 +261,17 @@ Shape::matchesParamsAfterId(BaseShape *b
            attrs == aattrs &&
            ((flags ^ aflags) & PUBLIC_FLAGS) == 0 &&
            shortid_ == ashortid;
 }
 
 inline bool
 Shape::getUserId(JSContext *cx, jsid *idp) const
 {
+    AutoAssertCanGC cangc;
     const Shape *self = this;
 #ifdef DEBUG
     {
         SkipRoot skip(cx, &self);
         MaybeCheckStackRoots(cx);
     }
 #endif
     if (self->hasShortID()) {
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -3604,16 +3604,17 @@ js_strchr_limit(const jschar *s, jschar 
     return NULL;
 }
 
 namespace js {
 
 jschar *
 InflateString(JSContext *cx, const char *bytes, size_t *lengthp, FlationCoding fc)
 {
+    AutoAssertCanGC cangc;
     size_t nchars;
     jschar *chars;
     size_t nbytes = *lengthp;
 
     // Malformed UTF8 chars could trigger errors and hence GC
     MaybeCheckStackRoots(cx);
 
     if (js_CStringsAreUTF8 || fc == CESU8Encoding) {
--- a/js/src/jsstr.h
+++ b/js/src/jsstr.h
@@ -120,16 +120,17 @@ ToStringSlow(JSContext *cx, const Value 
 /*
  * Convert the given value to a string.  This method includes an inline
  * fast-path for the case where the value is already a string; if the value is
  * known not to be a string, use ToStringSlow instead.
  */
 static JS_ALWAYS_INLINE JSString *
 ToString(JSContext *cx, const js::Value &v)
 {
+    AutoAssertCanGC cangc;
 #ifdef DEBUG
     {
         SkipRoot skip(cx, &v);
         MaybeCheckStackRoots(cx);
     }
 #endif
 
     if (v.isString())