Add assertions that fire when a JSAutoRequest, JSAutoSuspendRequest, JSAutoLocalRootScope, JSAutoTempValueRooter, JSAutoTempIdRooter, JSAutoIdArray, JSAutoEnumStateRooter, or JSAutoResolveFlags is used as a temporary. (Bug 518633) r=igor

js/src/jsapi.h

@@ -584,7 +584,9 @@ JS_END_EXTERN_C
 
 class JSAutoRequest {
   public:
-    JSAutoRequest(JSContext *cx) : mContext(cx), mSaveDepth(0) {
+    JSAutoRequest(JSContext *cx JS_GUARD_OBJECT_NOTIFIER_PARAM)
+        : mContext(cx), mSaveDepth(0) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_BeginRequest(mContext);
     }
     ~JSAutoRequest() {
@@ -601,6 +603,7 @@ class JSAutoRequest {
   protected:
     JSContext *mContext;
     jsrefcount mSaveDepth;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 
 #if 0
   private:
@@ -611,7 +614,9 @@ class JSAutoRequest {
 
 class JSAutoSuspendRequest {
   public:
-    JSAutoSuspendRequest(JSContext *cx) : mContext(cx), mSaveDepth(0) {
+    JSAutoSuspendRequest(JSContext *cx JS_GUARD_OBJECT_NOTIFIER_PARAM)
+        : mContext(cx), mSaveDepth(0) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         if (mContext) {
             mSaveDepth = JS_SuspendRequest(mContext);
         }
@@ -630,6 +635,7 @@ class JSAutoSuspendRequest {
   protected:
     JSContext *mContext;
     jsrefcount mSaveDepth;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 
 #if 0
   private:
@@ -989,7 +995,9 @@ JS_END_EXTERN_C
 
 class JSAutoLocalRootScope {
   public:
-    JSAutoLocalRootScope(JSContext *cx) : mContext(cx) {
+    JSAutoLocalRootScope(JSContext *cx JS_GUARD_OBJECT_NOTIFIER_PARAM)
+        : mContext(cx) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_EnterLocalRootScope(mContext);
     }
     ~JSAutoLocalRootScope() {
@@ -1002,6 +1010,7 @@ class JSAutoLocalRootScope {
 
   protected:
     JSContext *mContext;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 
 #if 0
   private:

js/src/jscntxt.h

@@ -1246,20 +1246,28 @@ FrameAtomBase(JSContext *cx, JSStackFrame *fp)
 class JSAutoTempValueRooter
 {
   public:
-    JSAutoTempValueRooter(JSContext *cx, size_t len, jsval *vec)
+    JSAutoTempValueRooter(JSContext *cx, size_t len, jsval *vec
+                          JS_GUARD_OBJECT_NOTIFIER_PARAM)
         : mContext(cx) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_PUSH_TEMP_ROOT(mContext, len, vec, &mTvr);
     }
-    explicit JSAutoTempValueRooter(JSContext *cx, jsval v = JSVAL_NULL)
+    explicit JSAutoTempValueRooter(JSContext *cx, jsval v = JSVAL_NULL
+                                   JS_GUARD_OBJECT_NOTIFIER_PARAM)
         : mContext(cx) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_PUSH_SINGLE_TEMP_ROOT(mContext, v, &mTvr);
     }
-    JSAutoTempValueRooter(JSContext *cx, JSString *str)
+    JSAutoTempValueRooter(JSContext *cx, JSString *str
+                          JS_GUARD_OBJECT_NOTIFIER_PARAM)
         : mContext(cx) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_PUSH_TEMP_ROOT_STRING(mContext, str, &mTvr);
     }
-    JSAutoTempValueRooter(JSContext *cx, JSObject *obj)
+    JSAutoTempValueRooter(JSContext *cx, JSObject *obj
+                          JS_GUARD_OBJECT_NOTIFIER_PARAM)
         : mContext(cx) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_PUSH_TEMP_ROOT_OBJECT(mContext, obj, &mTvr);
     }
 
@@ -1280,13 +1288,16 @@ class JSAutoTempValueRooter
 #endif
 
     JSTempValueRooter mTvr;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 class JSAutoTempIdRooter
 {
   public:
-    explicit JSAutoTempIdRooter(JSContext *cx, jsid id = INT_TO_JSID(0))
+    explicit JSAutoTempIdRooter(JSContext *cx, jsid id = INT_TO_JSID(0)
+                                JS_GUARD_OBJECT_NOTIFIER_PARAM)
         : mContext(cx) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_PUSH_SINGLE_TEMP_ROOT(mContext, ID_TO_VALUE(id), &mTvr);
     }
 
@@ -1300,11 +1311,15 @@ class JSAutoTempIdRooter
   private:
     JSContext *mContext;
     JSTempValueRooter mTvr;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 class JSAutoIdArray {
   public:
-    JSAutoIdArray(JSContext *cx, JSIdArray *ida) : cx(cx), idArray(ida) {
+    JSAutoIdArray(JSContext *cx, JSIdArray *ida
+                  JS_GUARD_OBJECT_NOTIFIER_PARAM)
+        : cx(cx), idArray(ida) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         if (ida)
             JS_PUSH_TEMP_ROOT(cx, ida->length, ida->vector, &tvr);
     }
@@ -1329,15 +1344,18 @@ class JSAutoIdArray {
     JSContext * const cx;
     JSIdArray * const idArray;
     JSTempValueRooter tvr;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 /* The auto-root for enumeration object and its state. */
 class JSAutoEnumStateRooter : public JSTempValueRooter
 {
   public:
-    JSAutoEnumStateRooter(JSContext *cx, JSObject *obj, jsval *statep)
+    JSAutoEnumStateRooter(JSContext *cx, JSObject *obj, jsval *statep
+                          JS_GUARD_OBJECT_NOTIFIER_PARAM)
         : mContext(cx), mStatep(statep)
     {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         JS_ASSERT(obj);
         JS_ASSERT(statep);
         JS_PUSH_TEMP_ROOT_COMMON(cx, obj, this, JSTVU_ENUMERATOR, object);
@@ -1355,13 +1373,16 @@ class JSAutoEnumStateRooter : public JSTempValueRooter
   private:
     JSContext   *mContext;
     jsval       *mStatep;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 class JSAutoResolveFlags
 {
   public:
-    JSAutoResolveFlags(JSContext *cx, uintN flags)
+    JSAutoResolveFlags(JSContext *cx, uintN flags
+                       JS_GUARD_OBJECT_NOTIFIER_PARAM)
         : mContext(cx), mSaved(cx->resolveFlags) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
         cx->resolveFlags = flags;
     }
 
@@ -1370,6 +1391,7 @@ class JSAutoResolveFlags
   private:
     JSContext *mContext;
     uintN mSaved;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 #endif /* __cpluscplus */

js/src/jsutil.h

@@ -204,4 +204,97 @@ static JS_INLINE void js_free(void* p) {
 
 JS_END_EXTERN_C
 
+#ifdef __cplusplus
+
+/**
+ * The following classes are designed to cause assertions to detect
+ * inadvertent use of guard objects as temporaries.  In other words,
+ * when we have a guard object whose only purpose is its constructor and
+ * destructor (and is never otherwise referenced), the intended use
+ * might be:
+ *     JSAutoTempValueRooter tvr(cx, 1, &val);
+ * but is is easy to accidentally write:
+ *     JSAutoTempValueRooter(cx, 1, &val);
+ * which compiles just fine, but runs the destructor well before the
+ * intended time.
+ *
+ * They work by adding (#ifdef DEBUG) an additional parameter to the
+ * guard object's constructor, with a default value, so that users of
+ * the guard object's API do not need to do anything.  The default value
+ * of this parameter is a temporary object.  C++ (ISO/IEC 14882:1998),
+ * section 12.2 [class.temporary], clauses 4 and 5 seem to assume a
+ * guarantee that temporaries are destroyed in the reverse of their
+ * construction order, but I actually can't find a statement that that
+ * is true in the general case (beyond the two specific cases mentioned
+ * there).  However, it seems to be true.
+ *
+ * These classes are intended to be used only via the macros immediately
+ * below them:
+ *   JS_DECL_USE_GUARD_OBJECT_NOTIFIER declares (ifdef DEBUG) a member
+ *     variable, and should be put where a declaration of a private
+ *     member variable would be placed.
+ *   JS_GUARD_OBJECT_NOTIFIER_PARAM should be placed at the end of the
+ *     parameters to each constructor of the guard object; it declares
+ *     (ifdef DEBUG) an additional parameter.
+ *   JS_GUARD_OBJECT_NOTIFIER_INIT is a statement that belongs in each
+ *     constructor.  It uses the parameter declared by
+ *     JS_GUARD_OBJECT_NOTIFIER_PARAM.
+ */
+#ifdef DEBUG
+class JSGuardObjectNotifier
+{
+private:
+    bool* mStatementDone;
+public:
+    JSGuardObjectNotifier() : mStatementDone(NULL) {}
+
+    ~JSGuardObjectNotifier() {
+        *mStatementDone = true;
+    }
+
+    void SetStatementDone(bool *aStatementDone) {
+        mStatementDone = aStatementDone;
+    }
+};
+
+class JSGuardObjectNotificationReceiver
+{
+private:
+    bool mStatementDone;
+public:
+    JSGuardObjectNotificationReceiver() : mStatementDone(false) {}
+
+    ~JSGuardObjectNotificationReceiver() {
+        // Assert that the guard object was not used as a temporary.
+        // (Note that this assert might also fire if Init is not called
+        // because the guard object's implementation is not using the
+        // above macros correctly.)
+        JS_ASSERT(mStatementDone);
+    }
+
+    void Init(const JSGuardObjectNotifier &aNotifier) {
+       // aNotifier is passed as a const reference so that we can pass a
+       // temporary, but we really intend it as non-const
+       const_cast<JSGuardObjectNotifier&>(aNotifier).
+           SetStatementDone(&mStatementDone);
+    }
+};
+
+#define JS_DECL_USE_GUARD_OBJECT_NOTIFIER \
+    JSGuardObjectNotificationReceiver _mCheckNotUsedAsTemporary;
+#define JS_GUARD_OBJECT_NOTIFIER_PARAM \
+    , const JSGuardObjectNotifier& _notifier = JSGuardObjectNotifier()
+#define JS_GUARD_OBJECT_NOTIFIER_INIT \
+    JS_BEGIN_MACRO _mCheckNotUsedAsTemporary.Init(_notifier); JS_END_MACRO
+
+#else /* defined(DEBUG) */
+
+#define JS_DECL_USE_GUARD_OBJECT_NOTIFIER
+#define JS_GUARD_OBJECT_NOTIFIER_PARAM
+#define JS_GUARD_OBJECT_NOTIFIER_INIT JS_BEGIN_MACRO JS_END_MACRO
+
+#endif /* !defined(DEBUG) */
+
+#endif /* defined(__cplusplus) */
+
 #endif /* jsutil_h___ */