bug 507573 - put activation clenup. r=brendan

js/src/jsdbgapi.cpp

@@ -672,11 +672,7 @@ js_watch_set(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
                       : wp->setter(cx, obj, userid, vp));
                 if (injectFrame) {
                     /* Evil code can cause us to have an arguments object. */
-                    if (frame.callobj)
-                        ok &= js_PutCallObject(cx, &frame);
-                    if (frame.argsobj)
-                        ok &= js_PutArgsObject(cx, &frame);
-
+                    ok = frame.putActivationObjects(cx);
                     cx->fp = frame.down;
                     if (argv != smallv)
                         cx->free(argv);

js/src/jsfun.cpp

@@ -241,8 +241,6 @@ js_GetArgsProperty(JSContext *cx, JSStackFrame *fp, jsid id, jsval *vp)
 JSObject *
 js_GetArgsObject(JSContext *cx, JSStackFrame *fp)
 {
-    JSObject *argsobj, *global, *parent;
-
     /*
      * We must be in a function activation; the function must be lightweight
      * or else fp must have a variable object.
@@ -254,17 +252,10 @@ js_GetArgsObject(JSContext *cx, JSStackFrame *fp)
         fp = fp->down;
 
     /* Create an arguments object for fp only if it lacks one. */
-    argsobj = JSVAL_TO_OBJECT(fp->argsobj);
+    JSObject *argsobj = JSVAL_TO_OBJECT(fp->argsobj);
     if (argsobj)
         return argsobj;
 
-    /* Link the new object to fp so it can get actual argument values. */
-    argsobj = js_NewObject(cx, &js_ArgumentsClass, NULL, NULL);
-    if (!argsobj || !JS_SetPrivate(cx, argsobj, fp)) {
-        cx->weakRoots.newborn[GCX_OBJECT] = NULL;
-        return NULL;
-    }
-
     /*
      * Give arguments an intrinsic scope chain link to fp's global object.
      * Since the arguments object lacks a prototype because js_ArgumentsClass
@@ -276,10 +267,15 @@ js_GetArgsObject(JSContext *cx, JSStackFrame *fp)
      * js_GetClassPrototype not being able to find a global object containing
      * the standard prototype by starting from arguments and following parent.
      */
-    global = fp->scopeChain;
+    JSObject *parent, *global = fp->scopeChain;
     while ((parent = OBJ_GET_PARENT(cx, global)) != NULL)
         global = parent;
-    STOBJ_SET_PARENT(argsobj, global);
+    argsobj = js_NewObject(cx, &js_ArgumentsClass, NULL, global, 0);
+    if (!argsobj)
+        return NULL;
+
+    /* Link the new object to fp so it can get actual argument values. */
+    JS_SetPrivate(cx, argsobj, fp);
     fp->argsobj = OBJECT_TO_JSVAL(argsobj);
     return argsobj;
 }
@@ -287,7 +283,7 @@ js_GetArgsObject(JSContext *cx, JSStackFrame *fp)
 static JSBool
 args_enumerate(JSContext *cx, JSObject *obj);
 
-JS_FRIEND_API(JSBool)
+JSBool
 js_PutArgsObject(JSContext *cx, JSStackFrame *fp)
 {
     JSObject *argsobj;
@@ -339,32 +335,23 @@ js_PutArgsObject(JSContext *cx, JSStackFrame *fp)
 }
 
 static JSBool
-args_delProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
+args_delProperty(JSContext *cx, JSObject *obj, jsval idval, jsval *vp)
 {
-    jsint slot;
-    JSStackFrame *fp;
-
-    if (!JSVAL_IS_INT(id))
-        return JS_TRUE;
-    fp = (JSStackFrame *)
-         JS_GetInstancePrivate(cx, obj, &js_ArgumentsClass, NULL);
+    JSStackFrame *fp = (JSStackFrame *) JS_GetPrivate(cx, obj);
     if (!fp)
         return JS_TRUE;
     JS_ASSERT(fp->argsobj);
 
-    slot = JSVAL_TO_INT(id);
-    switch (slot) {
-      case ARGS_CALLEE:
-      case ARGS_LENGTH:
-        SET_OVERRIDE_BIT(fp, slot);
-        break;
-
-      default:
-        if ((uintN)slot < fp->argc && !MarkArgDeleted(cx, fp, slot))
-            return JS_FALSE;
-        break;
+    if (JSVAL_IS_INT(idval)) {
+        uintN arg = uintN(JSVAL_TO_INT(idval));
+        if (arg < fp->argc && !MarkArgDeleted(cx, fp, arg))
+            return false;
+    } else if (idval == ATOM_KEY(cx->runtime->atomState.lengthAtom)) {
+        SET_OVERRIDE_BIT(fp, ARGS_LENGTH);
+    } else if (idval == ATOM_KEY(cx->runtime->atomState.calleeAtom)) {
+        SET_OVERRIDE_BIT(fp, ARGS_CALLEE);
     }
-    return JS_TRUE;
+    return true;
 }
 
 static JS_REQUIRES_STACK JSObject *
@@ -536,23 +523,27 @@ WrapEscapingClosure(JSContext *cx, JSStackFrame *fp, JSObject *funobj, JSFunctio
 }
 
 static JSBool
-args_getProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
+ArgGetter(JSContext *cx, JSObject *obj, jsval idval, jsval *vp)
 {
-    jsint slot;
-    JSStackFrame *fp;
-
-    if (!JSVAL_IS_INT(id))
-        return JS_TRUE;
-    fp = (JSStackFrame *)
-         JS_GetInstancePrivate(cx, obj, &js_ArgumentsClass, NULL);
+    JSStackFrame *fp = (JSStackFrame *)
+                       JS_GetInstancePrivate(cx, obj, &js_ArgumentsClass, NULL);
     if (!fp)
-        return JS_TRUE;
-    JS_ASSERT(fp->argsobj);
+        return true;
 
-    slot = JSVAL_TO_INT(id);
-    switch (slot) {
-      case ARGS_CALLEE:
-        if (!TEST_OVERRIDE_BIT(fp, slot)) {
+    if (JSVAL_IS_INT(idval)) {
+        /*
+         * arg can exceed the number of arguments if a script changed the
+         * prototype to point to another Arguments object with a bigger argc.
+         */
+        uintN arg = uintN(JSVAL_TO_INT(idval));
+        if (arg < fp->argc && !ArgWasDeleted(cx, fp, arg))
+            *vp = fp->argv[arg];
+    } else if (idval == ATOM_KEY(cx->runtime->atomState.lengthAtom)) {
+        if (!TEST_OVERRIDE_BIT(fp, ARGS_LENGTH))
+            *vp = INT_TO_JSVAL((jsint)fp->argc);
+    } else {
+        JS_ASSERT(idval == ATOM_KEY(cx->runtime->atomState.calleeAtom));
+        if (!TEST_OVERRIDE_BIT(fp, ARGS_CALLEE)) {
             /*
              * If this function or one in it needs upvars that reach above it
              * in the scope chain, it must not be a null closure (it could be a
@@ -564,182 +555,139 @@ args_getProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
             if (fp->fun->needsWrapper()) {
                 JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
                                      JSMSG_OPTIMIZED_CLOSURE_LEAK);
-                return JS_FALSE;
+                return false;
             }
             *vp = OBJECT_TO_JSVAL(fp->callee);
         }
-        break;
-
-      case ARGS_LENGTH:
-        if (!TEST_OVERRIDE_BIT(fp, slot))
-            *vp = INT_TO_JSVAL((jsint)fp->argc);
-        break;
-
-      default:
-        if ((uintN)slot < fp->argc && !ArgWasDeleted(cx, fp, slot))
-            *vp = fp->argv[slot];
-        break;
     }
-    return JS_TRUE;
+    return true;
 }
 
 static JSBool
-args_setProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
+ArgSetter(JSContext *cx, JSObject *obj, jsval idval, jsval *vp)
 {
-    JSStackFrame *fp;
-    jsint slot;
-
-    if (!JSVAL_IS_INT(id))
-        return JS_TRUE;
-    fp = (JSStackFrame *)
-         JS_GetInstancePrivate(cx, obj, &js_ArgumentsClass, NULL);
+    JSStackFrame *fp = (JSStackFrame *)
+                       JS_GetInstancePrivate(cx, obj, &js_ArgumentsClass, NULL);
     if (!fp)
-        return JS_TRUE;
-    JS_ASSERT(fp->argsobj);
+        return true;
 
-    slot = JSVAL_TO_INT(id);
-    switch (slot) {
-      case ARGS_CALLEE:
-      case ARGS_LENGTH:
-        SET_OVERRIDE_BIT(fp, slot);
-        break;
-
-      default:
+    if (JSVAL_IS_INT(idval)) {
+        uintN arg = uintN(JSVAL_TO_INT(idval));
         if (FUN_INTERPRETED(fp->fun) &&
-            (uintN)slot < fp->argc &&
-            !ArgWasDeleted(cx, fp, slot)) {
-            fp->argv[slot] = *vp;
+            arg < fp->argc &&
+            !ArgWasDeleted(cx, fp, arg)) {
+            fp->argv[arg] = *vp;
         }
-        break;
+    } else {
+        JS_ASSERT(idval == ATOM_KEY(cx->runtime->atomState.lengthAtom) ||
+                  idval == ATOM_KEY(cx->runtime->atomState.calleeAtom));
+        SET_OVERRIDE_BIT(fp,
+                         (idval == ATOM_KEY(cx->runtime->atomState.lengthAtom))
+                         ? ARGS_LENGTH
+                         : ARGS_CALLEE);
     }
-    return JS_TRUE;
+    return true;
 }
 
 static JSBool
-args_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
+args_resolve(JSContext *cx, JSObject *obj, jsval idval, uintN flags,
              JSObject **objp)
 {
-    JSStackFrame *fp;
-    uintN slot;
-    JSString *str;
-    JSAtom *atom;
-    intN tinyid;
-    jsval value;
-
+    JS_ASSERT(STOBJ_GET_CLASS(obj) == &js_ArgumentsClass);
     *objp = NULL;
-    fp = (JSStackFrame *)
-         JS_GetInstancePrivate(cx, obj, &js_ArgumentsClass, NULL);
+
+    JSStackFrame *fp = (JSStackFrame *) JS_GetPrivate(cx, obj);
     if (!fp)
-        return JS_TRUE;
+        return true;
     JS_ASSERT(fp->argsobj);
 
-    if (JSVAL_IS_INT(id)) {
-        slot = JSVAL_TO_INT(id);
-        if (slot < fp->argc && !ArgWasDeleted(cx, fp, slot)) {
-            /* XXX ECMA specs DontEnum, contrary to other array-like objects */
-            if (!js_DefineProperty(cx, obj, INT_JSVAL_TO_JSID(id),
-                                   fp->argv[slot],
-                                   args_getProperty, args_setProperty,
-                                   0, NULL)) {
-                return JS_FALSE;
-            }
-            *objp = obj;
-        }
-    } else if (JSVAL_IS_STRING(id)) {
-        str = JSVAL_TO_STRING(id);
-        atom = cx->runtime->atomState.lengthAtom;
+    jsid id;
+    jsval v;
+    if (JSVAL_IS_INT(idval)) {
+        uintN arg = uintN(JSVAL_TO_INT(idval));
+        if (arg >= fp->argc || ArgWasDeleted(cx, fp, arg))
+            return true;
+        id = INT_JSVAL_TO_JSID(idval);
+        v = fp->argv[arg];
+    } else {
+        if (!JSVAL_IS_STRING(idval))
+            return true;
+        JSString *str = JSVAL_TO_STRING(idval);
+        JSAtom *atom = cx->runtime->atomState.lengthAtom;
         if (str == ATOM_TO_STRING(atom)) {
-            tinyid = ARGS_LENGTH;
-            value = INT_TO_JSVAL(fp->argc);
+            if (TEST_OVERRIDE_BIT(fp, ARGS_LENGTH))
+                return true;
+            v = INT_TO_JSVAL(fp->argc);
         } else {
             atom = cx->runtime->atomState.calleeAtom;
             if (str == ATOM_TO_STRING(atom)) {
-                tinyid = ARGS_CALLEE;
-                value = OBJECT_TO_JSVAL(fp->callee);
+                if (TEST_OVERRIDE_BIT(fp, ARGS_CALLEE))
+                    return true;
+                v = INT_TO_JSVAL(fp->callee);
             } else {
-                atom = NULL;
-
-                /* Quell GCC overwarnings. */
-                tinyid = 0;
-                value = JSVAL_NULL;
+                return true;
             }
         }
-
-        if (atom && !TEST_OVERRIDE_BIT(fp, tinyid)) {
-            if (!js_DefineNativeProperty(cx, obj, ATOM_TO_JSID(atom), value,
-                                         args_getProperty, args_setProperty, 0,
-                                         SPROP_HAS_SHORTID, tinyid, NULL)) {
-                return JS_FALSE;
-            }
-            *objp = obj;
-        }
+        id = ATOM_TO_JSID(atom);
     }
 
-    return JS_TRUE;
+    /* XXX ECMA specs DontEnum, contrary to other array-like objects */
+    if (!js_DefineProperty(cx, obj, id, v, ArgGetter, ArgSetter, 0, NULL))
+        return false;
+    *objp = obj;
+    return true;
 }
 
 static JSBool
 args_enumerate(JSContext *cx, JSObject *obj)
 {
-    JSStackFrame *fp;
-    JSObject *pobj;
-    JSProperty *prop;
-    uintN slot, argc;
+    JS_ASSERT(STOBJ_GET_CLASS(obj) == &js_ArgumentsClass);
 
-    fp = (JSStackFrame *)
-         JS_GetInstancePrivate(cx, obj, &js_ArgumentsClass, NULL);
+    JSStackFrame *fp = (JSStackFrame *) JS_GetPrivate(cx, obj);
     if (!fp)
         return JS_TRUE;
     JS_ASSERT(fp->argsobj);
 
     /*
-     * Trigger reflection with value snapshot in args_resolve using a series
+     * Trigger reflection in with value snapshot in args_resolve using a series
      * of js_LookupProperty calls.  We handle length, callee, and the indexed
      * argument properties.  We know that args_resolve covers all these cases
      * and creates direct properties of obj, but that it may fail to resolve
      * length or callee if overridden.
      */
-    if (!js_LookupProperty(cx, obj,
-                           ATOM_TO_JSID(cx->runtime->atomState.lengthAtom),
-                           &pobj, &prop)) {
-        return JS_FALSE;
-    }
-    if (prop)
-        OBJ_DROP_PROPERTY(cx, pobj, prop);
+    int argc = int(fp->argc);
+    for (int i = -2; i != argc; i++) {
+        jsid id = (i == -2)
+                  ? ATOM_TO_JSID(cx->runtime->atomState.lengthAtom)
+                  : (i == -1)
+                  ? ATOM_TO_JSID(cx->runtime->atomState.calleeAtom)
+                  : INT_JSVAL_TO_JSID(INT_TO_JSVAL(i));
 
-    if (!js_LookupProperty(cx, obj,
-                           ATOM_TO_JSID(cx->runtime->atomState.calleeAtom),
-                           &pobj, &prop)) {
-        return JS_FALSE;
-    }
-    if (prop)
-        OBJ_DROP_PROPERTY(cx, pobj, prop);
-
-    argc = fp->argc;
-    for (slot = 0; slot < argc; slot++) {
-        if (!js_LookupProperty(cx, obj, INT_TO_JSID((jsint)slot), &pobj, &prop))
-            return JS_FALSE;
+        JSObject *pobj;
+        JSProperty *prop;
+        if (!js_LookupProperty(cx, obj, id, &pobj, &prop))
+            return false;
         if (prop)
             OBJ_DROP_PROPERTY(cx, pobj, prop);
     }
-    return JS_TRUE;
+    return true;
 }
 
 JSBool JS_FASTCALL
 js_PutArguments(JSContext *cx, JSObject *argsobj, uint32 length, JSObject *callee, jsval *args)
 {
     if (!js_DefineProperty(cx, argsobj, ATOM_TO_JSID(cx->runtime->atomState.lengthAtom),
-                           INT_TO_JSVAL(length), args_getProperty, args_setProperty, 0, NULL)) {
+                           INT_TO_JSVAL(length), ArgGetter, ArgSetter, 0, NULL)) {
         return false;
     }
     if (!js_DefineProperty(cx, argsobj, ATOM_TO_JSID(cx->runtime->atomState.calleeAtom),
-                           OBJECT_TO_JSVAL(callee), args_getProperty, args_setProperty, 0, NULL)) {
+                           OBJECT_TO_JSVAL(callee), ArgGetter, ArgSetter, 0, NULL)) {
         return false;
     }
 
     for (uintN i = 0; i < length; ++i) {
         if (!js_DefineProperty(cx, argsobj, INT_TO_JSID(i), args[i],
-                               args_getProperty, args_setProperty, 0, NULL)) {
+                               ArgGetter, ArgSetter, 0, NULL)) {
             return false;
         }
     }
@@ -784,7 +732,7 @@ JSClass js_ArgumentsClass = {
     JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_HAS_RESERVED_SLOTS(1) |
     JSCLASS_MARK_IS_TRACE | JSCLASS_HAS_CACHED_PROTO(JSProto_Object),
     JS_PropertyStub,    args_delProperty,
-    args_getProperty,   args_setProperty,
+    JS_PropertyStub,    JS_PropertyStub,
     args_enumerate,     (JSResolveOp) args_resolve,
     JS_ConvertStub,     NULL,
     NULL,               NULL,
@@ -931,13 +879,25 @@ GetCallObjectFunction(JSObject *obj)
     return GET_FUNCTION_PRIVATE(cx, JSVAL_TO_OBJECT(v));
 }
 
-JS_FRIEND_API(JSBool)
+JSBool
 js_PutCallObject(JSContext *cx, JSStackFrame *fp)
 {
-    JSObject *callobj;
-    JSBool ok;
-    JSFunction *fun;
-    uintN n;
+    JSObject *callobj = fp->callobj;
+    JS_ASSERT(callobj);
+
+    /*
+     * Get the arguments object to snapshot fp's actual argument values.
+     */
+    JSBool ok = JS_TRUE;
+    if (fp->argsobj) {
+        if (!TEST_OVERRIDE_BIT(fp, CALL_ARGUMENTS))
+            STOBJ_SET_SLOT(callobj, JSSLOT_CALL_ARGUMENTS, fp->argsobj);
+        ok &= js_PutArgsObject(cx, fp);
+    }
+
+    JSFunction *fun = fp->fun;
+    JS_ASSERT(fun == GetCallObjectFunction(callobj));
+    uintN n = fun->countArgsAndVars();
 
     /*
      * Since for a call object all fixed slots happen to be taken, we can copy
@@ -945,26 +905,6 @@ js_PutCallObject(JSContext *cx, JSStackFrame *fp)
      */
     JS_STATIC_ASSERT(JS_INITIAL_NSLOTS - JSSLOT_PRIVATE ==
                      1 + CALL_CLASS_FIXED_RESERVED_SLOTS);
-
-    callobj = fp->callobj;
-    if (!callobj)
-        return JS_TRUE;
-
-    /*
-     * Get the arguments object to snapshot fp's actual argument values.
-     */
-    ok = JS_TRUE;
-    if (fp->argsobj) {
-        if (!TEST_OVERRIDE_BIT(fp, CALL_ARGUMENTS)) {
-            STOBJ_SET_SLOT(callobj, JSSLOT_CALL_ARGUMENTS,
-                           fp->argsobj);
-        }
-        ok &= js_PutArgsObject(cx, fp);
-    }
-
-    fun = fp->fun;
-    JS_ASSERT(fun == GetCallObjectFunction(callobj));
-    n = fun->countArgsAndVars();
     if (n != 0) {
         n += JS_INITIAL_NSLOTS;
         JS_LOCK_OBJ(cx, callobj);

js/src/jsfun.h

@@ -273,7 +273,7 @@ js_ReportIsNotFunction(JSContext *cx, jsval *vp, uintN flags);
 extern JSObject *
 js_GetCallObject(JSContext *cx, JSStackFrame *fp);
 
-extern JS_FRIEND_API(JSBool)
+extern JSBool
 js_PutCallObject(JSContext *cx, JSStackFrame *fp);
 
 extern JSBool
@@ -316,7 +316,7 @@ js_GetArgsProperty(JSContext *cx, JSStackFrame *fp, jsid id, jsval *vp);
 extern JSObject *
 js_GetArgsObject(JSContext *cx, JSStackFrame *fp);
 
-extern JS_FRIEND_API(JSBool)
+extern JSBool
 js_PutArgsObject(JSContext *cx, JSStackFrame *fp);
 
 extern JSBool

js/src/jsinterp.cpp

@@ -1394,13 +1394,7 @@ out:
             hook(cx, &frame, JS_FALSE, &ok, hookData);
     }
 
-    /* If frame has a call object, sync values and clear back-pointer. */
-    if (frame.callobj)
-        ok &= js_PutCallObject(cx, &frame);
-
-    /* If frame has an arguments object, sync values and clear back-pointer. */
-    if (frame.argsobj)
-        ok &= js_PutArgsObject(cx, &frame);
+    ok &= frame.putActivationObjects(cx);
 
     *vp = frame.rval;
 
@@ -3282,11 +3276,7 @@ js_Interpret(JSContext *cx)
                  * calls eval unexpectedly (in a way that is hidden from the
                  * compiler). See bug 325540.
                  */
-                if (fp->callobj)
-                    ok &= js_PutCallObject(cx, fp);
-
-                if (fp->argsobj)
-                    ok &= js_PutArgsObject(cx, fp);
+                ok &= fp->putActivationObjects(cx);
 
 #ifdef INCLUDE_MOZILLA_DTRACE
                 /* DTrace function return, inlines */

js/src/jsinterp.h

@@ -131,6 +131,23 @@ struct JSStackFrame {
                                        script->staticLevel */
 
     inline void assertValidStackDepth(uintN depth);
+
+    JSBool putActivationObjects(JSContext *cx) {
+        /*
+         * The order of calls here is important as js_PutCallObject needs to
+         * access argsobj.
+         */
+        JSBool ok;
+        if (callobj) {
+            ok = js_PutCallObject(cx, this);
+            JS_ASSERT(!argsobj);
+        } else if (argsobj) {
+            ok = js_PutArgsObject(cx, this);
+        } else {
+            ok = JS_TRUE;
+        }
+        return ok;
+    }
 };
 
 #ifdef __cplusplus