From 2182ce61bef940343a05763754ff32f31c89d97d Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Wed, 4 Jun 2008 17:00:46 -0700 Subject: [PATCH 1/3] Fix 260106, r=shaver. --- js/src/jsemit.cpp | 64 ++++++++++++++++++++++++++++++++--------- js/src/jsinterp.cpp | 46 ++++++++++++++++++++++++++---- js/src/jsopcode.cpp | 69 +++++++++++++++++++++++++++++++++++++++++++-- js/src/jsopcode.tbl | 10 ++++++- js/src/jsxdrapi.h | 2 +- 5 files changed, 168 insertions(+), 23 deletions(-) diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index de4ae569342e..bf85f2c595c1 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -175,6 +175,9 @@ UpdateDepth(JSContext *cx, JSCodeGenerator *cg, ptrdiff_t target) case JSOP_EVAL: nuses = 2 + GET_ARGC(pc); /* stack: fun, this, [argc arguments] */ break; + case JSOP_NEWARRAY: + nuses = GET_UINT24(pc); + break; default: JS_ASSERT(0); } @@ -253,7 +256,13 @@ js_EmitN(JSContext *cx, JSCodeGenerator *cg, JSOp op, size_t extra) *next = (jsbytecode)op; memset(next + 1, 0, BYTECODE_SIZE(extra)); CG_NEXT(cg) = next + length; - UpdateDepth(cx, cg, offset); + + /* + * Don't UpdateDepth if op's use-count comes from the immediate + * operand yet to be stored in the extra bytes after op. + */ + if (js_CodeSpec[op].nuses >= 0) + UpdateDepth(cx, cg, offset); } return offset; } @@ -6049,14 +6058,32 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) #endif /* * Emit code for [a, b, c] of the form: + * * t = new Array; t[0] = a; t[1] = b; t[2] = c; t; - * but use a stack slot for t and avoid dup'ing and popping it via + * + * but use a stack slot for t and avoid dup'ing and popping it using * the JSOP_NEWINIT and JSOP_INITELEM bytecodes. + * + * If no sharp variable is defined and the initialiser is not for an + * array comprehension, use JSOP_NEWARRAY. */ - if (js_Emit2(cx, cg, JSOP_NEWINIT, (jsbytecode) JSProto_Array) < 0) - return JS_FALSE; - pn2 = pn->pn_head; + op = JSOP_NEWARRAY; + +#if JS_HAS_SHARP_VARS + if (pn2 && pn2->pn_type == TOK_DEFSHARP) + op = JSOP_NEWINIT; +#endif +#if JS_HAS_GENERATORS + if (pn->pn_type == TOK_ARRAYCOMP) + op = JSOP_NEWINIT; +#endif + + if (op == JSOP_NEWINIT && + js_Emit2(cx, cg, op, (jsbytecode) JSProto_Array) < 0) { + return JS_FALSE; + } + #if JS_HAS_SHARP_VARS if (pn2 && pn2->pn_type == TOK_DEFSHARP) { EMIT_UINT16_IMM_OP(JSOP_DEFSHARP, (jsatomid)pn2->pn_num); @@ -6088,19 +6115,18 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) #endif /* JS_HAS_GENERATORS */ for (atomIndex = 0; pn2; atomIndex++, pn2 = pn2->pn_next) { - if (!EmitNumberOp(cx, atomIndex, cg)) + if (op == JSOP_NEWINIT && !EmitNumberOp(cx, atomIndex, cg)) return JS_FALSE; - /* FIXME 260106: holes in a sparse initializer are void-filled. */ if (pn2->pn_type == TOK_COMMA) { - if (js_Emit1(cx, cg, JSOP_PUSH) < 0) + if (js_Emit1(cx, cg, JSOP_HOLE) < 0) return JS_FALSE; } else { if (!js_EmitTree(cx, cg, pn2)) return JS_FALSE; } - if (js_Emit1(cx, cg, JSOP_INITELEM) < 0) + if (op == JSOP_NEWINIT && js_Emit1(cx, cg, JSOP_INITELEM) < 0) return JS_FALSE; } @@ -6110,9 +6136,19 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) return JS_FALSE; } - /* Emit an op for sharp array cleanup and decompilation. */ - if (js_Emit1(cx, cg, JSOP_ENDINIT) < 0) - return JS_FALSE; + if (op == JSOP_NEWARRAY) { + JS_ASSERT(atomIndex == pn->pn_count); + off = js_EmitN(cx, cg, op, 3); + if (off < 0) + return JS_FALSE; + pc = CG_CODE(cg, off); + SET_UINT24(pc, atomIndex); + UpdateDepth(cx, cg, off); + } else { + /* Emit an op for sharp array cleanup and decompilation. */ + if (js_Emit1(cx, cg, JSOP_ENDINIT) < 0) + return JS_FALSE; + } break; case TOK_RC: @@ -6125,9 +6161,11 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) #endif /* * Emit code for {p:a, '%q':b, 2:c} of the form: + * * t = new Object; t.p = a; t['%q'] = b; t[2] = c; t; + * * but use a stack slot for t and avoid dup'ing and popping it via - * the JSOP_NEWINIT and JSOP_INITELEM bytecodes. + * the JSOP_NEWINIT and JSOP_INITELEM/JSOP_INITPROP bytecodes. */ if (js_Emit2(cx, cg, JSOP_NEWINIT, (jsbytecode) JSProto_Object) < 0) return JS_FALSE; diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 1eb93c5f7e22..a0ebc9359834 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -5936,6 +5936,20 @@ interrupt: DO_NEXT_OP(len); #endif /* JS_HAS_GETTER_SETTER */ + BEGIN_CASE(JSOP_HOLE) + PUSH_OPND(JSVAL_HOLE); + END_CASE(JSOP_HOLE) + + BEGIN_CASE(JSOP_NEWARRAY) + len = GET_UINT24(regs.pc); + JS_ASSERT(len <= regs.sp - fp->spbase); + obj = js_NewArrayObject(cx, len, regs.sp - len); + if (!obj) + goto error; + regs.sp -= len - 1; + STORE_OPND(-1, OBJECT_TO_JSVAL(obj)); + END_CASE(JSOP_NEWARRAY) + BEGIN_CASE(JSOP_NEWINIT) i = GET_INT8(regs.pc); JS_ASSERT(i == JSProto_Array || i == JSProto_Object); @@ -6096,20 +6110,42 @@ interrupt: /* Pop the element's value into rval. */ JS_ASSERT(regs.sp - fp->spbase >= 3); rval = FETCH_OPND(-1); - FETCH_ELEMENT_ID(obj, -2, id); /* Find the object being initialized at top of stack. */ lval = FETCH_OPND(-3); JS_ASSERT(!JSVAL_IS_PRIMITIVE(lval)); obj = JSVAL_TO_OBJECT(lval); - /* Set the property named by obj[id] to rval. */ + /* Fetch id now that we have obj. */ + FETCH_ELEMENT_ID(obj, -2, id); + + /* + * Check for property redeclaration strict warning (we may be in + * an object initialiser, not an array initialiser). + */ if (!js_CheckRedeclaration(cx, obj, id, JSPROP_INITIALIZER, NULL, NULL)) { goto error; } - if (!OBJ_SET_PROPERTY(cx, obj, id, &rval)) - goto error; + + /* + * If rval is a hole, do not call OBJ_SET_PROPERTY. In this case, + * obj must be an array, so if the current op is the last element + * initialiser, set the array length to one greater than id. + */ + if (rval == JSVAL_HOLE) { + JS_ASSERT(OBJ_IS_ARRAY(cx, obj)); + JS_ASSERT(JSID_IS_INT(id)); + JS_ASSERT((jsuint) JSID_TO_INT(id) < ARRAY_INIT_LIMIT); + if ((JSOp) regs.pc[JSOP_INITELEM_LENGTH] == JSOP_ENDINIT && + !js_SetLengthProperty(cx, obj, + (jsuint) (JSID_TO_INT(id) + 1))) { + goto error; + } + } else { + if (!OBJ_SET_PROPERTY(cx, obj, id, &rval)) + goto error; + } regs.sp -= 2; END_CASE(JSOP_INITELEM); @@ -6708,8 +6744,6 @@ interrupt: L_JSOP_DEFXMLNS: # endif - L_JSOP_UNUSED117: - #else /* !JS_THREADED_INTERP */ default: #endif diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index b2e03dd3404f..cf0accc147d5 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -364,7 +364,7 @@ js_Disassemble1(JSContext *cx, JSScript *script, jsbytecode *pc, break; case JOF_UINT24: - JS_ASSERT(op == JSOP_UINT24); + JS_ASSERT(op == JSOP_UINT24 || op == JSOP_NEWARRAY); i = (jsint)GET_UINT24(pc); goto print_int; @@ -851,7 +851,11 @@ GetStr(SprintStack *ss, uintN i) return OFF2STR(&ss->sprinter, off); } -/* Gap between stacked strings to allow for insertion of parens and commas. */ +/* + * Gap between stacked strings to allow for insertion of parens and commas + * when auto-parenthesizing expressions and decompiling array initialisers + * (see the JSOP_NEWARRAY case in Decompile). + */ #define PAREN_SLOP (2 + 1) /* @@ -4188,6 +4192,59 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop) todo = -2; break; + case JSOP_HOLE: + todo = SprintPut(&ss->sprinter, "", 0); + break; + + case JSOP_NEWARRAY: + { + ptrdiff_t off; + char *base, *from, *to; + + /* + * All operands are stacked and ready for in-place formatting. + * We know that PAREN_SLOP is 3 here, and take advantage of it + * to avoid strdup'ing. + */ + argc = GET_UINT24(pc); + LOCAL_ASSERT(ss->top >= (uintN) argc); + sn = js_GetSrcNote(jp->script, pc); + if (argc == 0) { + todo = Sprint(&ss->sprinter, "[%s]", + (sn && SN_TYPE(sn) == SRC_CONTINUE) + ? ", " + : ""); + } else { + ss->top -= argc; + off = GetOff(ss, ss->top); + LOCAL_ASSERT(off >= PAREN_SLOP); + base = OFF2STR(&ss->sprinter, off); + to = base + 1; + i = 0; + for (;;) { + /* Move to the next string that had been stacked. */ + from = OFF2STR(&ss->sprinter, off); + todo = strlen(from); + memmove(to, from, todo); + to += todo; + if (++i == argc && + !(sn && SN_TYPE(sn) == SRC_CONTINUE)) { + break; + } + *to++ = ','; + *to++ = ' '; + off = GetOff(ss, ss->top + i); + } + LOCAL_ASSERT(to - base < ss->sprinter.offset - PAREN_SLOP); + *base = '['; + *to++ = ']'; + *to = '\0'; + ss->sprinter.offset = STR2OFF(&ss->sprinter, to); + todo = STR2OFF(&ss->sprinter, base); + } + break; + } + case JSOP_NEWINIT: { i = GET_INT8(pc); @@ -5069,6 +5126,14 @@ ReconstructPCStack(JSContext *cx, JSScript *script, jsbytecode *pc, continue; } + if (op == JSOP_NEWARRAY) { + pcdepth -= GET_UINT24(pc) - 1; + LOCAL_ASSERT(pcdepth > 0); + if (pcstack) + pcstack[pcdepth - 1] = pc; + continue; + } + /* * A (C ? T : E) expression requires skipping either T (if begin is in * E) or both T and E (if begin is after the whole expression) before diff --git a/js/src/jsopcode.tbl b/js/src/jsopcode.tbl index fc7bcf5187ac..8aa08fe0269f 100644 --- a/js/src/jsopcode.tbl +++ b/js/src/jsopcode.tbl @@ -266,7 +266,7 @@ OPDEF(JSOP_RETSUB, 115,"retsub", NULL, 1, 0, 0, 0, JOF_BYTE) OPDEF(JSOP_EXCEPTION, 116,"exception", NULL, 1, 0, 1, 0, JOF_BYTE) /* Embedded lineno to speedup pc->line mapping. */ -OPDEF(JSOP_LINENO, 117,"lineno", NULL, 3, 0, 0, 0, JOF_UINT16) +OPDEF(JSOP_LINENO, 117,"lineno", NULL, 3, 0, 0, 0, JOF_UINT16) /* * ECMA-compliant switch statement ops. @@ -521,3 +521,11 @@ OPDEF(JSOP_INT32, 228, "int32", NULL, 5, 0, 1, 16, JOF_INT32) * Get the value of the 'length' property from a stacked object. */ OPDEF(JSOP_LENGTH, 229, "length", NULL, 1, 1, 1, 18, JOF_BYTE|JOF_PROP) + +/* + * JSOP_NEWARRAY optimizes array literal evaluation using the interpreter stack. + * JSOP_HOLE pushes a JSVAL_HOLE (used with JSOP_NEWINIT and JSOP_NEWARRAY). + */ +OPDEF(JSOP_NEWARRAY, 230,"newarray", NULL, 4, -1, 1, 19, JOF_UINT24) +OPDEF(JSOP_HOLE, 231,"hole", NULL, 1, 0, 1, 0, JOF_BYTE) + diff --git a/js/src/jsxdrapi.h b/js/src/jsxdrapi.h index e4589231d87f..3026b49ee215 100644 --- a/js/src/jsxdrapi.h +++ b/js/src/jsxdrapi.h @@ -202,7 +202,7 @@ JS_XDRFindClassById(JSXDRState *xdr, uint32 id); * before deserialization of bytecode. If the saved version does not match * the current version, abort deserialization and invalidate the file. */ -#define JSXDR_BYTECODE_VERSION (0xb973c0de - 24) +#define JSXDR_BYTECODE_VERSION (0xb973c0de - 25) /* * Library-private functions. From 9d417c25f92f99ef681b6fb8d5ce1e21629e896c Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Thu, 5 Jun 2008 16:00:25 -0700 Subject: [PATCH 2/3] Followup patch for bug 260106. --- js/src/jsarray.cpp | 27 ++++++++++++++++++++------- js/src/jsarray.h | 3 ++- js/src/jsinterp.cpp | 2 +- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 1f61825a802e..dfdaaa9f2827 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -1461,7 +1461,8 @@ InitArrayElements(JSContext *cx, JSObject *obj, jsuint start, jsuint end, } static JSBool -InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, jsval *vector) +InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, jsval *vector, + JSBool holey = JS_FALSE) { JS_ASSERT(OBJ_IS_ARRAY(cx, obj)); @@ -1470,8 +1471,18 @@ InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, jsval *vector) if (vector) { if (!EnsureLength(cx, obj, length)) return JS_FALSE; - memcpy(obj->dslots, vector, length * sizeof (jsval)); - obj->fslots[JSSLOT_ARRAY_COUNT] = length; + + jsuint count = length; + if (!holey) { + memcpy(obj->dslots, vector, length * sizeof (jsval)); + } else { + for (jsuint i = 0; i < length; i++) { + if (vector[i] == JSVAL_HOLE) + --count; + obj->dslots[i] = vector[i]; + } + } + obj->fslots[JSSLOT_ARRAY_COUNT] = count; } else { obj->fslots[JSSLOT_ARRAY_COUNT] = 0; } @@ -2348,7 +2359,8 @@ array_concat(JSContext *cx, uintN argc, jsval *vp) /* Create a new Array object and root it using *vp. */ aobj = JS_THIS_OBJECT(cx, vp); if (OBJ_IS_DENSE_ARRAY(cx, aobj)) { - nobj = js_NewArrayObject(cx, ARRAY_DENSE_LENGTH(aobj), aobj->dslots); + nobj = js_NewArrayObject(cx, ARRAY_DENSE_LENGTH(aobj), aobj->dslots, + JS_TRUE); if (!nobj) return JS_FALSE; length = aobj->fslots[JSSLOT_ARRAY_LENGTH]; @@ -2479,7 +2491,8 @@ array_slice(JSContext *cx, uintN argc, jsval *vp) begin = end; if (OBJ_IS_DENSE_ARRAY(cx, obj) && end <= ARRAY_DENSE_LENGTH(obj)) { - nobj = js_NewArrayObject(cx, end - begin, obj->dslots + begin); + nobj = js_NewArrayObject(cx, end - begin, obj->dslots + begin, + JS_TRUE); if (!nobj) return JS_FALSE; *vp = OBJECT_TO_JSVAL(nobj); @@ -2924,7 +2937,7 @@ js_InitArrayClass(JSContext *cx, JSObject *obj) } JSObject * -js_NewArrayObject(JSContext *cx, jsuint length, jsval *vector) +js_NewArrayObject(JSContext *cx, jsuint length, jsval *vector, JSBool holey) { JSTempValueRooter tvr; JSObject *obj; @@ -2934,7 +2947,7 @@ js_NewArrayObject(JSContext *cx, jsuint length, jsval *vector) return NULL; JS_PUSH_TEMP_ROOT_OBJECT(cx, obj, &tvr); - if (!InitArrayObject(cx, obj, length, vector)) + if (!InitArrayObject(cx, obj, length, vector, holey)) obj = NULL; JS_POP_TEMP_ROOT(cx, &tvr); diff --git a/js/src/jsarray.h b/js/src/jsarray.h index bc7c6bbdc9e3..faa307f1e964 100644 --- a/js/src/jsarray.h +++ b/js/src/jsarray.h @@ -64,7 +64,8 @@ extern JSObject * js_InitArrayClass(JSContext *cx, JSObject *obj); extern JSObject * -js_NewArrayObject(JSContext *cx, jsuint length, jsval *vector); +js_NewArrayObject(JSContext *cx, jsuint length, jsval *vector, + JSBool holey = JS_FALSE); /* Create an array object that starts out already made slow/sparse. */ extern JSObject * diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index a0ebc9359834..310937d89d7a 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -5943,7 +5943,7 @@ interrupt: BEGIN_CASE(JSOP_NEWARRAY) len = GET_UINT24(regs.pc); JS_ASSERT(len <= regs.sp - fp->spbase); - obj = js_NewArrayObject(cx, len, regs.sp - len); + obj = js_NewArrayObject(cx, len, regs.sp - len, JS_TRUE); if (!obj) goto error; regs.sp -= len - 1; From 7d3ff26c38cdb83e062cecdb38e80dfabf284d0d Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Fri, 6 Jun 2008 13:23:19 -0700 Subject: [PATCH 3/3] Third and final followup patch for bug 260106. --- js/src/jsarray.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index dfdaaa9f2827..5c420c5d17a7 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -2346,7 +2346,7 @@ static JSBool array_concat(JSContext *cx, uintN argc, jsval *vp) { jsval *argv, v; - JSObject *nobj, *aobj; + JSObject *aobj, *nobj; jsuint length, alength, slot; uintN i; JSBool hole, ok; @@ -2359,11 +2359,21 @@ array_concat(JSContext *cx, uintN argc, jsval *vp) /* Create a new Array object and root it using *vp. */ aobj = JS_THIS_OBJECT(cx, vp); if (OBJ_IS_DENSE_ARRAY(cx, aobj)) { - nobj = js_NewArrayObject(cx, ARRAY_DENSE_LENGTH(aobj), aobj->dslots, - JS_TRUE); + /* + * Clone aobj but pass the minimum of its length and capacity (aka + * "dense length"), to handle a = [1,2,3]; a.length = 10000 "dense" + * cases efficiently. In such a case we'll pass 8 (not 3) due to the + * ARRAY_GROWBY over-allocation policy, which will cause nobj to be + * over-allocated to 16. But in the normal case where length is <= + * capacity, nobj and aobj will have the same dense length. + */ + length = aobj->fslots[JSSLOT_ARRAY_LENGTH]; + jsuint capacity = ARRAY_DENSE_LENGTH(aobj); + nobj = js_NewArrayObject(cx, JS_MIN(length, capacity), aobj->dslots, + aobj->fslots[JSSLOT_ARRAY_COUNT] != + (jsval) length); if (!nobj) return JS_FALSE; - length = aobj->fslots[JSSLOT_ARRAY_LENGTH]; nobj->fslots[JSSLOT_ARRAY_LENGTH] = length; *vp = OBJECT_TO_JSVAL(nobj); if (argc == 0) @@ -2492,7 +2502,8 @@ array_slice(JSContext *cx, uintN argc, jsval *vp) if (OBJ_IS_DENSE_ARRAY(cx, obj) && end <= ARRAY_DENSE_LENGTH(obj)) { nobj = js_NewArrayObject(cx, end - begin, obj->dslots + begin, - JS_TRUE); + obj->fslots[JSSLOT_ARRAY_COUNT] != + obj->fslots[JSSLOT_ARRAY_LENGTH]); if (!nobj) return JS_FALSE; *vp = OBJECT_TO_JSVAL(nobj);