From ca4763311f9758d2865c127dfe06afc0edbb2c3a Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Tue, 8 Mar 2011 23:51:27 -0800 Subject: [PATCH 01/13] Fix assertion botched by function using a previously mentioned name and therefore claiming its placeholder or declared definition; the function must have sane end vs. begin source coordinates for error reporting purposes (640075, r=njn). --- js/src/jsparse.cpp | 7 +++ js/src/jsscan.cpp | 61 +++++++++---------- js/src/tests/js1_8_5/regress/jstests.list | 1 + .../tests/js1_8_5/regress/regress-640075.js | 15 +++++ 4 files changed, 52 insertions(+), 32 deletions(-) create mode 100644 js/src/tests/js1_8_5/regress/regress-640075.js diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index ace28e973b7..8db2ddd9ede 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -3103,6 +3103,13 @@ Parser::functionDef(JSAtom *funAtom, FunctionType type, uintN lambda) fn->pn_type = TOK_FUNCTION; fn->pn_arity = PN_FUNC; fn->pn_pos.begin = pn->pn_pos.begin; + + /* + * Set fn->pn_pos.end too, in case of error before we parse the + * closing brace. See bug 640075. + */ + fn->pn_pos.end = pn->pn_pos.end; + fn->pn_body = NULL; fn->pn_cookie.makeFree(); diff --git a/js/src/jsscan.cpp b/js/src/jsscan.cpp index fc1e55b48f6..37cce886511 100644 --- a/js/src/jsscan.cpp +++ b/js/src/jsscan.cpp @@ -399,9 +399,7 @@ TokenStream::reportCompileErrorNumberVA(JSParseNode *pn, uintN flags, uintN erro { JSErrorReport report; char *message; - size_t linelength; jschar *linechars; - const jschar *linelimit; char *linebytes; bool warning; JSBool ok; @@ -441,39 +439,39 @@ TokenStream::reportCompileErrorNumberVA(JSParseNode *pn, uintN flags, uintN erro /* * Given a token, T, that we want to complain about: if T's (starting) - * lineno doesn't match TokenStream's lineno, that means we've scanned - * past the line that T starts on, which makes it hard to print some or - * all of T's (starting) line for context. So we don't even try, leaving - * report.linebuf and friends zeroed. This means that any error involving - * a multi-line token (eg. an unterminated multi-line string literal) - * won't have a context printed. + * lineno doesn't match TokenStream's lineno, that means we've scanned past + * the line that T starts on, which makes it hard to print some or all of + * T's (starting) line for context. + * + * So we don't even try, leaving report.linebuf and friends zeroed. This + * means that any error involving a multi-line token (eg. an unterminated + * multi-line string literal) won't have a context printed. */ - if (report.lineno != lineno) - goto report; + if (report.lineno == lineno) { + size_t linelength = userbuf.findEOL() - linebase; - linelimit = userbuf.findEOL(); - linelength = linelimit - linebase; + linechars = (jschar *)cx->malloc((linelength + 1) * sizeof(jschar)); + if (!linechars) { + warning = false; + goto out; + } + memcpy(linechars, linebase, linelength * sizeof(jschar)); + linechars[linelength] = 0; + linebytes = js_DeflateString(cx, linechars, linelength); + if (!linebytes) { + warning = false; + goto out; + } - linechars = (jschar *)cx->malloc((linelength + 1) * sizeof(jschar)); - if (!linechars) { - warning = false; - goto out; + /* Unicode and char versions of the offending source line, without final \n */ + report.linebuf = linebytes; + report.uclinebuf = linechars; + + /* The lineno check above means we should only see single-line tokens here. */ + JS_ASSERT(tp->begin.lineno == tp->end.lineno); + report.tokenptr = report.linebuf + tp->begin.index; + report.uctokenptr = report.uclinebuf + tp->begin.index; } - memcpy(linechars, linebase, linelength * sizeof(jschar)); - linechars[linelength] = 0; - linebytes = js_DeflateString(cx, linechars, linelength); - if (!linebytes) { - warning = false; - goto out; - } - /* Unicode and char versions of the offending source line, without final \n */ - report.linebuf = linebytes; - report.uclinebuf = linechars; - - /* The lineno check above means we should only see single-line tokens here. */ - JS_ASSERT(tp->begin.lineno == tp->end.lineno); - report.tokenptr = report.linebuf + tp->begin.index; - report.uctokenptr = report.uclinebuf + tp->begin.index; /* * If there's a runtime exception type associated with this error @@ -491,7 +489,6 @@ TokenStream::reportCompileErrorNumberVA(JSParseNode *pn, uintN flags, uintN erro * XXX it'd probably be best if there was only one call to this * function, but there seem to be two error reporter call points. */ - report: onError = cx->errorReporter; /* diff --git a/js/src/tests/js1_8_5/regress/jstests.list b/js/src/tests/js1_8_5/regress/jstests.list index 78846f3be71..79afa408acb 100644 --- a/js/src/tests/js1_8_5/regress/jstests.list +++ b/js/src/tests/js1_8_5/regress/jstests.list @@ -92,3 +92,4 @@ script regress-634210-4.js script regress-635195.js script regress-636394.js script regress-636364.js +script regress-640075.js diff --git a/js/src/tests/js1_8_5/regress/regress-640075.js b/js/src/tests/js1_8_5/regress/regress-640075.js new file mode 100644 index 00000000000..09e5463c2b9 --- /dev/null +++ b/js/src/tests/js1_8_5/regress/regress-640075.js @@ -0,0 +1,15 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + */ + +"use strict"; +try { + eval("(function() { eval(); function eval() {} })"); + assertEq(0, 1); +} catch (e) { + assertEq(e.name, "SyntaxError"); +} + +reportCompare(0, 0, "ok"); From 76f4d326e39e332c89d8db9a32e0741a8e40e504 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 15:21:07 -0800 Subject: [PATCH 02/13] Bug 602397 - TM: clean up TraceRecorder::alu(). r=wmccloskey. --- js/src/jstracer.cpp | 325 +++++++++++++++++++------------------------- js/src/jstracer.h | 8 +- 2 files changed, 142 insertions(+), 191 deletions(-) diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 1419e35779b..7485b2fe39f 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -1713,51 +1713,6 @@ fcallinfo(LIns *ins) return ins->isop(LIR_calld) ? ins->callInfo() : NULL; } -/* - * Determine whether this operand is guaranteed to not overflow the specified - * integer operation. - */ -static void -ChecksRequired(LOpcode op, LIns* op1, LIns* op2, - bool* needsOverflowCheck, bool* needsNegZeroCheck) -{ - Interval x = Interval::of(op1, 3); - Interval y = Interval::of(op2, 3); - Interval z(0, 0); - - switch (op) { - case LIR_addi: - z = Interval::add(x, y); - *needsNegZeroCheck = false; - break; - - case LIR_subi: - z = Interval::sub(x, y); - *needsNegZeroCheck = false; - break; - - case LIR_muli: { - z = Interval::mul(x, y); - // A would-be negative zero result can only occur if we have - // mul(0, -n) or mul(-n, 0), where n != 0. In particular, a multiply - // where one operand is a positive immediate cannot result in negative - // zero. - // - // This assumes that -0 cannot be an operand; if one had occurred we - // would have already exited the trace in order to promote the - // computation back to doubles. - *needsNegZeroCheck = (x.canBeZero() && y.canBeNegative()) || - (y.canBeZero() && x.canBeNegative()); - break; - } - - default: - JS_NOT_REACHED("needsOverflowCheck"); - } - - *needsOverflowCheck = z.hasOverflowed; -} - /* * JSStackFrame::numActualArgs is only defined for function frames. Since the * actual arguments of the entry frame are kept on trace, argc is included in @@ -4469,30 +4424,6 @@ TraceRecorder::guard(bool expected, LIns* cond, ExitType exitType, return guard(expected, cond, snapshot(exitType), abortIfAlwaysExits); } -/* - * Emit a guard a 32-bit integer arithmetic operation op(d0, d1) and - * using the supplied side exit if it overflows. - */ -JS_REQUIRES_STACK LIns* -TraceRecorder::guard_xov(LOpcode op, LIns* d0, LIns* d1, VMSideExit* exit) -{ - JS_ASSERT(exit->exitType == OVERFLOW_EXIT); - - GuardRecord* guardRec = createGuardRecord(exit); - switch (op) { - case LIR_addi: - return w.addxovi(d0, d1, guardRec); - case LIR_subi: - return w.subxovi(d0, d1, guardRec); - case LIR_muli: - return w.mulxovi(d0, d1, guardRec); - default: - break; - } - JS_NOT_REACHED("unexpected opcode"); - return NULL; -} - JS_REQUIRES_STACK VMSideExit* TraceRecorder::copy(VMSideExit* copy) { @@ -8389,17 +8320,31 @@ TraceRecorder::guardNonNeg(LIns* d0, LIns* d1, VMSideExit* exit) } JS_REQUIRES_STACK LIns* -TraceRecorder::alu(LOpcode v, jsdouble v0, jsdouble v1, LIns* s0, LIns* s1) +TraceRecorder::tryToDemote(LOpcode v, jsdouble v0, jsdouble v1, LIns* s0, LIns* s1) { /* - * To even consider this operation for demotion, both operands have to be - * integers and the oracle must not give us a negative hint for the - * instruction. + * If the operands and result of an arithmetic operation are all integers + * at record-time, and the oracle doesn't direct us otherwise, we + * speculatively emit a demoted (integer) operation, betting that at + * runtime we will get integer results again. + * + * We also have to protect against various edge cases. For example, + * to protect against overflow we emit a guard that will inform the oracle + * on overflow and cause a non-demoted trace to be attached that uses + * floating-point math for this operation; the exception to this case is + * if the operands guarantee that the result will be an integer (e.g. + * z = d0 * d1 with 0 <= (d0|d1) <= 0xffff guarantees z <= fffe0001). */ + if (!oracle || oracle->isInstructionUndemotable(cx->regs->pc) || - !IsPromotedInt32(s0) || !IsPromotedInt32(s1)) { - out: + !IsPromotedInt32(s0) || !IsPromotedInt32(s1)) + { + undemotable: if (v == LIR_modd) { + /* + * LIR_modd is a placeholder that Nanojit doesn't actually support! + * Convert it to a call. + */ LIns* args[] = { s1, s0 }; return w.call(&js_dmod_ci, args); } @@ -8408,58 +8353,96 @@ TraceRecorder::alu(LOpcode v, jsdouble v0, jsdouble v1, LIns* s0, LIns* s1) return result; } - jsdouble r; - switch (v) { - case LIR_addd: - r = v0 + v1; - break; - case LIR_subd: - r = v0 - v1; - break; - case LIR_muld: - r = v0 * v1; - if (r == 0.0 && (v0 < 0.0 || v1 < 0.0)) - goto out; - break; -#if defined NANOJIT_IA32 || defined NANOJIT_X64 - case LIR_divd: - if (v1 == 0) - goto out; - r = v0 / v1; - break; - case LIR_modd: - if (v0 < 0 || v1 == 0 || (s1->isImmD() && v1 < 0)) - goto out; - r = js_dmod(v0, v1); - break; -#endif - default: - goto out; - } - - /* - * The result must be an integer at record time, otherwise there is no - * point in trying to demote it. - */ - if (jsint(r) != r || JSDOUBLE_IS_NEGZERO(r)) - goto out; - LIns* d0 = w.demoteToInt32(s0); LIns* d1 = w.demoteToInt32(s1); - - /* - * Speculatively emit an integer operation, betting that at runtime we - * will get integer results again. - */ - VMSideExit* exit = NULL; + jsdouble r; LIns* result; + switch (v) { + case LIR_addd: { + r = v0 + v1; + if (jsint(r) != r || JSDOUBLE_IS_NEGZERO(r)) + goto undemotable; + + Interval i0 = Interval::of(d0, 3); + Interval i1 = Interval::of(d1, 3); + result = Interval::add(i0, i1).hasOverflowed + ? w.addxovi(d0, d1, createGuardRecord(snapshot(OVERFLOW_EXIT))) + : w.addi(d0, d1); + break; + } + + case LIR_subd: { + r = v0 - v1; + if (jsint(r) != r || JSDOUBLE_IS_NEGZERO(r)) + goto undemotable; + + Interval i0 = Interval::of(d0, 3); + Interval i1 = Interval::of(d1, 3); + result = Interval::sub(i0, i1).hasOverflowed + ? w.subxovi(d0, d1, createGuardRecord(snapshot(OVERFLOW_EXIT))) + : w.subi(d0, d1); + break; + } + + case LIR_muld: { + r = v0 * v1; + if (jsint(r) != r || JSDOUBLE_IS_NEGZERO(r)) + goto undemotable; + if (r == 0.0 && (v0 < 0.0 || v1 < 0.0)) + goto undemotable; + + Interval i0 = Interval::of(d0, 3); + Interval i1 = Interval::of(d1, 3); + result = Interval::mul(i0, i1).hasOverflowed + ? w.mulxovi(d0, d1, createGuardRecord(snapshot(OVERFLOW_EXIT))) + : w.muli(d0, d1); + + /* + * A would-be negative zero result can only occur if we have + * mul(0, -n) or mul(-n, 0), where n != 0. In particular, a multiply + * where one operand is a positive immediate cannot result in negative + * zero. + * + * This assumes that -0 cannot be an operand; if one had occurred we + * would have already exited the trace in order to promote the + * computation back to doubles. + */ + bool needsNegZeroCheck = (i0.canBeZero() && i1.canBeNegative()) || + (i1.canBeZero() && i0.canBeNegative()); + if (needsNegZeroCheck) { + /* + * Make sure we don't lose a -0. We exit if the result is zero and if + * either operand is negative. We start out using a weaker guard, checking + * if either argument is negative. If this ever fails, we recompile with + * a stronger, but slower, guard. + */ + if (v0 < 0.0 || v1 < 0.0 || oracle->isInstructionSlowZeroTest(cx->regs->pc)) { + guard(true, + w.eqi0(w.andi(w.eqi0(result), + w.ori(w.ltiN(d0, 0), + w.ltiN(d1, 0)))), + snapshot(OVERFLOW_EXIT)); + } else { + guardNonNeg(d0, d1, snapshot(MUL_ZERO_EXIT)); + } + } + break; + } + #if defined NANOJIT_IA32 || defined NANOJIT_X64 - case LIR_divd: + case LIR_divd: { + if (v1 == 0) + goto undemotable; + r = v0 / v1; + if (jsint(r) != r || JSDOUBLE_IS_NEGZERO(r)) + goto undemotable; + + /* Check for this case ourselves; Nanojit won't do it for us. */ if (d0->isImmI() && d1->isImmI()) return w.i2d(w.immi(jsint(r))); - exit = snapshot(OVERFLOW_EXIT); + VMSideExit* exit = snapshot(OVERFLOW_EXIT); /* * If the divisor is greater than zero its always safe to execute @@ -8473,9 +8456,8 @@ TraceRecorder::alu(LOpcode v, jsdouble v0, jsdouble v1, LIns* s0, LIns* s1) w.eqiN(d1, -1))), exit); w.label(mbr); } - } else { - if (d1->immI() == -1) - guard(false, w.eqiN(d0, 0x80000000), exit); + } else if (d1->immI() == -1) { + guard(false, w.eqiN(d0, 0x80000000), exit); } v = LIR_divi; result = w.divi(d0, d1); @@ -8485,13 +8467,22 @@ TraceRecorder::alu(LOpcode v, jsdouble v0, jsdouble v1, LIns* s0, LIns* s1) /* Don't lose a -0. */ guard(false, w.eqi0(result), exit); + break; + } case LIR_modd: { + if (v0 < 0 || v1 == 0 || (s1->isImmD() && v1 < 0)) + goto undemotable; + r = js_dmod(v0, v1); + if (jsint(r) != r || JSDOUBLE_IS_NEGZERO(r)) + goto undemotable; + + /* Check for this case ourselves; Nanojit won't do it for us. */ if (d0->isImmI() && d1->isImmI()) return w.i2d(w.immi(jsint(r))); - exit = snapshot(OVERFLOW_EXIT); + VMSideExit* exit = snapshot(OVERFLOW_EXIT); /* Make sure we don't trigger division by zero at runtime. */ if (!d1->isImmI()) @@ -8499,12 +8490,12 @@ TraceRecorder::alu(LOpcode v, jsdouble v0, jsdouble v1, LIns* s0, LIns* s1) v = LIR_modi; result = w.modi(w.divi(d0, d1)); - /* If the result is not 0, it is always within the integer domain. */ + /* + * If the result is not 0, it is always within the integer domain. + * Otherwise, we must exit if the lhs is negative since the result is + * -0 in this case, which is not in the integer domain. + */ if (MaybeBranch mbr = w.jf(w.eqi0(result))) { - /* - * If the result is zero, we must exit if the lhs is negative since - * the result is -0 in this case, which is not in the integer domain. - */ guard(false, w.ltiN(d0, 0), exit); w.label(mbr); } @@ -8512,50 +8503,16 @@ TraceRecorder::alu(LOpcode v, jsdouble v0, jsdouble v1, LIns* s0, LIns* s1) } #endif - default: - v = arithOpcodeD2I(v); - JS_ASSERT(v == LIR_addi || v == LIR_muli || v == LIR_subi); - - /* - * If the operands guarantee that the result will be an integer (e.g. - * z = x * y with 0 <= (x|y) <= 0xffff guarantees z <= fffe0001), we - * don't have to guard against an overflow. Otherwise we emit a guard - * that will inform the oracle and cause a non-demoted trace to be - * attached that uses floating-point math for this operation. - */ - bool needsOverflowCheck = true, needsNegZeroCheck = true; - ChecksRequired(v, d0, d1, &needsOverflowCheck, &needsNegZeroCheck); - if (needsOverflowCheck) { - exit = snapshot(OVERFLOW_EXIT); - result = guard_xov(v, d0, d1, exit); - } else { - result = w.ins2(v, d0, d1); - } - if (needsNegZeroCheck) { - JS_ASSERT(v == LIR_muli); - /* - * Make sure we don't lose a -0. We exit if the result is zero and if - * either operand is negative. We start out using a weaker guard, checking - * if either argument is negative. If this ever fails, we recompile with - * a stronger, but slower, guard. - */ - if (v0 < 0.0 || v1 < 0.0 - || !oracle || oracle->isInstructionSlowZeroTest(cx->regs->pc)) - { - if (!exit) - exit = snapshot(OVERFLOW_EXIT); - - guard(true, - w.eqi0(w.andi(w.eqi0(result), - w.ori(w.ltiN(d0, 0), - w.ltiN(d1, 0)))), - exit); - } else { - guardNonNeg(d0, d1, snapshot(MUL_ZERO_EXIT)); - } - } + default: + JS_NOT_REACHED("tryToDemote"); + result = NULL; break; } + + /* + * Successful demotion! Convert result to a double. This i2d will be + * removed if the result feeds into another integer or demoted operation. + */ JS_ASSERT_IF(d0->isImmI() && d1->isImmI(), result->isImmI(jsint(r))); return w.i2d(result); } @@ -8861,7 +8818,7 @@ TraceRecorder::incHelper(const Value &v, LIns*& v_ins, Value &v_after, AutoValueRooter tvr(cx); *tvr.addr() = v; ValueToNumber(cx, tvr.value(), &num); - v_ins_after = alu(LIR_addd, num, incr, v_ins, w.immd(incr)); + v_ins_after = tryToDemote(LIR_addd, num, incr, v_ins, w.immd(incr)); v_after.setDouble(num + incr); } @@ -9315,17 +9272,13 @@ TraceRecorder::relational(LOpcode op, bool tryBranchAfterCond) } JS_REQUIRES_STACK RecordingStatus -TraceRecorder::unary(LOpcode op) +TraceRecorder::unaryIntOp(LOpcode op) { Value& v = stackval(-1); - bool intop = retTypes[op] == LTy_I; + JS_ASSERT(retTypes[op] == LTy_I); if (v.isNumber()) { LIns* a = get(&v); - if (intop) - a = d2i(a); - a = w.ins1(op, a); - if (intop) - a = w.i2d(a); + a = w.i2d(w.ins1(op, d2i(a))); set(&v, a); return RECORD_CONTINUE; } @@ -9409,12 +9362,12 @@ TraceRecorder::binary(LOpcode op) } if (leftIsNumber && rightIsNumber) { if (intop) { - a = (op == LIR_rshui) ? d2u(a) : d2i(a); - b = d2i(b); + a = (op == LIR_rshui) + ? w.ui2d(w.ins2(op, d2u(a), d2i(b))) + : w.i2d(w.ins2(op, d2i(a), d2i(b))); + } else { + a = tryToDemote(op, lnum, rnum, a, b); } - a = alu(op, lnum, rnum, a, b); - if (intop) - a = (op == LIR_rshui) ? w.ui2d(a) : w.i2d(a); set(&l, a); return RECORD_CONTINUE; } @@ -10832,7 +10785,7 @@ TraceRecorder::record_JSOP_NOT() JS_REQUIRES_STACK AbortableRecordingStatus TraceRecorder::record_JSOP_BITNOT() { - return InjectStatus(unary(LIR_noti)); + return InjectStatus(unaryIntOp(LIR_noti)); } JS_REQUIRES_STACK AbortableRecordingStatus @@ -10861,7 +10814,7 @@ TraceRecorder::record_JSOP_NEG() -v.toNumber() == (int)-v.toNumber()) { VMSideExit* exit = snapshot(OVERFLOW_EXIT); - a = guard_xov(LIR_subi, w.immi(0), w.demoteToInt32(a), exit); + a = w.subxovi(w.immi(0), w.demoteToInt32(a), createGuardRecord(exit)); if (!a->isImmI() && a->isop(LIR_subxovi)) { guard(false, w.eqiN(a, 0), exit); // make sure we don't lose a -0 } @@ -15754,7 +15707,7 @@ TraceRecorder::record_JSOP_ARGCNT() // interpreter, so we have to check for that in the trace entry frame. // We also have to check that arguments.length has not been mutated // at record time, because if so we will generate incorrect constant - // LIR, which will assert in alu(). + // LIR, which will assert in tryToDemote(). if (fp->hasArgsObj() && fp->argsObj().isArgsLengthOverridden()) RETURN_STOP_A("can't trace JSOP_ARGCNT if arguments.length has been modified"); LIns *a_ins = getFrameObjPtr(fp->addressOfArgs()); diff --git a/js/src/jstracer.h b/js/src/jstracer.h index e9c9d102a58..e91e9f9c406 100644 --- a/js/src/jstracer.h +++ b/js/src/jstracer.h @@ -1199,8 +1199,6 @@ class TraceRecorder bool abortIfAlwaysExits = false); JS_REQUIRES_STACK RecordingStatus guard(bool expected, nanojit::LIns* cond, VMSideExit* exit, bool abortIfAlwaysExits = false); - JS_REQUIRES_STACK nanojit::LIns* guard_xov(nanojit::LOpcode op, nanojit::LIns* d0, - nanojit::LIns* d1, VMSideExit* exit); nanojit::LIns* writeBack(nanojit::LIns* i, nanojit::LIns* base, ptrdiff_t offset, bool shouldDemoteToInt32); @@ -1276,8 +1274,8 @@ class TraceRecorder JS_REQUIRES_STACK void stack(int n, nanojit::LIns* i); JS_REQUIRES_STACK void guardNonNeg(nanojit::LIns* d0, nanojit::LIns* d1, VMSideExit* exit); - JS_REQUIRES_STACK nanojit::LIns* alu(nanojit::LOpcode op, jsdouble v0, jsdouble v1, - nanojit::LIns* s0, nanojit::LIns* s1); + JS_REQUIRES_STACK nanojit::LIns* tryToDemote(nanojit::LOpcode op, jsdouble v0, jsdouble v1, + nanojit::LIns* s0, nanojit::LIns* s1); nanojit::LIns* d2i(nanojit::LIns* f, bool resultCanBeImpreciseIfFractional = false); nanojit::LIns* d2u(nanojit::LIns* d); @@ -1313,7 +1311,7 @@ class TraceRecorder Value& rval); JS_REQUIRES_STACK AbortableRecordingStatus relational(nanojit::LOpcode op, bool tryBranchAfterCond); - JS_REQUIRES_STACK RecordingStatus unary(nanojit::LOpcode op); + JS_REQUIRES_STACK RecordingStatus unaryIntOp(nanojit::LOpcode op); JS_REQUIRES_STACK RecordingStatus binary(nanojit::LOpcode op); JS_REQUIRES_STACK RecordingStatus guardShape(nanojit::LIns* obj_ins, JSObject* obj, From 119428d74f4539685e092059f4c80691a230d1d1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 15:26:38 -0800 Subject: [PATCH 03/13] Bug 640076 - TOK_XMLATTR tokens can span multiple lines. r=brendan. --- js/src/jsscan.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/js/src/jsscan.cpp b/js/src/jsscan.cpp index 37cce886511..c23a1c5b89c 100644 --- a/js/src/jsscan.cpp +++ b/js/src/jsscan.cpp @@ -806,6 +806,7 @@ IsTokenSane(Token *tp) /* Only certain token kinds can be multi-line. */ switch (tp->type) { case TOK_STRING: + case TOK_XMLATTR: case TOK_XMLSPACE: case TOK_XMLTEXT: case TOK_XMLCOMMENT: From f7477c2d888bee14bfd56052f9e4e8f2f6cf09fe Mon Sep 17 00:00:00 2001 From: Paul Biggar Date: Wed, 9 Mar 2011 15:52:21 -0800 Subject: [PATCH 04/13] Bug 640085 - Fix segfault in shark builds (r=gal) --- js/src/shell/js.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 0ed4edf79c8..24fa0eee75d 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -4752,8 +4752,6 @@ static const char *const shell_help_messages[] = { JS_STATIC_ASSERT(JS_ARRAY_LENGTH(shell_help_messages) - PROFILING_FUNCTION_COUNT == JS_ARRAY_LENGTH(shell_functions) - 1 /* JS_FS_END */); -#undef PROFILING_FUNCTION_COUNT - #ifdef DEBUG static void CheckHelpMessages() @@ -4762,7 +4760,7 @@ CheckHelpMessages() const char *lp; /* Messages begin with "function_name(" prefix and don't end with \n. */ - for (m = shell_help_messages; m != JS_ARRAY_END(shell_help_messages); ++m) { + for (m = shell_help_messages; m != JS_ARRAY_END(shell_help_messages) - PROFILING_FUNCTION_COUNT; ++m) { lp = strchr(*m, '('); JS_ASSERT(lp); JS_ASSERT(memcmp(shell_functions[m - shell_help_messages].name, @@ -4774,6 +4772,8 @@ CheckHelpMessages() # define CheckHelpMessages() ((void) 0) #endif +#undef PROFILING_FUNCTION_COUNT + static JSBool Help(JSContext *cx, uintN argc, jsval *vp) { From 7115ff9cf1873c3d24b38dbb737d0288a9866b36 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 19:10:41 -0800 Subject: [PATCH 05/13] Bug 639743 - JM: clean up Executable{Pool,Allocator} some more, 1/6. r=dvander. --- js/src/assembler/jit/ExecutableAllocator.h | 20 ++------------------ js/src/jscompartment.cpp | 2 +- js/src/methodjit/MethodJIT.cpp | 2 +- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/js/src/assembler/jit/ExecutableAllocator.h b/js/src/assembler/jit/ExecutableAllocator.h index 115f4ff33ff..94a063e72d1 100644 --- a/js/src/assembler/jit/ExecutableAllocator.h +++ b/js/src/assembler/jit/ExecutableAllocator.h @@ -197,30 +197,14 @@ private: class ExecutableAllocator { enum ProtectionSeting { Writable, Executable }; - // Initialization can fail so we use a create method instead. - ExecutableAllocator() {} public: static size_t pageSize; - // Returns NULL on OOM. - static ExecutableAllocator *create() + ExecutableAllocator() { - /* We can't (easily) use js_new() here because the constructor is private. */ - void *memory = js_malloc(sizeof(ExecutableAllocator)); - ExecutableAllocator *allocator = memory ? new(memory) ExecutableAllocator() : NULL; - if (!allocator) - return allocator; - if (!pageSize) intializePageSize(); - ExecutablePool *pool = ExecutablePool::create(JIT_ALLOCATOR_LARGE_ALLOC_SIZE); - if (!pool) { - js_delete(allocator); - return NULL; - } - JS_ASSERT(allocator->m_smallAllocationPools.empty()); - allocator->m_smallAllocationPools.append(pool); - return allocator; + JS_ASSERT(m_smallAllocationPools.empty()); } ~ExecutableAllocator() diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 4ee7b055dc9..1fb9d484ffd 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -149,7 +149,7 @@ JSCompartment::init() #endif #if ENABLE_YARR_JIT - regExpAllocator = JSC::ExecutableAllocator::create(); + regExpAllocator = js_new(); if (!regExpAllocator) return false; #endif diff --git a/js/src/methodjit/MethodJIT.cpp b/js/src/methodjit/MethodJIT.cpp index cdd9b6c1e8a..71e71881daf 100644 --- a/js/src/methodjit/MethodJIT.cpp +++ b/js/src/methodjit/MethodJIT.cpp @@ -689,7 +689,7 @@ JS_STATIC_ASSERT(JSVAL_PAYLOAD_MASK == 0x00007FFFFFFFFFFFLL); bool JaegerCompartment::Initialize() { - execAlloc_ = JSC::ExecutableAllocator::create(); + execAlloc_ = js_new(); if (!execAlloc_) return false; From e3dd8f0822506e215f897572f77daebfb0cba4ce Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 19:11:07 -0800 Subject: [PATCH 06/13] Bug 639743 - JM: clean up Executable{Pool,Allocator} some more, 2/6. r=dvander. --- js/src/assembler/jit/ExecutableAllocator.h | 73 +++++++++------------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/js/src/assembler/jit/ExecutableAllocator.h b/js/src/assembler/jit/ExecutableAllocator.h index 94a063e72d1..effeaf542a0 100644 --- a/js/src/assembler/jit/ExecutableAllocator.h +++ b/js/src/assembler/jit/ExecutableAllocator.h @@ -102,8 +102,7 @@ inline size_t roundUpAllocationSize(size_t request, size_t granularity) namespace JSC { - // These are reference-counted. A new one (from the constructor or create) - // starts with a count of 1. + // These are reference-counted. A new one starts with a count of 1. class ExecutablePool { friend class ExecutableAllocator; private: @@ -148,16 +147,15 @@ public: } private: - static ExecutablePool* create(size_t n) + ExecutablePool(Allocation a) + : m_freePtr(a.pages), m_end(m_freePtr + a.size), m_allocation(a), m_refCount(1), + m_destroy(false), m_gcNumber(0) + { } + + ~ExecutablePool() { - /* We can't (easily) use js_new() here because the constructor is private. */ - void *memory = js_malloc(sizeof(ExecutablePool)); - ExecutablePool *pool = memory ? new(memory) ExecutablePool(n) : NULL; - if (!pool || !pool->m_freePtr) { - pool->destroy(); - return NULL; - } - return pool; + if (m_allocation.pages) + ExecutablePool::systemRelease(m_allocation); } void* alloc(size_t n) @@ -176,12 +174,6 @@ private: js_free(this); } - ~ExecutablePool() - { - if (m_allocation.pages) - ExecutablePool::systemRelease(m_allocation); - } - size_t available() const { JS_ASSERT(m_end >= m_freePtr); return m_end - m_freePtr; @@ -190,8 +182,6 @@ private: // On OOM, this will return an Allocation where pages is NULL. static Allocation systemAlloc(size_t n); static void systemRelease(const Allocation& alloc); - - ExecutablePool(size_t n); }; class ExecutableAllocator { @@ -239,6 +229,24 @@ public: } private: + ExecutablePool* createPool(size_t n) + { + size_t allocSize = roundUpAllocationSize(n, JIT_ALLOCATOR_PAGE_SIZE); + if (allocSize == OVERSIZE_ALLOCATION) + return NULL; +#ifdef DEBUG_STRESS_JSC_ALLOCATOR + ExecutablePool::Allocation a = ExecutablePool::systemAlloc(size_t(4294967291)); +#else + ExecutablePool::Allocation a = ExecutablePool::systemAlloc(allocSize); +#endif + if (!a.pages) + return NULL; + + /* We can't (easily) use js_new() here because the constructor is private. */ + void *memory = js_malloc(sizeof(ExecutablePool)); + return memory ? new(memory) ExecutablePool(a) : NULL; + } + ExecutablePool* poolForSize(size_t n) { #ifndef DEBUG_STRESS_JSC_ALLOCATOR @@ -261,10 +269,10 @@ private: // If the request is large, we just provide a unshared allocator if (n > JIT_ALLOCATOR_LARGE_ALLOC_SIZE) - return ExecutablePool::create(n); + return createPool(n); // Create a new allocator - ExecutablePool* pool = ExecutablePool::create(JIT_ALLOCATOR_LARGE_ALLOC_SIZE); + ExecutablePool* pool = createPool(JIT_ALLOCATOR_LARGE_ALLOC_SIZE); if (!pool) return NULL; // At this point, local |pool| is the owner. @@ -410,29 +418,6 @@ private: static void intializePageSize(); }; -// This constructor can fail due to OOM. If it does, m_freePtr will be -// set to NULL. -inline ExecutablePool::ExecutablePool(size_t n) : m_refCount(1), m_destroy(false), m_gcNumber(0) -{ - size_t allocSize = roundUpAllocationSize(n, JIT_ALLOCATOR_PAGE_SIZE); - if (allocSize == OVERSIZE_ALLOCATION) { - m_freePtr = NULL; - return; - } -#ifdef DEBUG_STRESS_JSC_ALLOCATOR - Allocation mem = systemAlloc(size_t(4294967291)); -#else - Allocation mem = systemAlloc(allocSize); -#endif - if (!mem.pages) { - m_freePtr = NULL; - return; - } - m_allocation = mem; - m_freePtr = mem.pages; - m_end = m_freePtr + allocSize; -} - } #endif // ENABLE(ASSEMBLER) From f355e8d6ef95f06b526b2b97b6efd1c71e7699a9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 19:11:59 -0800 Subject: [PATCH 07/13] Bug 639743 - JM: clean up Executable{Pool,Allocator} some more, 3/6. r=dvander. --- js/src/assembler/jit/ExecutableAllocator.h | 45 ++++++++++------------ 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/js/src/assembler/jit/ExecutableAllocator.h b/js/src/assembler/jit/ExecutableAllocator.h index effeaf542a0..c50c8931dd4 100644 --- a/js/src/assembler/jit/ExecutableAllocator.h +++ b/js/src/assembler/jit/ExecutableAllocator.h @@ -72,30 +72,6 @@ extern "C" __declspec(dllimport) void CacheRangeFlush(LPVOID pAddr, DWORD dwLeng #define INITIAL_PROTECTION_FLAGS (PROT_READ | PROT_WRITE | PROT_EXEC) #endif -namespace JSC { - -// Something included via windows.h defines a macro with this name, -// which causes the function below to fail to compile. -#ifdef _MSC_VER -# undef max -#endif - -const size_t OVERSIZE_ALLOCATION = size_t(-1); - -inline size_t roundUpAllocationSize(size_t request, size_t granularity) -{ - if ((std::numeric_limits::max() - granularity) <= request) - return OVERSIZE_ALLOCATION; - - // Round up to next page boundary - size_t size = request + (granularity - 1); - size = size & ~(granularity - 1); - JS_ASSERT(size >= request); - return size; -} - -} - #if ENABLE_ASSEMBLER //#define DEBUG_STRESS_JSC_ALLOCATOR @@ -160,7 +136,6 @@ private: void* alloc(size_t n) { - JS_ASSERT(n != OVERSIZE_ALLOCATION); JS_ASSERT(n <= available()); void *result = m_freePtr; m_freePtr += n; @@ -229,6 +204,26 @@ public: } private: + static const size_t OVERSIZE_ALLOCATION = size_t(-1); + + static size_t roundUpAllocationSize(size_t request, size_t granularity) + { + // Something included via windows.h defines a macro with this name, + // which causes the function below to fail to compile. + #ifdef _MSC_VER + # undef max + #endif + + if ((std::numeric_limits::max() - granularity) <= request) + return OVERSIZE_ALLOCATION; + + // Round up to next page boundary + size_t size = request + (granularity - 1); + size = size & ~(granularity - 1); + JS_ASSERT(size >= request); + return size; + } + ExecutablePool* createPool(size_t n) { size_t allocSize = roundUpAllocationSize(n, JIT_ALLOCATOR_PAGE_SIZE); From 5f718ef2816955a108806fbb62f8e79f8c556594 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 19:12:26 -0800 Subject: [PATCH 08/13] Bug 639743 - JM: clean up Executable{Pool,Allocator} some more, 4/6. r=dvander. --- js/src/assembler/jit/ExecutableAllocator.h | 4 ++-- js/src/assembler/jit/ExecutableAllocatorOS2.cpp | 4 ++-- js/src/assembler/jit/ExecutableAllocatorPosix.cpp | 4 ++-- js/src/assembler/jit/ExecutableAllocatorSymbian.cpp | 6 +++--- js/src/assembler/jit/ExecutableAllocatorWin.cpp | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/js/src/assembler/jit/ExecutableAllocator.h b/js/src/assembler/jit/ExecutableAllocator.h index c50c8931dd4..6fc21fe7934 100644 --- a/js/src/assembler/jit/ExecutableAllocator.h +++ b/js/src/assembler/jit/ExecutableAllocator.h @@ -168,7 +168,7 @@ public: ExecutableAllocator() { if (!pageSize) - intializePageSize(); + pageSize = determinePageSize(); JS_ASSERT(m_smallAllocationPools.empty()); } @@ -410,7 +410,7 @@ private: static const size_t maxSmallPools = 4; typedef js::Vector SmallExecPoolVector; SmallExecPoolVector m_smallAllocationPools; - static void intializePageSize(); + static size_t determinePageSize(); }; } diff --git a/js/src/assembler/jit/ExecutableAllocatorOS2.cpp b/js/src/assembler/jit/ExecutableAllocatorOS2.cpp index 217bbc5a9b3..03089bd8adc 100644 --- a/js/src/assembler/jit/ExecutableAllocatorOS2.cpp +++ b/js/src/assembler/jit/ExecutableAllocatorOS2.cpp @@ -33,9 +33,9 @@ namespace JSC { -void ExecutableAllocator::intializePageSize() +size_t ExecutableAllocator::determinePageSize() { - ExecutableAllocator::pageSize = 4096u; + return 4096u; } ExecutablePool::Allocation ExecutablePool::systemAlloc(size_t n) diff --git a/js/src/assembler/jit/ExecutableAllocatorPosix.cpp b/js/src/assembler/jit/ExecutableAllocatorPosix.cpp index 43370d20d5a..b67e21e6789 100644 --- a/js/src/assembler/jit/ExecutableAllocatorPosix.cpp +++ b/js/src/assembler/jit/ExecutableAllocatorPosix.cpp @@ -33,9 +33,9 @@ namespace JSC { -void ExecutableAllocator::intializePageSize() +size_t ExecutableAllocator::determinePageSize() { - ExecutableAllocator::pageSize = getpagesize(); + return getpagesize(); } ExecutablePool::Allocation ExecutablePool::systemAlloc(size_t n) diff --git a/js/src/assembler/jit/ExecutableAllocatorSymbian.cpp b/js/src/assembler/jit/ExecutableAllocatorSymbian.cpp index 305adb24305..73e3358a91f 100644 --- a/js/src/assembler/jit/ExecutableAllocatorSymbian.cpp +++ b/js/src/assembler/jit/ExecutableAllocatorSymbian.cpp @@ -32,18 +32,18 @@ const size_t MOVING_MEM_PAGE_SIZE = 256 * 1024; namespace JSC { -void ExecutableAllocator::intializePageSize() +size_t ExecutableAllocator::determinePageSize() { #if WTF_CPU_ARMV5_OR_LOWER // The moving memory model (as used in ARMv5 and earlier platforms) // on Symbian OS limits the number of chunks for each process to 16. // To mitigate this limitation increase the pagesize to // allocate less of larger chunks. - ExecutableAllocator::pageSize = MOVING_MEM_PAGE_SIZE; + return MOVING_MEM_PAGE_SIZE; #else TInt page_size; UserHal::PageSizeInBytes(page_size); - ExecutableAllocator::pageSize = page_size; + return page_size; #endif } diff --git a/js/src/assembler/jit/ExecutableAllocatorWin.cpp b/js/src/assembler/jit/ExecutableAllocatorWin.cpp index e59564a01e2..9d24b46ab1d 100644 --- a/js/src/assembler/jit/ExecutableAllocatorWin.cpp +++ b/js/src/assembler/jit/ExecutableAllocatorWin.cpp @@ -32,11 +32,11 @@ namespace JSC { -void ExecutableAllocator::intializePageSize() +size_t ExecutableAllocator::determinePageSize() { SYSTEM_INFO system_info; GetSystemInfo(&system_info); - ExecutableAllocator::pageSize = system_info.dwPageSize; + return system_info.dwPageSize; } ExecutablePool::Allocation ExecutablePool::systemAlloc(size_t n) From 0e0267473049b011c7e914d33fb9878dbc7ee6d1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 19:13:01 -0800 Subject: [PATCH 09/13] Bug 639743 - JM: clean up Executable{Pool,Allocator} some more, 5/6. r=dvander. --- js/src/assembler/jit/ExecutableAllocator.cpp | 1 + js/src/assembler/jit/ExecutableAllocator.h | 34 +++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/js/src/assembler/jit/ExecutableAllocator.cpp b/js/src/assembler/jit/ExecutableAllocator.cpp index a00c497fa46..8c1eebc6813 100644 --- a/js/src/assembler/jit/ExecutableAllocator.cpp +++ b/js/src/assembler/jit/ExecutableAllocator.cpp @@ -30,6 +30,7 @@ namespace JSC { size_t ExecutableAllocator::pageSize = 0; +size_t ExecutableAllocator::largeAllocSize = 0; } diff --git a/js/src/assembler/jit/ExecutableAllocator.h b/js/src/assembler/jit/ExecutableAllocator.h index 6fc21fe7934..ec3be1295ab 100644 --- a/js/src/assembler/jit/ExecutableAllocator.h +++ b/js/src/assembler/jit/ExecutableAllocator.h @@ -54,16 +54,6 @@ extern "C" __declspec(dllimport) void CacheRangeFlush(LPVOID pAddr, DWORD dwLength, DWORD dwFlags); #endif -#define JIT_ALLOCATOR_PAGE_SIZE (ExecutableAllocator::pageSize) -/* - * On Windows, VirtualAlloc effectively allocates in 64K chunks. (Technically, - * it allocates in page chunks, but the starting address is always a multiple - * of 64K, so each allocation uses up 64K of address space.) So a size less - * than that would be pointless. But it turns out that 64KB is a reasonable - * size for all platforms. - */ -#define JIT_ALLOCATOR_LARGE_ALLOC_SIZE (ExecutableAllocator::pageSize * 16) - #if ENABLE_ASSEMBLER_WX_EXCLUSIVE #define PROTECTION_FLAGS_RW (PROT_READ | PROT_WRITE) #define PROTECTION_FLAGS_RX (PROT_READ | PROT_EXEC) @@ -163,12 +153,21 @@ class ExecutableAllocator { enum ProtectionSeting { Writable, Executable }; public: - static size_t pageSize; - ExecutableAllocator() { - if (!pageSize) + if (!pageSize) { pageSize = determinePageSize(); + /* + * On Windows, VirtualAlloc effectively allocates in 64K chunks. + * (Technically, it allocates in page chunks, but the starting + * address is always a multiple of 64K, so each allocation uses up + * 64K of address space.) So a size less than that would be + * pointless. But it turns out that 64KB is a reasonable size for + * all platforms. (This assumes 4KB pages.) + */ + largeAllocSize = pageSize * 16; + } + JS_ASSERT(m_smallAllocationPools.empty()); } @@ -204,6 +203,9 @@ public: } private: + static size_t pageSize; + static size_t largeAllocSize; + static const size_t OVERSIZE_ALLOCATION = size_t(-1); static size_t roundUpAllocationSize(size_t request, size_t granularity) @@ -226,7 +228,7 @@ private: ExecutablePool* createPool(size_t n) { - size_t allocSize = roundUpAllocationSize(n, JIT_ALLOCATOR_PAGE_SIZE); + size_t allocSize = roundUpAllocationSize(n, pageSize); if (allocSize == OVERSIZE_ALLOCATION) return NULL; #ifdef DEBUG_STRESS_JSC_ALLOCATOR @@ -263,11 +265,11 @@ private: #endif // If the request is large, we just provide a unshared allocator - if (n > JIT_ALLOCATOR_LARGE_ALLOC_SIZE) + if (n > largeAllocSize) return createPool(n); // Create a new allocator - ExecutablePool* pool = createPool(JIT_ALLOCATOR_LARGE_ALLOC_SIZE); + ExecutablePool* pool = createPool(largeAllocSize); if (!pool) return NULL; // At this point, local |pool| is the owner. From d08f2f1dfa424391763e7ecdf7cbb4fd81fea725 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Mar 2011 19:13:29 -0800 Subject: [PATCH 10/13] Bug 639743 - JM: clean up Executable{Pool,Allocator} some more, 6/6. r=dvander. --- js/src/assembler/jit/ExecutableAllocator.h | 29 ++++++++++------------ 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/js/src/assembler/jit/ExecutableAllocator.h b/js/src/assembler/jit/ExecutableAllocator.h index ec3be1295ab..9e687337bd2 100644 --- a/js/src/assembler/jit/ExecutableAllocator.h +++ b/js/src/assembler/jit/ExecutableAllocator.h @@ -95,7 +95,18 @@ public: // remember whether m_destroy was computed for the currently active GC. size_t m_gcNumber; -public: + void release(bool willDestroy = false) + { + JS_ASSERT(m_refCount != 0); + JS_ASSERT_IF(willDestroy, m_refCount = 1); + if (--m_refCount == 0) { + /* We can't (easily) use js_delete() here because the destructor is private. */ + this->~ExecutablePool(); + js_free(this); + } + } + +private: // It should be impossible for us to roll over, because only small // pools have multiple holders, and they have one holder per chunk // of generated code, and they only hold 16KB or so of code. @@ -105,13 +116,6 @@ public: ++m_refCount; } - void release() - { - JS_ASSERT(m_refCount != 0); - if (--m_refCount == 0) - this->destroy(); - } - private: ExecutablePool(Allocation a) : m_freePtr(a.pages), m_end(m_freePtr + a.size), m_allocation(a), m_refCount(1), @@ -132,13 +136,6 @@ private: return result; } - void destroy() - { - /* We can't (easily) use js_delete() here because the destructor is private. */ - this->~ExecutablePool(); - js_free(this); - } - size_t available() const { JS_ASSERT(m_end >= m_freePtr); return m_end - m_freePtr; @@ -174,7 +171,7 @@ public: ~ExecutableAllocator() { for (size_t i = 0; i < m_smallAllocationPools.length(); i++) - m_smallAllocationPools[i]->destroy(); + m_smallAllocationPools[i]->release(/* willDestroy = */true); } // alloc() returns a pointer to some memory, and also (by reference) a From 5d2018797b117f5aaeeb65c725372b90d9401e3c Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 9 Mar 2011 21:04:47 -0800 Subject: [PATCH 11/13] Bug 614608 - Rewrite String.prototype.split to be more ES5 compliant. r=jwalden --- js/src/jsregexp.h | 8 +- js/src/jsstr.cpp | 426 +++++++++++------- js/src/tests/ecma/String/15.5.4.8-2.js | 6 +- js/src/tests/ecma_5/String/jstests.list | 3 + js/src/tests/ecma_5/String/split-01.js | 47 ++ .../String/split-undefined-separator.js | 37 ++ js/src/tests/ecma_5/String/split-xregexp.js | 118 +++++ 7 files changed, 465 insertions(+), 180 deletions(-) create mode 100644 js/src/tests/ecma_5/String/split-01.js create mode 100644 js/src/tests/ecma_5/String/split-undefined-separator.js create mode 100644 js/src/tests/ecma_5/String/split-xregexp.js diff --git a/js/src/jsregexp.h b/js/src/jsregexp.h index ae72dc2eb64..7017a27e307 100644 --- a/js/src/jsregexp.h +++ b/js/src/jsregexp.h @@ -138,10 +138,6 @@ class RegExpStatics JS_ASSERT(pairNum < pairCount()); } - bool pairIsPresent(size_t pairNum) const { - return get(pairNum, 0) >= 0; - } - /* Precondition: paren is present. */ size_t getParenLength(size_t pairNum) const { checkParenNum(pairNum); @@ -273,6 +269,10 @@ class RegExpStatics JS_CALL_STRING_TRACER(trc, matchPairsInput, "res->matchPairsInput"); } + bool pairIsPresent(size_t pairNum) const { + return get(pairNum, 0) >= 0; + } + /* Value creators. */ bool createPendingInput(JSContext *cx, Value *out) const; diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index 6cf23b9bc13..efb13801aea 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -2537,116 +2537,263 @@ js::str_replace(JSContext *cx, uintN argc, Value *vp) return BuildFlatReplacement(cx, rdata.str, rdata.repstr, *fm, vp); } -/* - * Subroutine used by str_split to find the next split point in str, starting - * at offset *ip and looking either for the separator substring given by sep, or - * for the next re match. In the re case, return the matched separator in *sep, - * and the possibly updated offset in *ip. - * - * Return -2 on error, -1 on end of string, >= 0 for a valid index of the next - * separator occurrence if found, or str->length if no separator is found. - */ -static jsint -find_split(JSContext *cx, RegExpStatics *res, JSString *str, js::RegExp *re, jsint *ip, - JSSubString *sep) +class SplitMatchResult { + size_t endIndex_; + size_t length_; + + public: + void setFailure() { + JS_STATIC_ASSERT(SIZE_MAX > JSString::MAX_LENGTH); + endIndex_ = SIZE_MAX; + } + bool isFailure() const { + return (endIndex_ == SIZE_MAX); + } + size_t endIndex() const { + JS_ASSERT(!isFailure()); + return endIndex_; + } + size_t length() const { + JS_ASSERT(!isFailure()); + return length_; + } + void setResult(size_t length, size_t endIndex) { + length_ = length; + endIndex_ = endIndex; + } +}; + +template +static JSObject * +SplitHelper(JSContext *cx, JSLinearString *str, uint32 limit, Matcher splitMatch) { - /* - * Stop if past end of string. If at end of string, we will compare the - * null char stored there (by js_NewString*) to sep->chars[j] in the while - * loop at the end of this function, so that - * - * "ab,".split(',') => ["ab", ""] - * - * and the resulting array converts back to the string "ab," for symmetry. - * However, we ape Perl and do this only if there is a sufficiently large - * limit argument (see str_split). - */ - jsint i = *ip; - size_t length = str->length(); - if ((size_t)i > length) - return -1; + size_t strLength = str->length(); + SplitMatchResult result; - const jschar *chars = str->getChars(cx); - if (!chars) - return -2; + /* Step 11. */ + if (strLength == 0) { + if (!splitMatch(cx, str, 0, &result)) + return NULL; - /* - * Match a regular expression against the separator at or above index i. - * Call js_ExecuteRegExp with true for the test argument. On successful - * match, get the separator from cx->regExpStatics.lastMatch. - */ - if (re) { - size_t index; - Value rval; + /* + * NB: Unlike in the non-empty string case, it's perfectly fine + * (indeed the spec requires it) if we match at the end of the + * string. Thus these cases should hold: + * + * var a = "".split(""); + * assertEq(a.length, 0); + * var b = "".split(/.?/); + * assertEq(b.length, 0); + */ + if (!result.isFailure()) + return NewDenseEmptyArray(cx); - again: - /* JS1.2 deviated from Perl by never matching at end of string. */ - index = (size_t)i; - if (!re->execute(cx, res, str, &index, true, &rval)) - return -2; - if (!rval.isTrue()) { - /* Mismatch: ensure our caller advances i past end of string. */ - sep->length = 1; - return length; - } - i = (jsint)index; - JS_ASSERT(sep); - res->getLastMatch(sep); - if (sep->length == 0) { - /* - * Empty string match: never split on an empty match at the start - * of a find_split cycle. Same rule as for an empty global match - * in DoMatch. - */ - if (i == *ip) { - /* - * "Bump-along" to avoid sticking at an empty match, but don't - * bump past end of string -- our caller must do that by adding - * sep->length to our return value. - */ - if ((size_t)i == length) - return -1; - i++; - goto again; - } - if ((size_t)i == length) { - /* - * If there was a trivial zero-length match at the end of the - * split, then we shouldn't output the matched string at the end - * of the split array. See ECMA-262 Ed. 3, 15.5.4.14, Step 15. - */ - sep->chars = NULL; - } - } - JS_ASSERT((size_t)i >= sep->length); - return i - sep->length; + Value v = StringValue(str); + return NewDenseCopiedArray(cx, 1, &v); } - /* - * Special case: if sep is the empty string, split str into one character - * substrings. Let our caller worry about whether to split once at end of - * string into an empty substring. - */ - if (sep->length == 0) - return ((size_t)i == length) ? -1 : i + 1; + /* Step 12. */ + size_t lastEndIndex = 0; + size_t index = 0; - /* - * Now that we know sep is non-empty, search starting at i in str for an - * occurrence of all of sep's chars. If we find them, return the index of - * the first separator char. Otherwise, return length. - */ - jsint match = StringMatch(chars + i, length - i, sep->chars, sep->length); - return match == -1 ? length : match + i; + /* Step 13. */ + AutoValueVector splits(cx); + + while (index < strLength) { + /* Step 13(a). */ + if (!splitMatch(cx, str, index, &result)) + return NULL; + + /* + * Step 13(b). + * + * Our match algorithm differs from the spec in that it returns the + * next index at which a match happens. If no match happens we're + * done. + * + * But what if the match is at the end of the string (and the string is + * not empty)? Per 13(c)(ii) this shouldn't be a match, so we have to + * specially exclude it. Thus this case should hold: + * + * var a = "abc".split(/\b/); + * assertEq(a.length, 1); + * assertEq(a[0], "abc"); + */ + if (result.isFailure()) + break; + + /* Step 13(c)(i). */ + size_t sepLength = result.length(); + size_t endIndex = result.endIndex(); + if (sepLength == 0 && endIndex == strLength) + break; + + /* Step 13(c)(ii). */ + if (endIndex == lastEndIndex) { + index++; + continue; + } + + /* Step 13(c)(iii). */ + JS_ASSERT(lastEndIndex < endIndex); + JS_ASSERT(sepLength <= strLength); + JS_ASSERT(lastEndIndex + sepLength <= endIndex); + + /* Steps 13(c)(iii)(1-3). */ + size_t subLength = size_t(endIndex - sepLength - lastEndIndex); + JSString *sub = js_NewDependentString(cx, str, lastEndIndex, subLength); + if (!sub || !splits.append(StringValue(sub))) + return NULL; + + /* Step 13(c)(iii)(4). */ + if (splits.length() == limit) + return NewDenseCopiedArray(cx, splits.length(), splits.begin()); + + /* Step 13(c)(iii)(5). */ + lastEndIndex = endIndex; + + /* Step 13(c)(iii)(6-7). */ + if (Matcher::returnsCaptures) { + RegExpStatics *res = cx->regExpStatics(); + for (size_t i = 0; i < res->parenCount(); i++) { + /* Steps 13(c)(iii)(7)(a-c). */ + if (res->pairIsPresent(i + 1)) { + JSSubString parsub; + res->getParen(i + 1, &parsub); + sub = js_NewStringCopyN(cx, parsub.chars, parsub.length); + if (!sub || !splits.append(StringValue(sub))) + return NULL; + } else { + if (!splits.append(UndefinedValue())) + return NULL; + } + + /* Step 13(c)(iii)(7)(d). */ + if (splits.length() == limit) + return NewDenseCopiedArray(cx, splits.length(), splits.begin()); + } + } + + /* Step 13(c)(iii)(8). */ + index = lastEndIndex; + } + + /* Steps 14-15. */ + JSString *sub = js_NewDependentString(cx, str, lastEndIndex, strLength - lastEndIndex); + if (!sub || !splits.append(StringValue(sub))) + return NULL; + + /* Step 16. */ + return NewDenseCopiedArray(cx, splits.length(), splits.begin()); } +/* + * The SplitMatch operation from ES5 15.5.4.14 is implemented using different + * matchers for regular expression and string separators. + * + * The algorithm differs from the spec in that the matchers return the next + * index at which a match happens. + */ +class SplitRegExpMatcher { + RegExpStatics *res; + RegExp *re; + + public: + static const bool returnsCaptures = true; + SplitRegExpMatcher(RegExp *re, RegExpStatics *res) : res(res), re(re) { + } + + inline bool operator()(JSContext *cx, JSLinearString *str, size_t index, + SplitMatchResult *result) { + Value rval; + if (!re->execute(cx, res, str, &index, true, &rval)) + return false; + if (!rval.isTrue()) { + result->setFailure(); + return true; + } + JSSubString sep; + res->getLastMatch(&sep); + + result->setResult(sep.length, index); + return true; + } +}; + +class SplitStringMatcher { + const jschar *sepChars; + size_t sepLength; + + public: + static const bool returnsCaptures = false; + SplitStringMatcher(JSLinearString *sep) { + sepChars = sep->chars(); + sepLength = sep->length(); + } + + inline bool operator()(JSContext *cx, JSLinearString *str, size_t index, + SplitMatchResult *res) { + JS_ASSERT(index == 0 || index < str->length()); + const jschar *chars = str->chars(); + jsint match = StringMatch(chars + index, str->length() - index, sepChars, sepLength); + if (match == -1) + res->setFailure(); + else + res->setResult(sepLength, index + match + sepLength); + return true; + } +}; + +/* ES5 15.5.4.14 */ static JSBool str_split(JSContext *cx, uintN argc, Value *vp) { + /* Steps 1-2. */ JSString *str = ThisToStringForStringProto(cx, vp); if (!str) return false; - if (argc == 0) { + /* Step 5: Use the second argument as the split limit, if given. */ + uint32 limit; + if (argc > 1 && !vp[3].isUndefined()) { + jsdouble d; + if (!ValueToNumber(cx, vp[3], &d)) + return false; + limit = js_DoubleToECMAUint32(d); + } else { + limit = UINT32_MAX; + } + + /* Step 8. */ + RegExp *re = NULL; + JSLinearString *sepstr = NULL; + bool sepUndefined = (argc == 0 || vp[2].isUndefined()); + if (!sepUndefined) { + if (VALUE_IS_REGEXP(cx, vp[2])) { + re = static_cast(vp[2].toObject().getPrivate()); + } else { + JSString *sep = js_ValueToString(cx, vp[2]); + if (!sep) + return false; + vp[2].setString(sep); + + sepstr = sep->ensureLinear(cx); + if (!sepstr) + return false; + } + } + + /* Step 9. */ + if (limit == 0) { + JSObject *aobj = NewDenseEmptyArray(cx); + if (!aobj) + return false; + vp->setObject(*aobj); + return true; + } + + /* Step 10. */ + if (sepUndefined) { Value v = StringValue(str); JSObject *aobj = NewDenseCopiedArray(cx, 1, &v); if (!aobj) @@ -2654,89 +2801,22 @@ str_split(JSContext *cx, uintN argc, Value *vp) vp->setObject(*aobj); return true; } - - RegExp *re; - JSSubString *sep, tmp; - if (VALUE_IS_REGEXP(cx, vp[2])) { - re = static_cast(vp[2].toObject().getPrivate()); - sep = &tmp; - - /* Set a magic value so we can detect a successful re match. */ - sep->chars = NULL; - sep->length = 0; - } else { - JSString *sepstr = js_ValueToString(cx, vp[2]); - if (!sepstr) - return false; - vp[2].setString(sepstr); - - /* - * Point sep at a local copy of sepstr's header because find_split - * will modify sep->length. - */ - tmp.length = sepstr->length(); - tmp.chars = sepstr->getChars(cx); - if (!tmp.chars) - return false; - re = NULL; - sep = &tmp; - } - - /* Use the second argument as the split limit, if given. */ - uint32 limit = 0; /* Avoid warning. */ - bool limited = (argc > 1) && !vp[3].isUndefined(); - if (limited) { - jsdouble d; - if (!ValueToNumber(cx, vp[3], &d)) - return false; - - /* Clamp limit between 0 and 1 + string length. */ - limit = js_DoubleToECMAUint32(d); - if (limit > str->length()) - limit = 1 + str->length(); - } - - AutoValueVector splits(cx); - - RegExpStatics *res = cx->regExpStatics(); - jsint i, j; - uint32 len = i = 0; - while ((j = find_split(cx, res, str, re, &i, sep)) >= 0) { - if (limited && len >= limit) - break; - - JSString *sub = js_NewDependentString(cx, str, i, size_t(j - i)); - if (!sub || !splits.append(StringValue(sub))) - return false; - len++; - - /* - * Imitate perl's feature of including parenthesized substrings that - * matched part of the delimiter in the new array, after the split - * substring that was delimited. - */ - if (re && sep->chars) { - for (uintN num = 0; num < res->parenCount(); num++) { - if (limited && len >= limit) - break; - JSSubString parsub; - res->getParen(num + 1, &parsub); - sub = js_NewStringCopyN(cx, parsub.chars, parsub.length); - if (!sub || !splits.append(StringValue(sub))) - return false; - len++; - } - sep->chars = NULL; - } - i = j + sep->length; - } - - if (j == -2) + JSLinearString *strlin = str->ensureLinear(cx); + if (!strlin) return false; - JSObject *aobj = NewDenseCopiedArray(cx, splits.length(), splits.begin()); + /* Steps 11-15. */ + JSObject *aobj; + if (re) { + aobj = SplitHelper(cx, strlin, limit, SplitRegExpMatcher(re, cx->regExpStatics())); + } else { + // NB: sepstr is anchored through its storage in vp[2]. + aobj = SplitHelper(cx, strlin, limit, SplitStringMatcher(sepstr)); + } if (!aobj) return false; + + /* Step 16. */ vp->setObject(*aobj); return true; } diff --git a/js/src/tests/ecma/String/15.5.4.8-2.js b/js/src/tests/ecma/String/15.5.4.8-2.js index de4d1d7b9d7..3a6c4e3bbc8 100644 --- a/js/src/tests/ecma/String/15.5.4.8-2.js +++ b/js/src/tests/ecma/String/15.5.4.8-2.js @@ -118,11 +118,11 @@ for ( var i = 0; i < TEST_STRING.length; i++ ) { eval("var s = new String( TEST_STRING ); s.split('')["+i+"]") ); } -// case where the value of the separator is undefined. in this case. the value of the separator -// should be ToString( separator ), or "undefined". +// case where the value of the separator is undefined. in this case, +// the this value is returned. var TEST_STRING = "thisundefinedisundefinedaundefinedstringundefinedobject"; -var EXPECT_STRING = new Array( "this", "is", "a", "string", "object" ); +var EXPECT_STRING = new Array( TEST_STRING ); new TestCase( SECTION, "var s = new String( "+ TEST_STRING +" ); s.split(void 0).length", diff --git a/js/src/tests/ecma_5/String/jstests.list b/js/src/tests/ecma_5/String/jstests.list index a69c939fdf9..b9cdebc4b58 100644 --- a/js/src/tests/ecma_5/String/jstests.list +++ b/js/src/tests/ecma_5/String/jstests.list @@ -2,3 +2,6 @@ url-prefix ../../jsreftest.html?test=ecma_5/String/ script 15.5.4.2.js script 15.5.4.7.js script 15.5.4.11-01.js +script split-01.js +script split-undefined-separator.js +script split-xregexp.js diff --git a/js/src/tests/ecma_5/String/split-01.js b/js/src/tests/ecma_5/String/split-01.js new file mode 100644 index 00000000000..3eda3b14873 --- /dev/null +++ b/js/src/tests/ecma_5/String/split-01.js @@ -0,0 +1,47 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + */ + +var BUGNUMBER = 614608; +var summary = "String.prototype.split tests"; + +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +function assertEqArr(a1, a2) { + assertEq(a1.length, a2.length); + + for(var i=0; iboldandcoded".split(/<(\/)?([^<>]+)>/), + ["A", undefined, "B", "bold", "/", "B", "and", undefined, + "CODE", "coded", "/", "CODE", ""]); + +if (typeof reportCompare === "function") + reportCompare(true, true); + +print("All tests passed!"); diff --git a/js/src/tests/ecma_5/String/split-undefined-separator.js b/js/src/tests/ecma_5/String/split-undefined-separator.js new file mode 100644 index 00000000000..46aa6c16f90 --- /dev/null +++ b/js/src/tests/ecma_5/String/split-undefined-separator.js @@ -0,0 +1,37 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + */ + +var BUGNUMBER = 614608; +var summary = "String.prototype.split with undefined separator"; + +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +function assertEqArr(a1, a2) { + assertEq(a1.length, a2.length); + + for(var i=0; i + * + * Distributed under the terms of the MIT license. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +var BUGNUMBER = 614608; +var summary = "String.prototype.split with regexp separator"; + +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +var ecmaSampleRe = /<(\/)?([^<>]+)>/; + +var testCode = [ + ["''.split()", [""]], + ["''.split(/./)", [""]], + ["''.split(/.?/)", []], + ["''.split(/.??/)", []], + ["'ab'.split(/a*/)", ["", "b"]], + ["'ab'.split(/a*?/)", ["a", "b"]], + ["'ab'.split(/(?:ab)/)", ["", ""]], + ["'ab'.split(/(?:ab)*/)", ["", ""]], + ["'ab'.split(/(?:ab)*?/)", ["a", "b"]], + ["'test'.split('')", ["t", "e", "s", "t"]], + ["'test'.split()", ["test"]], + ["'111'.split(1)", ["", "", "", ""]], + ["'test'.split(/(?:)/, 2)", ["t", "e"]], + ["'test'.split(/(?:)/, -1)", ["t", "e", "s", "t"]], + ["'test'.split(/(?:)/, undefined)", ["t", "e", "s", "t"]], + ["'test'.split(/(?:)/, null)", []], + ["'test'.split(/(?:)/, NaN)", []], + ["'test'.split(/(?:)/, true)", ["t"]], + ["'test'.split(/(?:)/, '2')", ["t", "e"]], + ["'test'.split(/(?:)/, 'two')", []], + ["'a'.split(/-/)", ["a"]], + ["'a'.split(/-?/)", ["a"]], + ["'a'.split(/-??/)", ["a"]], + ["'a'.split(/a/)", ["", ""]], + ["'a'.split(/a?/)", ["", ""]], + ["'a'.split(/a??/)", ["a"]], + ["'ab'.split(/-/)", ["ab"]], + ["'ab'.split(/-?/)", ["a", "b"]], + ["'ab'.split(/-??/)", ["a", "b"]], + ["'a-b'.split(/-/)", ["a", "b"]], + ["'a-b'.split(/-?/)", ["a", "b"]], + ["'a-b'.split(/-??/)", ["a", "-", "b"]], + ["'a--b'.split(/-/)", ["a", "", "b"]], + ["'a--b'.split(/-?/)", ["a", "", "b"]], + ["'a--b'.split(/-??/)", ["a", "-", "-", "b"]], + ["''.split(/()()/)", []], + ["'.'.split(/()()/)", ["."]], + ["'.'.split(/(.?)(.?)/)", ["", ".", "", ""]], + ["'.'.split(/(.??)(.??)/)", ["."]], + ["'.'.split(/(.)?(.)?/)", ["", ".", undefined, ""]], + ["'Aboldandcoded'.split(ecmaSampleRe)", + ["A", undefined, "B", "bold", "/", "B", + "and", undefined, "CODE", "coded", "/", + "CODE", ""]], + ["'tesst'.split(/(s)*/)", ["t", undefined, "e", "s", "t"]], + ["'tesst'.split(/(s)*?/)", ["t", undefined, "e", undefined, "s", + undefined, "s", undefined, "t"]], + ["'tesst'.split(/(s*)/)", ["t", "", "e", "ss", "t"]], + ["'tesst'.split(/(s*?)/)", ["t", "", "e", "", "s", "", "s", "", "t"]], + ["'tesst'.split(/(?:s)*/)", ["t", "e", "t"]], + ["'tesst'.split(/(?=s+)/)", ["te", "s", "st"]], + ["'test'.split('t')", ["", "es", ""]], + ["'test'.split('es')", ["t", "t"]], + ["'test'.split(/t/)", ["", "es", ""]], + ["'test'.split(/es/)", ["t", "t"]], + ["'test'.split(/(t)/)", ["", "t", "es", "t", ""]], + ["'test'.split(/(es)/)", ["t", "es", "t"]], + ["'test'.split(/(t)(e)(s)(t)/)", ["", "t", "e", "s", "t", ""]], + ["'.'.split(/(((.((.??)))))/)", ["", ".", ".", ".", "", "", ""]], + ["'.'.split(/(((((.??)))))/)", ["."]] +]; + +function testSplit() { + for (var i = 0; i < testCode.length; i++) { + var actual = eval(testCode[i][0]); + var expected = testCode[i][1]; + + assertEq(actual.length, expected.length); + + for(var j=0; j Date: Mon, 19 Apr 2010 17:40:13 -0700 Subject: [PATCH 12/13] Bug 558541 - Add a test. r=jorendorff --- js/src/tests/js1_8_5/extensions/jstests.list | 1 + js/src/tests/js1_8_5/extensions/regress-558541.js | 12 ++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 js/src/tests/js1_8_5/extensions/regress-558541.js diff --git a/js/src/tests/js1_8_5/extensions/jstests.list b/js/src/tests/js1_8_5/extensions/jstests.list index ef334872268..9868008eac5 100644 --- a/js/src/tests/js1_8_5/extensions/jstests.list +++ b/js/src/tests/js1_8_5/extensions/jstests.list @@ -8,6 +8,7 @@ skip-if(!xulRuntime.shell) script worker-init.js skip-if(!xulRuntime.shell) script worker-simple.js skip-if(!xulRuntime.shell) script worker-terminate.js skip-if(!xulRuntime.shell) script worker-timeout.js +script regress-558541.js script scripted-proxies.js script watch-undefined-setter.js script array-length-protochange.js diff --git a/js/src/tests/js1_8_5/extensions/regress-558541.js b/js/src/tests/js1_8_5/extensions/regress-558541.js new file mode 100644 index 00000000000..67eae896b66 --- /dev/null +++ b/js/src/tests/js1_8_5/extensions/regress-558541.js @@ -0,0 +1,12 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + * Contributor: Jeff Walden + */ + +options("strict"); +for (var i = 0; i < 5; i++) + Boolean.prototype = 42; + +reportCompare(true, true); From 183c92c8170201c941cd589de469cbc5a4ed3a6a Mon Sep 17 00:00:00 2001 From: Jacek Caban Date: Thu, 10 Mar 2011 11:32:23 +0000 Subject: [PATCH 13/13] Bug 633924 - MethodGIT broken on mingw-w64 win64 build r=dvander --- js/src/Makefile.in | 5 + js/src/methodjit/MachineRegs.h | 6 +- js/src/methodjit/MethodJIT.cpp | 18 +-- js/src/methodjit/MethodJIT.h | 2 +- js/src/methodjit/TrampolineMingwX64.s | 190 ++++++++++++++++++++++++++ 5 files changed, 205 insertions(+), 16 deletions(-) create mode 100644 js/src/methodjit/TrampolineMingwX64.s diff --git a/js/src/Makefile.in b/js/src/Makefile.in index cacd29e458e..15f621c453f 100644 --- a/js/src/Makefile.in +++ b/js/src/Makefile.in @@ -336,6 +336,11 @@ ifeq (x86_64, $(TARGET_CPU)) ifdef _MSC_VER ASFILES += TrampolineMasmX64.asm endif +ifeq ($(OS_ARCH),WINNT) +ifdef GNU_CC +ASFILES += TrampolineMingwX64.s +endif +endif ifdef SOLARIS_SUNPRO_CXX ASFILES += TrampolineSUNWX64.s endif diff --git a/js/src/methodjit/MachineRegs.h b/js/src/methodjit/MachineRegs.h index 149f1883ed8..cc5064d0926 100644 --- a/js/src/methodjit/MachineRegs.h +++ b/js/src/methodjit/MachineRegs.h @@ -71,7 +71,7 @@ struct Registers { #if defined(JS_CPU_X86) || defined(JS_CPU_X64) static const RegisterID ReturnReg = JSC::X86Registers::eax; -# if defined(JS_CPU_X86) || defined(_MSC_VER) +# if defined(JS_CPU_X86) || defined(_WIN64) static const RegisterID ArgReg0 = JSC::X86Registers::ecx; static const RegisterID ArgReg1 = JSC::X86Registers::edx; # if defined(JS_CPU_X64) @@ -111,7 +111,7 @@ struct Registers { # if defined(JS_CPU_X64) | (1 << JSC::X86Registers::r8) | (1 << JSC::X86Registers::r9) -# if !defined(_MSC_VER) +# if !defined(_WIN64) | (1 << JSC::X86Registers::esi) | (1 << JSC::X86Registers::edi) # endif @@ -125,7 +125,7 @@ struct Registers { // r13 is TypeMaskReg. // r14 is PayloadMaskReg. | (1 << JSC::X86Registers::r15) -# if defined(_MSC_VER) +# if defined(_WIN64) | (1 << JSC::X86Registers::esi) | (1 << JSC::X86Registers::edi) # endif diff --git a/js/src/methodjit/MethodJIT.cpp b/js/src/methodjit/MethodJIT.cpp index 71e71881daf..56c5b7b0b68 100644 --- a/js/src/methodjit/MethodJIT.cpp +++ b/js/src/methodjit/MethodJIT.cpp @@ -167,7 +167,7 @@ JS_STATIC_ASSERT(offsetof(JSFrameRegs, sp) == 0); # define HIDE_SYMBOL(name) #endif -#if defined(__GNUC__) +#if defined(__GNUC__) && !defined(_WIN64) /* If this assert fails, you need to realign VMFrame to 16 bytes. */ #ifdef JS_CPU_ARM @@ -558,9 +558,7 @@ SYMBOL_STRING(JaegerStubVeneer) ":" "\n" # else # error "Unsupported CPU!" # endif -#elif defined(_MSC_VER) - -#if defined(JS_CPU_X86) +#elif defined(_MSC_VER) && defined(JS_CPU_X86) /* * *** DANGER *** @@ -666,7 +664,9 @@ extern "C" { } } -#elif defined(JS_CPU_X64) +// Windows x64 uses assembler version since compiler doesn't support +// inline assembler +#elif defined(_WIN64) /* * *** DANGER *** @@ -678,13 +678,7 @@ JS_STATIC_ASSERT(offsetof(VMFrame, regs.fp) == 0x38); JS_STATIC_ASSERT(JSVAL_TAG_MASK == 0xFFFF800000000000LL); JS_STATIC_ASSERT(JSVAL_PAYLOAD_MASK == 0x00007FFFFFFFFFFFLL); -// Windows x64 uses assembler version since compiler doesn't support -// inline assembler -#else -# error "Unsupported CPU!" -#endif - -#endif /* _MSC_VER */ +#endif /* _WIN64 */ bool JaegerCompartment::Initialize() diff --git a/js/src/methodjit/MethodJIT.h b/js/src/methodjit/MethodJIT.h index 4a220f0466d..90d34415055 100644 --- a/js/src/methodjit/MethodJIT.h +++ b/js/src/methodjit/MethodJIT.h @@ -485,7 +485,7 @@ JSScript::nativeCodeForPC(bool constructing, jsbytecode *pc) return native; } -#ifdef _MSC_VER +#if defined(_MSC_VER) || defined(_WIN64) extern "C" void *JaegerThrowpoline(js::VMFrame *vmFrame); #else extern "C" void JaegerThrowpoline(); diff --git a/js/src/methodjit/TrampolineMingwX64.s b/js/src/methodjit/TrampolineMingwX64.s new file mode 100644 index 00000000000..af2315ea7b1 --- /dev/null +++ b/js/src/methodjit/TrampolineMingwX64.s @@ -0,0 +1,190 @@ +# -*- Mode: C++# tab-width: 4# indent-tabs-mode: nil# c-basic-offset: 4 -*- +# ***** BEGIN LICENSE BLOCK ***** +# Version: MPL 1.1/GPL 2.0/LGPL 2.1 +# +# The contents of this file are subject to the Mozilla Public License Version +# 1.1 (the "License")# you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS IS" basis, +# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License +# for the specific language governing rights and limitations under the +# License. +# +# The Original Code is mozilla.org code. +# +# The Initial Developer of the Original Code is Mozilla Japan. +# Portions created by the Initial Developer are Copyright (C) 2010 +# the Initial Developer. All Rights Reserved. +# +# Contributor(s): +# Makoto Kato +# +# Alternatively, the contents of this file may be used under the terms of +# either the GNU General Public License Version 2 or later (the "GPL"), or +# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), +# in which case the provisions of the GPL or the LGPL are applicable instead +# of those above. If you wish to allow use of your version of this file only +# under the terms of either the GPL or the LGPL, and not to allow others to +# use your version of this file under the terms of the MPL, indicate your +# decision by deleting the provisions above and replace them with the notice +# and other provisions required by the GPL or the LGPL. If you do not delete +# the provisions above, a recipient may use your version of this file under +# the terms of any one of the MPL, the GPL or the LGPL. +# +# ***** END LICENSE BLOCK ***** + + +.extern js_InternalThrow +.extern SetVMFrameRegs +.extern PushActiveVMFrame +.extern PopActiveVMFrame + +.text +.intel_syntax noprefix + +# JSBool JaegerTrampoline(JSContext *cx, JSStackFrame *fp, void *code, +# Value *stackLimit, void *safePoint)# +.globl JaegerTrampoline +.def JaegerTrampoline + .scl 3 + .type 46 +.endef +JaegerTrampoline: + push rbp + # .PUSHREG rbp + mov rbp, rsp + # .SETFRAME rbp, 0 + push r12 + # .PUSHREG r12 + push r13 + # .PUSHREG r13 + push r14 + # .PUSHREG r14 + push r15 + # .PUSHREG r15 + push rdi + # .PUSHREG rdi + push rsi + # .PUSHREG rsi + push rbx + # .PUSHREG rbx + # .ENDPROLOG + + # Load mask registers + mov r13, 0xffff800000000000 + mov r14, 0x7fffffffffff + + # Build the JIT frame. + # rcx = cx + # rdx = fp + # r9 = inlineCallCount + # fp must go into rbx + push rdx # entryFp + push r9 # inlineCallCount + push rcx # cx + push rdx # fp + mov rbx, rdx + + # Space for the rest of the VMFrame. + sub rsp, 0x28 + + # This is actually part of the VMFrame. + mov r10, [rbp+8*5+8] + push r10 + + # Set cx->regs and set the active frame. Save r8 and align frame in one + push r8 + mov rcx, rsp + sub rsp, 0x20 + call SetVMFrameRegs + lea rcx, [rsp+0x20] + call PushActiveVMFrame + add rsp, 0x20 + + # Jump into the JIT code. + jmp qword ptr [rsp] + +# void JaegerTrampolineReturn()# +.globl JaegerTrampolineReturn +.def JaegerTrampolineReturn + .scl 3 + .type 46 +.endef +JaegerTrampolineReturn: + # .ENDPROLOG + or rcx, rdx + mov qword ptr [rbx + 0x30], rcx + sub rsp, 0x20 + lea rcx, [rsp+0x20] + call PopActiveVMFrame + + add rsp, 0x58+0x20 + pop rbx + pop rsi + pop rdi + pop r15 + pop r14 + pop r13 + pop r12 + pop rbp + mov rax, 1 + ret + + +# void JaegerThrowpoline() +.globl JaegerThrowpoline +.def JaegerTrampoline + .scl 3 + .type 46 +.endef +JaegerThrowpoline: + # .ENDPROLOG + # For Windows x64 stub calls, we pad the stack by 32 before + # calling, so we must account for that here. See doStubCall. + lea rcx, [rsp+0x20] + call js_InternalThrow + test rax, rax + je throwpoline_exit + add rsp, 0x20 + jmp rax + +throwpoline_exit: + lea rcx, [rsp+0x20] + call PopActiveVMFrame + add rsp, 0x58+0x20 + pop rbx + pop rsi + pop rdi + pop r15 + pop r14 + pop r13 + pop r12 + pop rbp + xor rax, rax + ret + + + +# void InjectJaegerReturn()# +.globl InjectJaegerReturn +.def InjectJaegerReturn + .scl 3 + .type 46 +.endef +InjectJaegerReturn: + # .ENDPROLOG + mov rcx, qword ptr [rbx+0x30] # load fp->rval_ into typeReg + mov rax, qword ptr [rbx+0x28] # fp->ncode_ + + # Reimplementation of PunboxAssembler::loadValueAsComponents() + mov rdx, r14 + and rdx, rcx + xor rcx, rdx + + # For Windows x64 stub calls, we pad the stack by 32 before + # calling, so we must account for that here. See doStubCall. + mov rbx, qword ptr [rsp+0x38+0x20] # f.fp + add rsp, 0x20 + jmp rax # return