From 26a5b7f3fc0678a9570d207b32f29621be188041 Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Thu, 9 Apr 2009 18:44:54 -0700 Subject: [PATCH] Bug 487563 - Crash [@ js_Interpret] (r=mrbkap). --- js/src/jsfun.cpp | 23 +++------------------ js/src/jsinterp.cpp | 49 ++++++++++++++++++++++++++++----------------- js/src/jsinterp.h | 7 +++++++ js/src/jsparse.cpp | 33 ++++++++++++++++-------------- js/src/jsscript.h | 2 ++ js/src/jstracer.h | 3 --- js/src/jsxdrapi.h | 2 +- 7 files changed, 62 insertions(+), 57 deletions(-) diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index c176858828c..3173ae87a82 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -2205,26 +2205,9 @@ js_NewFlatClosure(JSContext *cx, JSFunction *fun) JSUpvarArray *uva = JS_SCRIPT_UPVARS(fun->u.i.script); JS_ASSERT(uva->length <= size_t(closure->dslots[-1])); - for (uint32 i = 0, n = uva->length; i < n; i++) { - uint32 cookie = uva->vector[i]; - - uintN upvarLevel = fun->u.i.script->staticLevel - UPVAR_FRAME_SKIP(cookie); - JS_ASSERT(upvarLevel <= JS_DISPLAY_SIZE); - JSStackFrame *fp2 = cx->display[upvarLevel]; - - uintN slot = UPVAR_FRAME_SLOT(cookie); - jsval *vp; - if (fp2->fun && slot < fp2->fun->nargs) { - vp = fp2->argv; - } else { - if (fp2->fun) - slot -= fp2->fun->nargs; - JS_ASSERT(slot < fp2->script->nslots); - vp = fp2->slots; - } - - closure->dslots[i] = vp[slot]; - } + uintN level = fun->u.i.script->staticLevel; + for (uint32 i = 0, n = uva->length; i < n; i++) + closure->dslots[i] = js_GetUpvar(cx, level, uva->vector[i]); return closure; } diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index cb0ef9d02ed..f6f2e8ac321 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -2054,6 +2054,34 @@ js_DoIncDec(JSContext *cx, const JSCodeSpec *cs, jsval *vp, jsval *vp2) return JS_TRUE; } +jsval +js_GetUpvar(JSContext *cx, uintN level, uintN cookie) +{ + level -= UPVAR_FRAME_SKIP(cookie); + JS_ASSERT(level < JS_DISPLAY_SIZE); + + JSStackFrame *fp = cx->display[level]; + JS_ASSERT(fp->script); + + uintN slot = UPVAR_FRAME_SLOT(cookie); + jsval *vp; + + if (!fp->fun) { + vp = fp->slots + fp->script->nfixed; + } else if (slot < fp->fun->nargs) { + vp = fp->argv; + } else if (slot == CALLEE_UPVAR_SLOT) { + vp = &fp->argv[-2]; + slot = 0; + } else { + slot -= fp->fun->nargs; + JS_ASSERT(slot < fp->script->nslots); + vp = fp->slots; + } + + return vp[slot]; +} + #ifdef DEBUG JS_STATIC_INTERPRET JS_REQUIRES_STACK void @@ -5682,29 +5710,14 @@ js_Interpret(JSContext *cx) BEGIN_CASE(JSOP_GETUPVAR) BEGIN_CASE(JSOP_CALLUPVAR) { - JSUpvarArray *uva; - uint32 skip; - JSStackFrame *fp2; + JSUpvarArray *uva = JS_SCRIPT_UPVARS(script); index = GET_UINT16(regs.pc); - uva = JS_SCRIPT_UPVARS(script); JS_ASSERT(index < uva->length); - skip = UPVAR_FRAME_SKIP(uva->vector[index]); - fp2 = cx->display[script->staticLevel - skip]; - JS_ASSERT(fp2->script); - slot = UPVAR_FRAME_SLOT(uva->vector[index]); - if (!fp2->fun) { - vp = fp2->slots + fp2->script->nfixed; - } else if (slot < fp2->fun->nargs) { - vp = fp2->argv; - } else { - slot -= fp2->fun->nargs; - JS_ASSERT(slot < fp2->script->nslots); - vp = fp2->slots; - } + rval = js_GetUpvar(cx, script->staticLevel, uva->vector[index]); + PUSH_OPND(rval); - PUSH_OPND(vp[slot]); if (op == JSOP_CALLUPVAR) PUSH_OPND(JSVAL_NULL); } diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 8996e9d03c3..204badf5675 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -606,6 +606,13 @@ js_OnUnknownMethod(JSContext *cx, jsval *vp); extern JSBool js_DoIncDec(JSContext *cx, const JSCodeSpec *cs, jsval *vp, jsval *vp2); +/* + * Given an active context, a static scope level, and an upvar cookie, return + * the value of the upvar. + */ +extern jsval +js_GetUpvar(JSContext *cx, uintN level, uintN cookie); + /* * Opcode tracing helper. When len is not 0, cx->fp->regs->pc[-len] gives the * previous opcode. diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index 411f8f63831..39d0d275497 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -748,13 +748,15 @@ static inline bool SetStaticLevel(JSTreeContext *tc, uintN staticLevel) { /* - * Reserve staticLevel 0xffff in order to reserve FREE_UPVAR_COOKIE. This - * is simpler than error-checking every MAKE_UPVAR_COOKIE, and practically - * speaking it leaves more than enough room for upvars. In fact we might - * want to split cookies with fewer bits for skip and more for slot, but - * only based on evidence. + * Reserve FREE_STATIC_LEVEL (0xffff) in order to reserve FREE_UPVAR_COOKIE + * (0xffffffff) and other cookies with that level. + * + * This is a lot simpler than error-checking every MAKE_UPVAR_COOKIE, and + * practically speaking it leaves more than enough room for upvars. In fact + * we might want to split cookie fields giving fewer bits for skip and more + * for slot, but only based on evidence. */ - if (staticLevel >= JS_BITMASK(16)) { + if (staticLevel >= FREE_STATIC_LEVEL) { JS_ReportErrorNumber(tc->compiler->context, js_GetErrorMessage, NULL, JSMSG_TOO_DEEP, js_function_str); return false; @@ -2198,7 +2200,7 @@ LeaveFunction(JSParseNode *fn, JSTreeContext *funtc, JSTreeContext *tc, if (atom == funAtom && lambda != 0) { dn->pn_op = JSOP_CALLEE; - dn->pn_cookie = MAKE_UPVAR_COOKIE(funtc->staticLevel, 0); + dn->pn_cookie = MAKE_UPVAR_COOKIE(funtc->staticLevel, CALLEE_UPVAR_SLOT); dn->pn_dflags |= PND_BOUND; /* @@ -2211,7 +2213,7 @@ LeaveFunction(JSParseNode *fn, JSTreeContext *funtc, JSTreeContext *tc, * code unfairly (see JSCompiler::setFunctionKinds, where this * flag is interpreted in its broader sense, not only to mean * "this function might leak arguments.callee"), we can perhaps - * try to work harder to add a TCF_FUN_CALLS_ITSELF flag and + * try to work harder to add a TCF_FUN_LEAKS_ITSELF flag and * use that more precisely, both here and for unnamed function * expressions. */ @@ -7875,7 +7877,6 @@ PrimaryExpr(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc, } else if (!afterDot && !(ts->flags & TSF_DESTRUCTURING)) { JSAtomListElement *ale = NULL; JSTreeContext *tcx = tc; - bool hit_named_lambda = false; JSDefinition *dn; do { @@ -7898,10 +7899,9 @@ PrimaryExpr(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc, } /* If this id names the current lambda's name, we are done. */ - if ((tc->flags & TCF_IN_FUNCTION) && - (tc->fun->flags & JSFUN_LAMBDA) && - tc->fun->atom == pn->pn_atom) { - hit_named_lambda = true; + if ((tcx->flags & TCF_IN_FUNCTION) && + (tcx->fun->flags & JSFUN_LAMBDA) && + tcx->fun->atom == pn->pn_atom) { break; } } while ((tcx = tcx->parent) != NULL); @@ -7950,14 +7950,17 @@ PrimaryExpr(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc, * a backward definition appears later; see NewBindingNode/Define). * * (b) If pn names the named function expression whose body we are - * parsing, there's no way an upvar could be referenced here. + * parsing, there's no way an upvar above tcx's static level could + * be referenced here. However, we add to upvars anyway, to treat + * the function's name as an upvar in case it is used in a nested + * function. * * (a) is is an optimization to handle forward upvar refs. Without * it, if we add only a lexdep, then inner functions making forward * refs to upvars will lose track of those upvars as their lexdeps * entries are propagated upward to their parent functions. */ - if (tcx != tc && !hit_named_lambda) { + if (tcx != tc) { ale = tc->upvars.add(tc->compiler, pn->pn_atom); if (!ale) return NULL; diff --git a/js/src/jsscript.h b/js/src/jsscript.h index 48bed022421..e1bbb97ae23 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -86,6 +86,8 @@ typedef struct JSUpvarArray { uint32 length; /* count of indexed upvar cookies */ } JSUpvarArray; +#define CALLEE_UPVAR_SLOT 0xffff +#define FREE_STATIC_LEVEL 0xffff #define FREE_UPVAR_COOKIE 0xffffffff #define MAKE_UPVAR_COOKIE(skip,slot) ((skip) << 16 | (slot)) #define UPVAR_FRAME_SKIP(cookie) ((uint32)(cookie) >> 16) diff --git a/js/src/jstracer.h b/js/src/jstracer.h index 942c9a715e7..1858fd27e63 100644 --- a/js/src/jstracer.h +++ b/js/src/jstracer.h @@ -367,9 +367,6 @@ struct InterpState #ifdef EXECUTE_TREE_TIMER uint64 startTime; #endif -#ifdef DEBUG - bool jsframe_pop_blocks_set_on_entry; -#endif /* * Used by _FAIL builtins; see jsbuiltins.h. The builtin sets the diff --git a/js/src/jsxdrapi.h b/js/src/jsxdrapi.h index 0030b37e038..dfbe2442b44 100644 --- a/js/src/jsxdrapi.h +++ b/js/src/jsxdrapi.h @@ -204,7 +204,7 @@ JS_XDRFindClassById(JSXDRState *xdr, uint32 id); * before deserialization of bytecode. If the saved version does not match * the current version, abort deserialization and invalidate the file. */ -#define JSXDR_BYTECODE_VERSION (0xb973c0de - 44) +#define JSXDR_BYTECODE_VERSION (0xb973c0de - 45) /* * Library-private functions.