From 3f4a58e4f0b9bf4f5cf345c5900c02a729d4ab0f Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Wed, 8 Apr 2009 13:14:02 -0700 Subject: [PATCH 1/4] Bug 487271 - Crash [@ js_Invoke ], and missing google-maps background, at padmapper.com (r=mrbkap). --- js/src/jscntxt.h | 2 +- js/src/jsemit.cpp | 34 +++++++++++++++++++++------------- js/src/jsfun.cpp | 12 ++++++------ js/src/jsparse.cpp | 15 ++++++++++++++- js/src/jsxdrapi.h | 2 +- 5 files changed, 43 insertions(+), 22 deletions(-) diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index a4ec3c317ee..e4aeabad8ca 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -856,7 +856,7 @@ struct JSContext { /* * Classic Algol "display" static link optimization. */ -#define JS_DISPLAY_SIZE 16 +#define JS_DISPLAY_SIZE 16U JSStackFrame *display[JS_DISPLAY_SIZE]; diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 9d67c8da6a2..11d870685c6 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -1912,6 +1912,14 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) JSStackFrame *caller = cg->compiler->callerFrame; if (caller) { JS_ASSERT(cg->flags & TCF_COMPILE_N_GO); + + /* + * Don't generate upvars on the left side of a for loop. See + * bug 470758. + */ + if (cg->flags & TCF_IN_FOR_INIT) + return JS_TRUE; + JS_ASSERT(caller->script); if (!caller->fun || caller->varobj != cg->scopeChain) return JS_TRUE; @@ -1922,18 +1930,16 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) * Optimize access to function's arguments and variable and the * arguments object. */ - if (op != JSOP_NAME || cg->staticLevel >= JS_DISPLAY_SIZE) + if (op != JSOP_NAME) return JS_TRUE; JSLocalKind localKind = js_LookupLocal(cx, caller->fun, atom, &index); if (localKind == JSLOCAL_NONE) return JS_TRUE; - /* - * Don't generate upvars on the left side of a for loop. See - * bug 470758. - */ - if (cg->flags & TCF_IN_FOR_INIT) + uintN upvarLevel = caller->fun->u.i.script->staticLevel; + JS_ASSERT(cg->staticLevel > upvarLevel); + if (upvarLevel >= JS_DISPLAY_SIZE) return JS_TRUE; ale = cg->upvarList.lookup(atom); @@ -1962,8 +1968,7 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) index += caller->fun->nargs; JS_ASSERT(index < JS_BIT(16)); - JS_ASSERT(cg->staticLevel > caller->fun->u.i.script->staticLevel); - uintN skip = cg->staticLevel - caller->fun->u.i.script->staticLevel; + uintN skip = cg->staticLevel - upvarLevel; cg->upvarMap.vector[ALE_INDEX(ale)] = MAKE_UPVAR_COOKIE(skip, index); } @@ -2034,15 +2039,18 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) JS_ASSERT(JOF_OPTYPE(op) == JOF_ATOM); /* - * If op is a mutating opcode or the function is heavyweight, we fall - * back on JSOP_*NAME*. + * If op is a mutating opcode, this upvar's static level is too big to + * index into the display, or the function is heavyweight, we fall back + * on JSOP_*NAME*. */ - if (op != JSOP_NAME || (cg->flags & TCF_FUN_HEAVYWEIGHT)) + if (op != JSOP_NAME) + return JS_TRUE; + if (level >= JS_DISPLAY_SIZE) + return JS_TRUE; + if (cg->flags & TCF_FUN_HEAVYWEIGHT) return JS_TRUE; if (FUN_FLAT_CLOSURE(cg->fun)) { - /* Flat closure is formed one frame up from its static level. */ - --skip; op = JSOP_GETDSLOT; } else { /* diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index e3f0420e798..c176858828c 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -2205,14 +2205,14 @@ js_NewFlatClosure(JSContext *cx, JSFunction *fun) JSUpvarArray *uva = JS_SCRIPT_UPVARS(fun->u.i.script); JS_ASSERT(uva->length <= size_t(closure->dslots[-1])); - JSStackFrame *fp = js_GetTopStackFrame(cx); - for (uint32 i = 0, n = uva->length; i < n; i++) { - JSStackFrame *fp2 = fp; - for (uintN skip = UPVAR_FRAME_SKIP(uva->vector[i]); skip != 0; --skip) - fp2 = fp2->down; + uint32 cookie = uva->vector[i]; - uintN slot = UPVAR_FRAME_SLOT(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; diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index 65794e42570..f204ae58a4d 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -1659,6 +1659,19 @@ FindFunArgs(JSFunctionBox *funbox, int level, JSFunctionBoxQueue *queue) JSParseNode *fn = funbox->node; int fnlevel = level; + /* + * An eval can leak funbox, functions along its ancestor line, and its + * immediate kids. Since FindFunArgs uses DFS and the parser propagates + * TCF_FUN_HEAVYWEIGHT bottom up, funbox's ancestor function nodes have + * already been marked as funargs by this point. Therefore we have to + * flag only funbox->node and funbox->kids' nodes here. + */ + if (funbox->tcflags & TCF_FUN_HEAVYWEIGHT) { + fn->pn_dflags |= PND_FUNARG; + for (JSFunctionBox *kid = funbox->kids; kid; kid = kid->siblings) + kid->node->pn_dflags |= PND_FUNARG; + } + if (fn->isFunArg()) { queue->push(funbox); fnlevel = int(funbox->level); @@ -2280,7 +2293,7 @@ LeaveFunction(JSParseNode *fn, JSTreeContext *funtc, JSTreeContext *tc, if (!fn->pn_body) return false; fn->pn_body->pn_type = TOK_UPVARS; - fn->pn_pos = body->pn_pos; + fn->pn_body->pn_pos = body->pn_pos; fn->pn_body->pn_names = funtc->upvars; fn->pn_body->pn_tree = body; funtc->upvars.clear(); diff --git a/js/src/jsxdrapi.h b/js/src/jsxdrapi.h index 8ff3d3daf01..0030b37e038 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 - 43) +#define JSXDR_BYTECODE_VERSION (0xb973c0de - 44) /* * Library-private functions. From 058279399270f38f501dd675a13d202cc19330d8 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Wed, 8 Apr 2009 17:45:17 -0700 Subject: [PATCH 2/4] Bug 487534 - TM: "Assertion failure: JSVAL_IS_NULL(v)" with function/regexp used as index. The other half of bug 484120, poorly reviewed by me -- no soup for me! r=graydon --- js/src/jstracer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 0f7b39cdf82..e148bb7827c 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -7663,6 +7663,8 @@ TraceRecorder::record_JSOP_SETELEM() jsid id; if (!JSVAL_IS_INT(idx)) { + if (!JSVAL_IS_PRIMITIVE(idx)) + ABORT_TRACE("non-primitive index"); // If index is not a string, turn it into a string. if (!js_InternNonIntElementId(cx, obj, idx, &id)) ABORT_TRACE("failed to intern non-int element id"); From 0b6ed952844d564940acce37a70298e1f41f9a3a Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Wed, 8 Apr 2009 18:27:44 -0700 Subject: [PATCH 3/4] Bug 487538 - bug 487271 left three tests broken in its wake (r=mrbkap). --- js/src/jsapi.cpp | 3 ++- js/src/jsparse.cpp | 9 +++++---- js/src/jsparse.h | 12 ++++++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 5a64a1d0533..8a65929e0fb 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4400,7 +4400,8 @@ JS_CloneFunctionObject(JSContext *cx, JSObject *funobj, JSObject *parent) uint32 i = 0, n = uva->length; for (; i < n; i++) { JSObject *obj = parent; - for (uintN skip = UPVAR_FRAME_SKIP(uva->vector[i]); skip != 0; --skip) { + int skip = UPVAR_FRAME_SKIP(uva->vector[i]); + while (--skip > 0) { if (!obj) { JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_CLONE_FUNOBJ_SCOPE); diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index f204ae58a4d..dbecb01a3dc 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -1352,6 +1352,7 @@ MakeDefIntoUse(JSDefinition *dn, JSParseNode *pn, JSAtom *atom, JSTreeContext *t pnu->pn_lexdef = (JSDefinition *) pn; pn->pn_dflags |= pnu->pn_dflags & (PND_ASSIGNED | PND_FUNARG); } + pn->pn_dflags |= dn->pn_dflags & (PND_ASSIGNED | PND_FUNARG); pn->dn_uses = dn; dn->pn_defn = false; @@ -1667,9 +1668,9 @@ FindFunArgs(JSFunctionBox *funbox, int level, JSFunctionBoxQueue *queue) * flag only funbox->node and funbox->kids' nodes here. */ if (funbox->tcflags & TCF_FUN_HEAVYWEIGHT) { - fn->pn_dflags |= PND_FUNARG; + fn->setFunArg(); for (JSFunctionBox *kid = funbox->kids; kid; kid = kid->siblings) - kid->node->pn_dflags |= PND_FUNARG; + kid->node->setFunArg(); } if (fn->isFunArg()) { @@ -1689,7 +1690,7 @@ FindFunArgs(JSFunctionBox *funbox, int level, JSFunctionBoxQueue *queue) JSDefinition *lexdep = ALE_DEFN(ale)->resolve(); if (!lexdep->isFreeVar() && int(lexdep->frameLevel()) <= fnlevel) { - fn->pn_dflags |= PND_FUNARG; + fn->setFunArg(); queue->push(funbox); break; } @@ -1737,7 +1738,7 @@ JSCompiler::markFunArgs(JSFunctionBox *funbox, uintN tcflags) * which suppresses revisiting this function (namely the * !lexdep->isFunArg() test just above). */ - lexdep->pn_dflags |= PND_FUNARG; + lexdep->setFunArg(); JSFunctionBox *afunbox = lexdep->pn_funbox; queue.push(afunbox); diff --git a/js/src/jsparse.h b/js/src/jsparse.h index 8b352e96617..a721f84dc60 100644 --- a/js/src/jsparse.h +++ b/js/src/jsparse.h @@ -462,6 +462,7 @@ struct JSParseNode { /* Defined below, see after struct JSDefinition. */ bool isAssigned() const; bool isFunArg() const; + void setFunArg(); void become(JSParseNode *pn2); void clear(); @@ -725,6 +726,17 @@ JSParseNode::isFunArg() const return test(PND_FUNARG); } +inline void +JSParseNode::setFunArg() +{ + if (pn_defn) { + ((JSDefinition *)this)->pn_dflags |= PND_FUNARG; + } else if (pn_used) { + pn_lexdef->pn_dflags |= PND_FUNARG; + pn_dflags |= PND_FUNARG; + } +} + struct JSObjectBox { JSObjectBox *traceLink; JSObjectBox *emitLink; From b9323a139430571dbacafbb1a80336aea6406f46 Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Wed, 8 Apr 2009 18:42:20 -0700 Subject: [PATCH 4/4] Bug 487209 - erroneous redeclaration of let ... with try {...} catch(e) {var e...} (r=mrbkap). --- js/src/jsemit.cpp | 7 ++++--- js/src/jsemit.h | 3 ++- js/src/jsparse.cpp | 19 +++++++++++++++++-- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 11d870685c6..7fbe30bbb33 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -1508,14 +1508,15 @@ js_DefineCompileTimeConstant(JSContext *cx, JSCodeGenerator *cg, JSAtom *atom, } JSStmtInfo * -js_LexicalLookup(JSTreeContext *tc, JSAtom *atom, jsint *slotp) +js_LexicalLookup(JSTreeContext *tc, JSAtom *atom, jsint *slotp, JSStmtInfo *stmt) { - JSStmtInfo *stmt; JSObject *obj; JSScope *scope; JSScopeProperty *sprop; - for (stmt = tc->topScopeStmt; stmt; stmt = stmt->downScope) { + if (!stmt) + stmt = tc->topScopeStmt; + for (; stmt; stmt = stmt->downScope) { if (stmt->type == STMT_WITH) break; diff --git a/js/src/jsemit.h b/js/src/jsemit.h index 6e39103bc2d..10333ca2142 100644 --- a/js/src/jsemit.h +++ b/js/src/jsemit.h @@ -557,7 +557,8 @@ js_DefineCompileTimeConstant(JSContext *cx, JSCodeGenerator *cg, JSAtom *atom, * found. Otherwise return null. */ extern JSStmtInfo * -js_LexicalLookup(JSTreeContext *tc, JSAtom *atom, jsint *slotp); +js_LexicalLookup(JSTreeContext *tc, JSAtom *atom, jsint *slotp, + JSStmtInfo *stmt = NULL); /* * Emit code into cg for the tree rooted at pn. diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index dbecb01a3dc..411f8f63831 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -2972,6 +2972,19 @@ PopStatement(JSTreeContext *tc) js_PopStatement(tc); } +static inline bool +OuterLet(JSTreeContext *tc, JSStmtInfo *stmt, JSAtom *atom) +{ + while (stmt->downScope) { + stmt = js_LexicalLookup(tc, atom, NULL, stmt->downScope); + if (!stmt) + return false; + if (stmt->type == STMT_BLOCK) + return true; + } + return false; +} + static JSBool BindVarOrConst(JSContext *cx, BindData *data, JSAtom *atom, JSTreeContext *tc) { @@ -3010,8 +3023,10 @@ BindVarOrConst(JSContext *cx, BindData *data, JSAtom *atom, JSTreeContext *tc) } else { if (JS_HAS_STRICT_OPTION(cx) ? op != JSOP_DEFVAR || dn_kind != JSDefinition::VAR - : op == JSOP_DEFCONST || dn_kind == JSDefinition::CONST || - dn_kind == JSDefinition::LET) { + : op == JSOP_DEFCONST || + dn_kind == JSDefinition::CONST || + (dn_kind == JSDefinition::LET && + (stmt->type != STMT_CATCH || OuterLet(tc, stmt, atom)))) { name = js_AtomToPrintableString(cx, atom); if (!name || !js_ReportCompileErrorNumber(cx, TS(tc->compiler), pn,