Bug 466905 - Fix JSOP_NEWARRAY to be not-buggy and use it when possible. NOT REVIEWED YET

js/src/jsemit.cpp

@@ -5994,7 +5994,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
          * array comprehension, use JSOP_NEWARRAY.
          */
         pn2 = pn->pn_head;
-        op = JSOP_NEWINIT;      // FIXME: 260106 patch disabled for now
+        op = JSOP_NEWARRAY;
 
 #if JS_HAS_SHARP_VARS
         if (pn2 && pn2->pn_type == TOK_DEFSHARP)

js/src/jsopcode.cpp

@@ -1,4 +1,4 @@
-/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set sw=4 ts=8 et tw=99:
  *
  * ***** BEGIN LICENSE BLOCK *****
@@ -3468,20 +3468,16 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
 #if JS_HAS_LVALUE_RETURN
               case JSOP_SETCALL:
 #endif
-                /* Turn off most parens (all if there's only one argument). */
                 argc = GET_ARGC(pc);
-                op = (argc == 1) ? JSOP_NOP : JSOP_SETNAME;
                 argv = (char **)
                     JS_malloc(cx, (size_t)(argc + 1) * sizeof *argv);
                 if (!argv)
                     return NULL;
 
+                op = JSOP_SETNAME;
                 ok = JS_TRUE;
-                for (i = argc; i > 0; i--) {
+                for (i = argc; i > 0; i--)
                     argv[i] = JS_strdup(cx, POP_STR());
-                    if (!argv[i])
-                        ok = JS_FALSE;
-                }
 
                 /* Skip the JSOP_PUSHOBJ-created empty string. */
                 LOCAL_ASSERT(ss->top >= 2);
@@ -3492,7 +3488,7 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
                  * Same for new (x(y).z) -- contrast with new x(y).z.
                  * See PROPAGATE_CALLNESS.
                  */
-                op = (JSOp) ss->opcodes[ss->top-1];
+                op = (JSOp) ss->opcodes[ss->top - 1];
                 lval = PopStr(ss,
                               (saveop == JSOP_NEW &&
                                (op == JSOP_CALL || 
@@ -3504,7 +3500,7 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
                 op = saveop;
 
                 argv[0] = JS_strdup(cx, lval);
-                if (!argv[i])
+                if (!argv[0])
                     ok = JS_FALSE;
 
                 lval = "(", rval = ")";
@@ -3531,10 +3527,8 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
                 if (Sprint(&ss->sprinter, rval) < 0)
                     ok = JS_FALSE;
 
-                for (i = 0; i <= argc; i++) {
-                    if (argv[i])
+                for (i = 0; i <= argc; i++)
                     JS_free(cx, argv[i]);
-                }
                 JS_free(cx, argv);
                 if (!ok)
                     return NULL;
@@ -4287,53 +4281,48 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
                 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)) {
+                    todo = SprintCString(&ss->sprinter, "[]");
                     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);
-                }
+
+                argv = (char **) JS_malloc(cx, size_t(argc) * sizeof *argv);
+                if (!argv)
+                    return NULL;
+
+                op = JSOP_SETNAME;
+                ok = JS_TRUE;
+                i = argc;
+                while (i > 0)
+                    argv[--i] = JS_strdup(cx, POP_STR());
+
+                todo = SprintCString(&ss->sprinter, "[");
+                if (todo < 0)
+                    break;
+
+                for (i = 0; i < argc; i++) {
+                    if (!argv[i] ||
+                        Sprint(&ss->sprinter, ss_format,
+                               argv[i], (i < argc - 1) ? ", " : "") < 0) {
+                        ok = JS_FALSE;
                         break;
                     }
+                }
+
+                for (i = 0; i < argc; i++)
+                    JS_free(cx, argv[i]);
+                JS_free(cx, argv);
+                if (!ok)
+                    return NULL;
+
+                sn = js_GetSrcNote(jp->script, pc);
+                if (sn && SN_TYPE(sn) == SRC_CONTINUE && SprintCString(&ss->sprinter, ", ") < 0)
+                    return NULL;
+                if (SprintCString(&ss->sprinter, "]") < 0)
+                    return NULL;
+                break;
 
               case JSOP_NEWINIT:
               {

js/src/jsopcode.tbl

@@ -559,8 +559,17 @@ OPDEF(JSOP_INT32,         228, "int32",        NULL,  5,  0,  1, 16,  JOF_INT32)
 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).
+ * Construct a new dense array whose contents are the values provided on the
+ * stack, consuming those values and replacing them with the newly-constructed
+ * array.  The topmost value is the last value in the new array, and the
+ * bottommost value is the first value in the array; the array length is a
+ * 24-bit immediate operand to the instruction.
  */
 OPDEF(JSOP_NEWARRAY,      230, "newarray",     NULL,  4, -1,  1, 19,  JOF_UINT24)
+
+/*
+ * Push a JSVAL_HOLE value onto the stack, representing an omitted property in
+ * an array literal (e.g. property 0 in the array [, 1]).  This opcode is used
+ * with the JSOP_NEWARRAY and JSOP_NEWINIT opcodes.
+ */
 OPDEF(JSOP_HOLE,          231, "hole",         NULL,  1,  0,  1,  0,  JOF_BYTE)

js/src/jstracer.cpp

@@ -4565,8 +4565,7 @@ TraceRecorder::incProp(jsint incr, bool pre)
     if (!inc(v, v_ins, incr, pre))
         return false;
 
-    if (!box_jsval(v, v_ins))
-        return false;
+    box_jsval(v, v_ins);
 
     LIns* dslots_ins = NULL;
     stobj_set_slot(obj_ins, slot, dslots_ins, v_ins);
@@ -4587,8 +4586,7 @@ TraceRecorder::incElem(jsint incr, bool pre)
         return false;
     if (!inc(*vp, v_ins, incr, pre))
         return false;
-    if (!box_jsval(*vp, v_ins))
-        return false;
+    box_jsval(*vp, v_ins);
     lir->insStorei(v_ins, addr_ins, 0);
     return true;
 }
@@ -5282,7 +5280,7 @@ TraceRecorder::native_get(LIns* obj_ins, LIns* pobj_ins, JSScopeProperty* sprop,
 // So box_jsval can emit no LIR_or at all to tag an object jsval.
 JS_STATIC_ASSERT(JSVAL_OBJECT == 0);
 
-JS_REQUIRES_STACK bool
+JS_REQUIRES_STACK void
 TraceRecorder::box_jsval(jsval v, LIns*& v_ins)
 {
     if (isNumber(v)) {
@@ -5290,19 +5288,19 @@ TraceRecorder::box_jsval(jsval v, LIns*& v_ins)
         v_ins = lir->insCall(&js_BoxDouble_ci, args);
         guard(false, lir->ins2(LIR_eq, v_ins, INS_CONST(JSVAL_ERROR_COOKIE)),
               OOM_EXIT);
-        return true;
+        return;
     }
     switch (JSVAL_TAG(v)) {
       case JSVAL_BOOLEAN:
         v_ins = lir->ins2i(LIR_pior, lir->ins2i(LIR_pilsh, v_ins, JSVAL_TAGBITS), JSVAL_BOOLEAN);
-        return true;
+        return;
       case JSVAL_OBJECT:
-        return true;
-      case JSVAL_STRING:
+        return;
+      default:
+        JS_ASSERT(JSVAL_TAG(v) == JSVAL_STRING);
         v_ins = lir->ins2(LIR_pior, v_ins, INS_CONST(JSVAL_STRING));
-        return true;
+        return;
     }
-    return false;
 }
 
 JS_REQUIRES_STACK bool
@@ -5589,8 +5587,7 @@ TraceRecorder::record_JSOP_POPV()
 {
     jsval& rval = stackval(-1);
     LIns *rval_ins = get(&rval);
-    if (!box_jsval(rval, rval_ins))
-        return false;
+    box_jsval(rval, rval_ins);
 
     // Store it in cx->fp->rval. NB: Tricky dependencies. cx->fp is the right
     // frame because POPV appears only in global and eval code and we don't
@@ -6027,8 +6024,7 @@ TraceRecorder::newArray(JSObject *ctor, uint32 argc, jsval *argv, jsval *rval)
         LIns *dslots_ins = NULL;
         for (uint32 i = 0; i < argc; i++) {
             LIns *elt_ins = get(argv + i);
-            if (!box_jsval(argv[i], elt_ins))
-                return false;
+            box_jsval(argv[i], elt_ins);
             stobj_set_dslot(arr_ins, i, dslots_ins, elt_ins, "set_array_elt");
         }
     }
@@ -6164,8 +6160,7 @@ TraceRecorder::functionCall(bool constructing, uintN argc)
                 if (!VALUE_IS_FUNCTION(cx, arg))
                     goto next_specialization;
             } else if (argtype == 'v') {
-                if (!box_jsval(arg, *argp))
-                    return false;
+                box_jsval(arg, *argp);
             } else {
                 goto next_specialization;
             }
@@ -6457,8 +6452,7 @@ TraceRecorder::record_SetPropHit(JSPropCacheEntry* entry, JSScopeProperty* sprop
     LIns* dslots_ins = NULL;
     LIns* v_ins = get(&r);
     LIns* boxed_ins = v_ins;
-    if (!box_jsval(r, boxed_ins))
-        return false;
+    box_jsval(r, boxed_ins);
     if (!native_set(obj_ins, sprop, dslots_ins, boxed_ins))
         return false;
 
@@ -6589,8 +6583,7 @@ TraceRecorder::record_JSOP_SETELEM()
     jsid id;
 
     LIns* boxed_v_ins = v_ins;
-    if (!box_jsval(v, boxed_v_ins))
-        ABORT_TRACE("boxing JSOP_SETELEM value");
+    box_jsval(v, boxed_v_ins);
 
     if (JSVAL_IS_STRING(idx)) {
         if (!js_ValueToStringId(cx, idx, &id))
@@ -7299,18 +7292,6 @@ TraceRecorder::record_JSOP_ENDINIT()
 {
     jsval& v = stackval(-1);
     JS_ASSERT(!JSVAL_IS_PRIMITIVE(v));
-    JSObject* obj = JSVAL_TO_OBJECT(v);
-    if (OBJ_IS_DENSE_ARRAY(cx, obj)) {
-        // Until we get JSOP_NEWARRAY working, we do our optimizing here...
-        if (obj->fslots[JSSLOT_ARRAY_LENGTH] == 1 &&
-            obj->dslots && JSVAL_IS_STRING(obj->dslots[0])) {
-            LIns* v_ins = get(&v);
-            JS_ASSERT(v_ins->isCall() && v_ins->callInfo() == &js_FastNewArray_ci);
-            LIns* args[] = { stack(1), callArgN(v_ins, 1), cx_ins };
-            v_ins = lir->insCall(&js_Array_1str_ci, args);
-            set(&v, v_ins);
-        }
-    }
     return true;
 }
 
@@ -8599,7 +8580,33 @@ TraceRecorder::record_JSOP_LENGTH()
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_NEWARRAY()
 {
+    JSObject* proto;
+    const CallInfo* ci = &js_NewUninitializedArray_ci;
+    if (!js_GetClassPrototype(cx, globalObj, INT_TO_JSID(JSProto_Array), &proto))
         return false;
+
+    uint32 len = GET_UINT24(cx->fp->regs->pc);
+    LIns* args[] = { lir->insImm(len), INS_CONSTPTR(proto), cx_ins };
+    LIns* v_ins = lir->insCall(ci, args);
+    guard(false, lir->ins_eq0(v_ins), OOM_EXIT);
+
+    // De-optimize when we might have setters on Array.prototype.
+    LIns* rt_ins = lir->insLoad(LIR_ldp, cx_ins, offsetof(JSContext, runtime));
+    guard(true,
+          lir->ins_eq0(lir->insLoad(LIR_ldp, rt_ins,
+                                    offsetof(JSRuntime, anyArrayProtoHasElement))),
+          MISMATCH_EXIT);
+
+    LIns* dslots_ins = NULL;
+    for (uint32 i = 0; i < len; i++) {
+        jsval& v = stackval(-len + i);
+        LIns* elt_ins = get(&v);
+        box_jsval(v, elt_ins);
+        stobj_set_dslot(v_ins, i, dslots_ins, elt_ins, "set_array_elt");
+    }
+
+    stack(-len, v_ins);
+    return true;
 }
 
 JS_REQUIRES_STACK bool

js/src/jstracer.h

@@ -414,7 +414,7 @@ class TraceRecorder : public avmplus::GCObject {
     JS_REQUIRES_STACK bool getProp(jsval& v);
     JS_REQUIRES_STACK bool getThis(nanojit::LIns*& this_ins);
 
-    JS_REQUIRES_STACK bool box_jsval(jsval v, nanojit::LIns*& v_ins);
+    JS_REQUIRES_STACK void box_jsval(jsval v, nanojit::LIns*& v_ins);
     JS_REQUIRES_STACK bool unbox_jsval(jsval v, nanojit::LIns*& v_ins);
     JS_REQUIRES_STACK bool guardClass(JSObject* obj, nanojit::LIns* obj_ins, JSClass* clasp,
                                       ExitType exitType = MISMATCH_EXIT);