From 09801bf00f1001ac31b768edc89c7d2e04f28867 Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Tue, 4 Aug 2009 12:55:58 +0200 Subject: [PATCH] bug 507573 - put activation clenup. r=brendan --- js/src/jsdbgapi.cpp | 6 +- js/src/jsfun.cpp | 308 ++++++++++++++++++-------------------------- js/src/jsfun.h | 4 +- js/src/jsinterp.cpp | 14 +- js/src/jsinterp.h | 17 +++ 5 files changed, 146 insertions(+), 203 deletions(-) diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 730a9d97bee3..36e14c600e2d 100644 --- a/js/src/jsdbgapi.cpp +++ b/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); diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 8a8f51c05abd..595ec229a7d8 100644 --- a/js/src/jsfun.cpp +++ b/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); diff --git a/js/src/jsfun.h b/js/src/jsfun.h index cbf76932d6c8..330413cffcb7 100644 --- a/js/src/jsfun.h +++ b/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 diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 271c5fed699e..809d8b1091ee 100644 --- a/js/src/jsinterp.cpp +++ b/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 */ diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 6f9cbffa0989..f671f0fd4bfb 100644 --- a/js/src/jsinterp.h +++ b/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