From 655888f31e9f7f91dd09ec738f9be21e6e7afa36 Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Thu, 28 Aug 2008 23:50:48 -0700 Subject: [PATCH 1/7] Consolidate equal and cmp code harder, trace switch ops, use INS_CONST more (bug to be filed -- bugzilla down atm). --- js/src/jstracer.cpp | 245 +++++++++++++++++++++----------------------- js/src/jstracer.h | 6 +- 2 files changed, 121 insertions(+), 130 deletions(-) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index c82963acd27..a28127d3390 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -2568,6 +2568,35 @@ TraceRecorder::ifop() return true; } +bool +TraceRecorder::switchop() +{ + jsval& v = stackval(-1); + if (isNumber(v)) { + jsdouble d = asNumber(v); + jsdpun u; + u.d = d; + guard(true, + addName(lir->ins2(LIR_feq, get(&v), lir->insImmq(u.u64)), + "guard(switch on numeric)"), + BRANCH_EXIT); + } else if (JSVAL_IS_STRING(v)) { + LIns* args[] = { get(&v), INS_CONSTPTR(JSVAL_TO_STRING(v)) }; + guard(true, + addName(lir->ins_eq0(lir->ins_eq0(lir->insCall(F_EqualStrings, args))), + "guard(switch on string)"), + BRANCH_EXIT); + } else if (JSVAL_IS_BOOLEAN(v)) { + guard(true, + addName(lir->ins2(LIR_eq, get(&v), lir->insImm(JSVAL_TO_BOOLEAN(v))), + "guard(switch on boolean)"), + BRANCH_EXIT); + } else { + ABORT_TRACE("switch on object, null, or undefined"); + } + return true; +} + bool TraceRecorder::inc(jsval& v, jsint incr, bool pre) { @@ -2649,13 +2678,15 @@ TraceRecorder::incElem(jsint incr, bool pre) } bool -TraceRecorder::cmp(LOpcode op, bool negate) +TraceRecorder::cmp(LOpcode op, int flags) { jsval& r = stackval(-1); jsval& l = stackval(-2); LIns* x; + bool negate = !!(flags & CMP_NEGATE); bool cond; if (JSVAL_IS_STRING(l) && JSVAL_IS_STRING(r)) { + JS_ASSERT(!negate); LIns* args[] = { get(&r), get(&l) }; x = lir->ins1(LIR_i2f, lir->insCall(F_CompareStrings, args)); x = lir->ins2i(op, x, 0); @@ -2736,7 +2767,7 @@ TraceRecorder::cmp(LOpcode op, bool negate) cond = (lnum == rnum) ^ negate; break; } - } else if (JSVAL_IS_BOOLEAN(l) && JSVAL_IS_BOOLEAN(r)) { + } else if (JSVAL_IS_BOOLEAN(l) && JSVAL_IS_BOOLEAN(r)) { x = lir->ins2(op, lir->ins1(LIR_i2f, get(&l)), lir->ins1(LIR_i2f, get(&r))); if (negate) x = lir->ins_eq0(x); @@ -2768,8 +2799,10 @@ TraceRecorder::cmp(LOpcode op, bool negate) /* The interpreter fuses comparisons and the following branch, so we have to do that here as well. */ - if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) - guard(cond, x, BRANCH_EXIT); + if (flags & CMP_TRY_BRANCH_AFTER_COND) { + if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) + guard(cond, x, BRANCH_EXIT); + } /* We update the stack after the guard. This is safe since the guard bails out at the comparison and the interpreter @@ -2780,6 +2813,61 @@ TraceRecorder::cmp(LOpcode op, bool negate) return true; } +// FIXME: we currently compare only like operand types; if for JSOP_EQ and +// JSOP_NE we ever evolve to handle conversions then we must insist on like +// "types" here (care required for 0 == -1, e.g.). +bool +TraceRecorder::equal(int flags) +{ + jsval& r = stackval(-1); + jsval& l = stackval(-2); + bool negate = !!(flags & CMP_NEGATE); + if (JSVAL_IS_STRING(l) && JSVAL_IS_STRING(r)) { + LIns* args[] = { get(&r), get(&l) }; + bool cond = js_EqualStrings(JSVAL_TO_STRING(l), JSVAL_TO_STRING(r)) ^ negate; + LIns* x = lir->ins_eq0(lir->insCall(F_EqualStrings, args)); + if (!negate) + x = lir->ins_eq0(x); + + /* The interpreter fuses comparisons and the following branch, + so we have to do that here as well. */ + if (CMP_TRY_BRANCH_AFTER_COND) { + if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) + guard(cond, x, BRANCH_EXIT); + } + + /* We update the stack after the guard. This is safe since + the guard bails out at the comparison and the interpreter + will therefore re-execute the comparison. This way the + value of the condition doesn't have to be calculated and + saved on the stack in most cases. */ + set(&l, x); + return true; + } + if (JSVAL_IS_OBJECT(l) && JSVAL_IS_OBJECT(r)) { + bool cond = (l == r) ^ negate; + LIns* x = lir->ins2(LIR_eq, get(&l), get(&r)); + if (negate) + x = lir->ins_eq0(x); + + /* The interpreter fuses comparisons and the following branch, + so we have to do that here as well. */ + if (CMP_TRY_BRANCH_AFTER_COND) { + if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) + guard(cond, x, BRANCH_EXIT); + } + + /* We update the stack after the guard. This is safe since + the guard bails out at the comparison and the interpreter + will therefore re-execute the comparison. This way the + value of the condition doesn't have to be calculated and + saved on the stack in most cases. */ + set(&l, x); + return true; + } + return cmp(LIR_feq, flags); +} + bool TraceRecorder::unary(LOpcode op) { @@ -3081,7 +3169,7 @@ TraceRecorder::native_get(LIns* obj_ins, LIns* pobj_ins, JSScopeProperty* sprop, if (sprop->slot != SPROP_INVALID_SLOT) v_ins = stobj_get_slot(pobj_ins, sprop->slot, dslots_ins); else - v_ins = lir->insImm(JSVAL_TO_BOOLEAN(JSVAL_VOID)); + v_ins = INS_CONST(JSVAL_TO_BOOLEAN(JSVAL_VOID)); return true; } @@ -3247,7 +3335,7 @@ TraceRecorder::record_EnterFrame() js_AtomToPrintableString(cx, cx->fp->fun->atom), callDepth);); JSStackFrame* fp = cx->fp; - LIns* void_ins = lir->insImm(JSVAL_TO_BOOLEAN(JSVAL_VOID)); + LIns* void_ins = INS_CONST(JSVAL_TO_BOOLEAN(JSVAL_VOID)); jsval* vp = &fp->argv[fp->argc]; jsval* vpstop = vp + (fp->fun->nargs - fp->argc); @@ -3291,7 +3379,7 @@ bool TraceRecorder::record_JSOP_INTERRUPT() bool TraceRecorder::record_JSOP_PUSH() { - stack(0, lir->insImm(JSVAL_TO_BOOLEAN(JSVAL_VOID))); + stack(0, INS_CONST(JSVAL_TO_BOOLEAN(JSVAL_VOID))); return true; } @@ -3392,112 +3480,40 @@ TraceRecorder::record_JSOP_BITAND() return binary(LIR_and); } -// See FIXME for JSOP_STRICTEQ before evolving JSOP_EQ to handle mixed types. bool TraceRecorder::record_JSOP_EQ() { - jsval& r = stackval(-1); - jsval& l = stackval(-2); - if (JSVAL_IS_STRING(l) && JSVAL_IS_STRING(r)) { - LIns* args[] = { get(&r), get(&l) }; - bool cond = js_EqualStrings(JSVAL_TO_STRING(l), JSVAL_TO_STRING(r)); - LIns* x = lir->ins_eq0(lir->ins_eq0(lir->insCall(F_EqualStrings, args))); - /* The interpreter fuses comparisons and the following branch, - so we have to do that here as well. */ - if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) - guard(cond, x, BRANCH_EXIT); - - /* We update the stack after the guard. This is safe since - the guard bails out at the comparison and the interpreter - will therefore re-execute the comparison. This way the - value of the condition doesn't have to be calculated and - saved on the stack in most cases. */ - set(&l, x); - return true; - } - if (JSVAL_IS_OBJECT(l) && JSVAL_IS_OBJECT(r)) { - bool cond = (l == r); - LIns* x = lir->ins2(LIR_eq, get(&l), get(&r)); - /* The interpreter fuses comparisons and the following branch, - so we have to do that here as well. */ - if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) - guard(cond, x, BRANCH_EXIT); - - /* We update the stack after the guard. This is safe since - the guard bails out at the comparison and the interpreter - will therefore re-execute the comparison. This way the - value of the condition doesn't have to be calculated and - saved on the stack in most cases. */ - set(&l, x); - return true; - } - return cmp(LIR_feq); + return equal(CMP_TRY_BRANCH_AFTER_COND); } -// See FIXME for JSOP_STRICTNE before evolving JSOP_NE to handle mixed types. bool TraceRecorder::record_JSOP_NE() { - jsval& r = stackval(-1); - jsval& l = stackval(-2); - if (JSVAL_IS_STRING(l) && JSVAL_IS_STRING(r)) { - LIns* args[] = { get(&r), get(&l) }; - bool cond = !js_EqualStrings(JSVAL_TO_STRING(l), JSVAL_TO_STRING(r)); - LIns* x = lir->ins_eq0(lir->insCall(F_EqualStrings, args)); - /* The interpreter fuses comparisons and the following branch, - so we have to do that here as well. */ - if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) - guard(cond, x, BRANCH_EXIT); - - /* We update the stack after the guard. This is safe since - the guard bails out at the comparison and the interpreter - will therefore re-execute the comparison. This way the - value of the condition doesn't have to be calculated and - saved on the stack in most cases. */ - set(&l, x); - return true; - } - if (JSVAL_IS_OBJECT(l) && JSVAL_IS_OBJECT(r)) { - bool cond = (l != r); - LIns* x = lir->ins_eq0(lir->ins2(LIR_eq, get(&l), get(&r))); - /* The interpreter fuses comparisons and the following branch, - so we have to do that here as well. */ - if (cx->fp->regs->pc[1] == JSOP_IFEQ || cx->fp->regs->pc[1] == JSOP_IFNE) - guard(cond, x, BRANCH_EXIT); - - /* We update the stack after the guard. This is safe since - the guard bails out at the comparison and the interpreter - will therefore re-execute the comparison. This way the - value of the condition doesn't have to be calculated and - saved on the stack in most cases. */ - set(&l, x); - return true; - } - return cmp(LIR_feq, true); + return equal(CMP_NEGATE | CMP_TRY_BRANCH_AFTER_COND); } bool TraceRecorder::record_JSOP_LT() { - return cmp(LIR_flt); + return cmp(LIR_flt, CMP_TRY_BRANCH_AFTER_COND); } bool TraceRecorder::record_JSOP_LE() { - return cmp(LIR_fle); + return cmp(LIR_fle, CMP_TRY_BRANCH_AFTER_COND); } bool TraceRecorder::record_JSOP_GT() { - return cmp(LIR_fgt); + return cmp(LIR_fgt, CMP_TRY_BRANCH_AFTER_COND); } bool TraceRecorder::record_JSOP_GE() { - return cmp(LIR_fge); + return cmp(LIR_fge, CMP_TRY_BRANCH_AFTER_COND); } bool @@ -3844,7 +3860,7 @@ TraceRecorder::record_JSOP_TYPEOF() bool TraceRecorder::record_JSOP_VOID() { - stack(-1, lir->insImm(JSVAL_TO_BOOLEAN(JSVAL_VOID))); + stack(-1, INS_CONST(JSVAL_TO_BOOLEAN(JSVAL_VOID))); return true; } @@ -4177,7 +4193,7 @@ TraceRecorder::interpretedFunctionCall(jsval& fval, JSFunction* fun, uintN argc) callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, callee)); lir->insStorei(lir->insImmPtr(fi.callpc), lirbuf->rp, callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, callpc)); - lir->insStorei(lir->insImm(fi.word), lirbuf->rp, + lir->insStorei(INS_CONST(fi.word), lirbuf->rp, callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, word)); atoms = fun->u.i.script->atomMap.vector; @@ -4475,7 +4491,7 @@ TraceRecorder::prop(JSObject* obj, LIns* obj_ins, uint32& slot, LIns*& v_ins) /* Check for non-existent property reference, which results in undefined. */ const JSCodeSpec& cs = js_CodeSpec[*cx->fp->regs->pc]; if (PCVAL_IS_NULL(pcval)) { - v_ins = lir->insImm(JSVAL_TO_BOOLEAN(JSVAL_VOID)); + v_ins = INS_CONST(JSVAL_TO_BOOLEAN(JSVAL_VOID)); JS_ASSERT(cs.ndefs == 1); stack(-cs.nuses, v_ins); slot = SPROP_INVALID_SLOT; @@ -4696,54 +4712,25 @@ TraceRecorder::record_JSOP_AND() bool TraceRecorder::record_JSOP_TABLESWITCH() { - return false; + return switchop(); } bool TraceRecorder::record_JSOP_LOOKUPSWITCH() { - jsval& v = stackval(-1); - if (isNumber(v)) { - jsdouble d = asNumber(v); - jsdpun u; - u.d = d; - guard(true, - addName(lir->ins2(LIR_feq, get(&v), lir->insImmq(u.u64)), - "guard(lookupswitch numeric)"), - BRANCH_EXIT); - } else if (JSVAL_IS_STRING(v)) { - LIns* args[] = { get(&v), INS_CONSTPTR(JSVAL_TO_STRING(v)) }; - guard(true, - addName(lir->ins_eq0(lir->ins_eq0(lir->insCall(F_EqualStrings, args))), - "guard(lookupswitch string)"), - BRANCH_EXIT); - } else if (JSVAL_IS_BOOLEAN(v)) { - guard(true, - addName(lir->ins2(LIR_eq, get(&v), lir->insImm(JSVAL_TO_BOOLEAN(v))), - "guard(lookupswitch boolean)"), - BRANCH_EXIT); - } else { - ABORT_TRACE("lookupswitch on object, null, or undefined"); - } - return true; + return switchop(); } bool TraceRecorder::record_JSOP_STRICTEQ() { - // FIXME: JSOP_EQ currently compares only like operand types; if it evolves - // to handle conversions we must insist on like "types" here (care required - // for 0 == -1, e.g.). - return record_JSOP_EQ(); + return equal(); } bool TraceRecorder::record_JSOP_STRICTNE() { - // FIXME: JSOP_NE currently compares only like operand types; if it evolves - // to handle conversions we must insist on like "types" here (care required - // for 0 == -1, e.g.). - return record_JSOP_NE(); + return equal(CMP_NEGATE); } bool @@ -5182,13 +5169,13 @@ TraceRecorder::record_JSOP_CONDSWITCH() bool TraceRecorder::record_JSOP_CASE() { - return false; + return equal() && ifop(); } bool TraceRecorder::record_JSOP_DEFAULT() { - return false; + return true; } bool @@ -5364,25 +5351,25 @@ TraceRecorder::record_JSOP_GOSUBX() bool TraceRecorder::record_JSOP_CASEX() { - return record_JSOP_CASE(); + return equal() && ifop(); } bool TraceRecorder::record_JSOP_DEFAULTX() { - return record_JSOP_DEFAULT(); + return true; } bool TraceRecorder::record_JSOP_TABLESWITCHX() { - return record_JSOP_TABLESWITCH(); + return switchop(); } bool TraceRecorder::record_JSOP_LOOKUPSWITCHX() { - return record_JSOP_LOOKUPSWITCH(); + return switchop(); } bool @@ -5780,7 +5767,7 @@ TraceRecorder::record_JSOP_STOP() JS_ASSERT(OBJECT_TO_JSVAL(fp->thisp) == fp->argv[-1]); rval_ins = get(&fp->argv[-1]); } else { - rval_ins = lir->insImm(JSVAL_TO_BOOLEAN(JSVAL_VOID)); + rval_ins = INS_CONST(JSVAL_TO_BOOLEAN(JSVAL_VOID)); } clearFrameSlotsFromCache(); return true; @@ -6014,7 +6001,7 @@ TraceRecorder::record_JSOP_NEWARRAY() bool TraceRecorder::record_JSOP_HOLE() { - stack(0, lir->insImm(JSVAL_TO_BOOLEAN(JSVAL_HOLE))); + stack(0, INS_CONST(JSVAL_TO_BOOLEAN(JSVAL_HOLE))); return true; } diff --git a/js/src/jstracer.h b/js/src/jstracer.h index 7d97659e53e..97fd443d935 100644 --- a/js/src/jstracer.h +++ b/js/src/jstracer.h @@ -270,12 +270,16 @@ class TraceRecorder { nanojit::LIns* f2i(nanojit::LIns* f); bool ifop(); + bool switchop(); bool inc(jsval& v, jsint incr, bool pre = true); bool inc(jsval& v, nanojit::LIns*& v_ins, jsint incr, bool pre = true); bool incProp(jsint incr, bool pre = true); bool incElem(jsint incr, bool pre = true); bool incName(jsint incr, bool pre = true); - bool cmp(nanojit::LOpcode op, bool negate = false); + + enum { CMP_NEGATE = 1, CMP_TRY_BRANCH_AFTER_COND = 2 }; + bool cmp(nanojit::LOpcode op, int flags = 0); + bool equal(int flags = 0); bool unary(nanojit::LOpcode op); bool binary(nanojit::LOpcode op); From 18d57492309c94bc8d21682b909e9e052477747d Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Fri, 29 Aug 2008 00:24:11 -0700 Subject: [PATCH 2/7] Fix upvar decompilation for eval-from-fun case (452441, r=igor). --- js/src/jsdbgapi.cpp | 4 +++- js/src/jsemit.cpp | 3 +++ js/src/jsemit.h | 8 ++++++++ js/src/jsobj.cpp | 5 +++-- js/src/jsparse.cpp | 5 +++-- js/src/jsscript.cpp | 2 +- 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 6e2075779c6..234f4711f3b 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -1267,7 +1267,9 @@ JS_EvaluateUCInStackFrame(JSContext *cx, JSStackFrame *fp, flags = fp->flags; fp->flags |= JSFRAME_DEBUGGER | JSFRAME_EVAL; script = js_CompileScript(cx, scobj, JS_StackFramePrincipals(cx, fp), - TCF_COMPILE_N_GO, chars, length, NULL, + TCF_COMPILE_N_GO | + TCF_PUT_STATIC_DEPTH(fp->script->staticDepth + 1), + chars, length, NULL, filename, lineno); fp->flags = flags; if (!script) diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 6b69c9ddf58..d034cec3ca2 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -1878,6 +1878,9 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) if (!(tc->flags & TCF_IN_FUNCTION)) { if ((cx->fp->flags & JSFRAME_SPECIAL) && cx->fp->fun) { + if (cg->staticDepth > JS_DISPLAY_SIZE) + goto out; + localKind = js_LookupLocal(cx, cx->fp->fun, atom, &index); if (localKind != JSLOCAL_NONE) { if (PN_OP(pn) == JSOP_NAME) { diff --git a/js/src/jsemit.h b/js/src/jsemit.h index d567272ee7f..2789f34b6b9 100644 --- a/js/src/jsemit.h +++ b/js/src/jsemit.h @@ -209,6 +209,14 @@ struct JSTreeContext { /* tree context for semantic checks */ TCF_FUN_USES_NONLOCALS | \ TCF_FUN_CLOSURE_VS_VAR) +/* + * Flags field, not stored in JSTreeContext.flags, for passing staticDepth + * into js_CompileScript. + */ +#define TCF_STATIC_DEPTH_MASK 0xffff0000 +#define TCF_GET_STATIC_DEPTH(f) ((uint32)(f) >> 16) +#define TCF_PUT_STATIC_DEPTH(d) ((uint16)(d) << 16) + #ifdef JS_SCOPE_DEPTH_METER # define JS_SCOPE_DEPTH_METERING(code) ((void) (code)) #else diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 3cdfdeadd78..461b1325dc9 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -1318,14 +1318,15 @@ js_obj_eval(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) fp->flags |= JSFRAME_EVAL; } while ((fp = fp->down) != caller); - script = js_CompileScript(cx, scopeobj, principals, TCF_COMPILE_N_GO, + script = js_CompileScript(cx, scopeobj, principals, + TCF_COMPILE_N_GO | + TCF_PUT_STATIC_DEPTH(caller->script->staticDepth + 1), JSSTRING_CHARS(str), JSSTRING_LENGTH(str), NULL, file, line); if (!script) { ok = JS_FALSE; goto out; } - script->staticDepth = caller->script->staticDepth + 1; if (argc < 2) { /* Execute using caller's new scope object (might be a Call object). */ diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index 7e835e668e5..6296523fa94 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -566,7 +566,7 @@ js_CompileScript(JSContext *cx, JSObject *obj, JSPrincipals *principals, void *sbrk(ptrdiff_t), *before = sbrk(0); #endif - JS_ASSERT(!(tcflags & ~(TCF_COMPILE_N_GO | TCF_NO_SCRIPT_RVAL))); + JS_ASSERT(!(tcflags & ~(TCF_COMPILE_N_GO | TCF_NO_SCRIPT_RVAL | TCF_STATIC_DEPTH_MASK))); if (!js_InitParseContext(cx, &pc, principals, chars, length, file, filename, lineno)) { @@ -591,7 +591,8 @@ js_CompileScript(JSContext *cx, JSObject *obj, JSPrincipals *principals, pc.tokenStream.lineno); /* From this point the control must flow via the label out. */ - cg.treeContext.flags |= tcflags; + cg.treeContext.flags |= (uint16) tcflags; + cg.staticDepth = TCF_GET_STATIC_DEPTH(tcflags); /* * Inline Statements() to emit as we go to save space. diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 0c2dfec6b72..b2f2eb45a1e 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -263,7 +263,7 @@ script_compile_sub(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, * and compile-time scope. */ fp->flags |= JSFRAME_SCRIPT_OBJECT; - tcflags = 0; + tcflags = caller ? TCF_PUT_STATIC_DEPTH(caller->staticDepth + 1) : 0; script = js_CompileScript(cx, scopeobj, principals, tcflags, JSSTRING_CHARS(str), JSSTRING_LENGTH(str), NULL, file, line); From f92de94117331b30ed248d075f52d270bd5eeec8 Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Fri, 29 Aug 2008 00:58:10 -0700 Subject: [PATCH 3/7] Fix bogus JOF_VARPROP test; fix uninitialized id in JSOP_IN recorder, should have caught it when I reviewed danderson's patch. --- js/src/jstracer.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index a28127d3390..171bb4ac582 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -3038,7 +3038,9 @@ TraceRecorder::test_property_cache(JSObject* obj, LIns* obj_ins, JSObject*& obj2 } } else { #ifdef DEBUG - ptrdiff_t pcoff = (mode == JOF_VARPROP) ? SLOTNO_LEN : 0; + JSOp op = JSOp(*cx->fp->regs->pc); + ptrdiff_t pcoff = (op == JSOP_GETARGPROP) ? ARGNO_LEN : + (op == JSOP_GETLOCALPROP) ? SLOTNO_LEN : 0; jsatomid index = js_GetIndexFromBytecode(cx, cx->fp->script, cx->fp->regs->pc, pcoff); JS_ASSERT(entry->kpc == (jsbytecode*) atoms[index]); JS_ASSERT(entry->kshape == jsuword(aobj)); @@ -5058,10 +5060,13 @@ TraceRecorder::record_JSOP_IN() ABORT_TRACE("JSOP_IN on E4X QName left operand"); jsid id; - if (JSVAL_IS_INT(lval)) + if (JSVAL_IS_INT(lval)) { id = INT_JSVAL_TO_JSID(lval); - else if (!JSVAL_IS_STRING(lval)) - ABORT_TRACE("non-string left operand to JSOP_IN"); + } else { + if (!JSVAL_IS_STRING(lval)) + ABORT_TRACE("non-string left operand to JSOP_IN"); + id = ATOM_TO_JSID(lval); + } // Expect what we see at trace recording time (hit or miss) to be the same // when executing the trace. Use a builtin helper for named properties, as From 5d1f73ba74089db91ec7f8d9cd1c59ec2a85c05d Mon Sep 17 00:00:00 2001 From: Robert Sayre Date: Fri, 29 Aug 2008 11:01:56 -0400 Subject: [PATCH 4/7] Add tests for continue statement. --- js/src/trace-test.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/js/src/trace-test.js b/js/src/trace-test.js index 377ca9fa625..2972d84e16b 100644 --- a/js/src/trace-test.js +++ b/js/src/trace-test.js @@ -1020,6 +1020,37 @@ function testDecayingInnerLoop() { testDecayingInnerLoop.expected = 5000; test(testDecayingInnerLoop); +function testContinue() { + var i; + var total = 0; + for (i = 0; i < 20; ++i) { + if (i == 11) + continue; + total++; + } + return total; +} +testContinue.expected = 19; +test(testContinue); + +function testContinueWithLabel() { + var i = 0; + var j = 20; + checkiandj : + while (i<10) { + i+=1; + checkj : + while (j>10) { + j-=1; + if ((j%2)==0) + continue checkj; + } + } + return i + j; +} +testContinueWithLabel.expected = 20; +test(testContinueWithLabel); + /* Keep these at the end so that we can see the summary after the trace-debug spew. */ print("\npassed:", passes.length && passes.join(",")); print("\nFAILED:", fails.length && fails.join(",")); From f35adba372203cafeec8cd55f95cef0e3f542c15 Mon Sep 17 00:00:00 2001 From: Robert Sayre Date: Fri, 29 Aug 2008 13:04:08 -0400 Subject: [PATCH 5/7] Add tests covering division. --- js/src/trace-test.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/js/src/trace-test.js b/js/src/trace-test.js index 2972d84e16b..57f87ff1b3a 100644 --- a/js/src/trace-test.js +++ b/js/src/trace-test.js @@ -1051,6 +1051,30 @@ function testContinueWithLabel() { testContinueWithLabel.expected = 20; test(testContinueWithLabel); +function testDivision() { + var a = 32768; + var b; + while (b !== 1) { + b = a / 2; + a = b; + } + return a; +} +testDivision.expected = 1; +test(testDivision); + +function testDivisionFloat() { + var a = 32768.0; + var b; + while (b !== 1) { + b = a / 2.0; + a = b; + } + return a === 1.0; +} +testDivisionFloat.expected = true; +test(testDivisionFloat); + /* Keep these at the end so that we can see the summary after the trace-debug spew. */ print("\npassed:", passes.length && passes.join(",")); print("\nFAILED:", fails.length && fails.join(",")); From f10bd19c8e969882f29c6f39f006f4dc426dd883 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 29 Aug 2008 13:05:41 -0700 Subject: [PATCH 6/7] Abort recording on invalid string indexes for JSOP_GETELEM (bug 452713, r=brendan). --- js/src/jstracer.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 171bb4ac582..ea7a19a2da1 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -4029,6 +4029,12 @@ TraceRecorder::record_JSOP_GETELEM() jsval& l = stackval(-2); if (JSVAL_IS_STRING(l) && JSVAL_IS_INT(r)) { + int i; + + i = JSVAL_TO_INT(r); + if ((size_t)i >= JSSTRING_LENGTH(JSVAL_TO_STRING(l))) + ABORT_TRACE("Invalid string index in JSOP_GETELEM"); + LIns* args[] = { f2i(get(&r)), get(&l), cx_ins }; LIns* unitstr_ins = lir->insCall(F_String_getelem, args); guard(false, lir->ins_eq0(unitstr_ins), MISMATCH_EXIT); From a2fdc866ffc76adf5fa04cb26100b9fbfd33c55e Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 29 Aug 2008 14:22:21 -0700 Subject: [PATCH 7/7] Fixed assumptions that nanojit's insCall() would not clobber the input argument array (bug 452853, r=gal). --- js/src/jstracer.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index ea7a19a2da1..ffc780150c4 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -2735,6 +2735,7 @@ TraceRecorder::cmp(LOpcode op, int flags) lnum = js_ValueToNumber(cx, &tmp[0]); args[0] = get(&r); + args[1] = cx_ins; if (JSVAL_IS_STRING(r)) { r_ins = lir->insCall(F_StringToNumber, args); } else if (JSVAL_TAG(r) == JSVAL_BOOLEAN) { @@ -2897,21 +2898,24 @@ TraceRecorder::binary(LOpcode op) bool leftNumber = isNumber(l), rightNumber = isNumber(r); if ((op >= LIR_sub && op <= LIR_ush) || // sub, mul, (callh), or, xor, (not,) lsh, rsh, ush (op >= LIR_fsub && op <= LIR_fdiv)) { // fsub, fmul, fdiv - LIns* args[] = { NULL, cx_ins }; + LIns* args[2]; if (JSVAL_IS_STRING(l)) { args[0] = a; + args[1] = cx_ins; a = lir->insCall(F_StringToNumber, args); leftNumber = true; } if (JSVAL_IS_STRING(r)) { args[0] = b; + args[1] = cx_ins; b = lir->insCall(F_StringToNumber, args); rightNumber = true; } } if (leftNumber && rightNumber) { if (intop) { - a = lir->insCall(op == LIR_ush ? F_DoubleToUint32 : F_DoubleToInt32, &a); + LIns *args[] = { a }; + a = lir->insCall(op == LIR_ush ? F_DoubleToUint32 : F_DoubleToInt32, args); b = f2i(b); } a = lir->ins2(op, a, b); @@ -3214,7 +3218,8 @@ TraceRecorder::unbox_jsval(jsval v, LIns*& v_ins) INS_CONST(JSVAL_TAGMASK)), JSVAL_DOUBLE))), MISMATCH_EXIT); - v_ins = lir->insCall(F_UnboxDouble, &v_ins); + LIns* args[] = { v_ins }; + v_ins = lir->insCall(F_UnboxDouble, args); return true; } switch (JSVAL_TAG(v)) {