Bug 612523 - unfuse JSOP_MOREITER; sanity returns (r=dvander,gal)

--HG--
extra : rebase_source : ceaec335708f14f6ef4f0afaeb7e407b328145fa

js/src/jsapi-tests/Makefile.in

@@ -76,6 +76,7 @@ CPPSRCS = \
   testUTF8.cpp \
   testVersion.cpp \
   testXDR.cpp \
+  testCustomIterator.cpp \
   $(NULL)
 
 DEFINES         += -DEXPORT_JS_API

js/src/jsapi-tests/testCustomIterator.cpp

@@ -0,0 +1,81 @@
+#include "tests.h"
+
+#include "jsvalue.h"
+
+int count = 0;
+
+static JSBool
+IterNext(JSContext *cx, uintN argc, jsval *vp)
+{
+    if (count++ == 100)
+        return JS_ThrowStopIteration(cx);
+    JS_SET_RVAL(cx, vp, INT_TO_JSVAL(count));
+    return true;
+}
+
+static JSObject *
+IterHook(JSContext *cx, JSObject *obj, JSBool keysonly)
+{
+    JSObject *iterObj = JS_NewObject(cx, NULL, NULL, NULL);
+    if (!iterObj)
+        return NULL;
+    if (!JS_DefineFunction(cx, iterObj, "next", IterNext, 0, 0))
+        return NULL;
+    return iterObj;
+}
+
+js::Class HasCustomIterClass = {
+    "HasCustomIter",
+    0,
+    js::PropertyStub,
+    js::PropertyStub,
+    js::PropertyStub,
+    js::PropertyStub,
+    js::EnumerateStub,
+    js::ResolveStub,
+    js::ConvertStub,
+    NULL,
+    NULL, /* reserved0 */
+    NULL, /* checkAccess */
+    NULL, /* call */
+    NULL, /* construct */
+    NULL, /* xdrObject */
+    NULL, /* hasInstance */
+    NULL, /* mark */
+    {
+        NULL,
+        NULL,
+        NULL,
+        IterHook,
+        NULL
+    }
+};
+
+JSBool
+IterClassConstructor(JSContext *cx, uintN argc, jsval *vp)
+{
+    JSObject *obj = JS_NewObjectForConstructor(cx, vp);
+    if (!obj)
+        return false;
+    JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(obj));
+    return true;
+}
+
+BEGIN_TEST(testCustomIterator_bug612523)
+{
+    CHECK(JS_InitClass(cx, JS_GetGlobalObject(cx), NULL, Jsvalify(&HasCustomIterClass),
+                       IterClassConstructor, 0, NULL, NULL, NULL, NULL));
+
+    jsval result;
+    EVAL("var o = new HasCustomIter(); \n"
+         "var j = 0; \n"
+         "for (var i in o) { ++j; }; \n"
+         "j;", &result);
+
+    CHECK(JSVAL_IS_INT(result));
+    CHECK(JSVAL_TO_INT(result) == 100);
+    CHECK(count == 101);
+
+    return true;
+}
+END_TEST(testCustomIterator_bug612523)

js/src/jsinterp.cpp

@@ -3070,8 +3070,6 @@ BEGIN_CASE(JSOP_MOREITER)
     if (!IteratorMore(cx, &regs.sp[-2].toObject(), &cond, &regs.sp[-1]))
         goto error;
     CHECK_INTERRUPT_HANDLER();
-    TRY_BRANCH_AFTER_COND(cond, 1);
-    JS_ASSERT(regs.pc[1] == JSOP_IFNEX);
     regs.sp[-1].setBoolean(cond);
 }
 END_CASE(JSOP_MOREITER)

js/src/jstracer.cpp

@@ -6661,7 +6661,7 @@ LeaveTree(TraceMonitor *tm, TracerState& state, VMSideExit* lr)
          * the interpreter stack, in pre-call state, with pc pointing to the
          * CALL/APPLY op, for correctness. Then we continued in native code.
          */
-        if (!(bs & (BUILTIN_ERROR | BUILTIN_NO_FIXUP_NEEDED))) {
+        if (!(bs & BUILTIN_ERROR)) {
             /*
              * The builtin or native deep-bailed but finished successfully
              * (no exception or error).
@@ -9610,7 +9610,7 @@ TraceRecorder::is_boxed_true(Address addr)
     LIns *tag_ins = w.ldiValueTag(addr);
     LIns *bool_ins = w.eqi(tag_ins, w.nameImmui(JSVAL_TAG_BOOLEAN));
     LIns *payload_ins = w.ldiValuePayload(addr);
-    return w.andi(bool_ins, payload_ins);
+    return w.gtiN(w.andi(bool_ins, payload_ins), 0);
 }
 
 LIns*
@@ -14313,13 +14313,10 @@ static JSBool FASTCALL
 IteratorMore(JSContext *cx, JSObject *iterobj, Value *vp)
 {
     if (!js_IteratorMore(cx, iterobj, vp)) {
-        SetBuiltinError(cx, BUILTIN_ERROR_NO_FIXUP_NEEDED);
-        return false;
-    } else if (cx->tracerState->builtinStatus) {
-        SetBuiltinError(cx, BUILTIN_NO_FIXUP_NEEDED);
+        SetBuiltinError(cx);
         return false;
     }
-    return true;
+    return cx->tracerState->builtinStatus == 0;
 }
 JS_DEFINE_CALLINFO_3(extern, BOOL_FAIL, IteratorMore, CONTEXT, OBJECT, VALUEPTR,
                      0, ACCSET_STORE_ANY)
@@ -14335,22 +14332,16 @@ TraceRecorder::record_JSOP_MOREITER()
 
     JSObject* iterobj = &iterobj_val.toObject();
     LIns* iterobj_ins = get(&iterobj_val);
-    bool cond;
     LIns* cond_ins;
 
     /* JSOP_FOR* already guards on this, but in certain rare cases we might record misformed loop traces. */
     if (iterobj->hasClass(&js_IteratorClass)) {
         guardClass(iterobj_ins, &js_IteratorClass, snapshot(BRANCH_EXIT), LOAD_NORMAL);
-        NativeIterator *ni = (NativeIterator *) iterobj->getPrivate();
-        void *cursor = ni->props_cursor;
-        void *end = ni->props_end;
 
         LIns *ni_ins = w.ldpObjPrivate(iterobj_ins);
         LIns *cursor_ins = w.ldpIterCursor(ni_ins);
         LIns *end_ins = w.ldpIterEnd(ni_ins);
 
-        /* Figure out whether the native iterator contains more values. */
-        cond = cursor < end;
         cond_ins = w.ltp(cursor_ins, end_ins);
     } else {
         guardNotClass(iterobj_ins, &js_IteratorClass, snapshot(BRANCH_EXIT), LOAD_NORMAL);
@@ -14361,40 +14352,12 @@ TraceRecorder::record_JSOP_MOREITER()
         LIns* args[] = { vp_ins, iterobj_ins, cx_ins };
         LIns* ok_ins = w.call(&IteratorMore_ci, args);
 
-        /*
-         * We cannot use pendingGuardCondition since monitorRecording may not be
-         * triggered if we close the loop below with endLoop. Instead, we guard
-         * here with STATUS_EXIT. By default, STATUS_EXIT means "advance the pc
-         * and fixup the stack", so IteratorMore sets BUILTIN_NO_FIXUP_NEEDED.
-         * If IteratorMore fails, we will reexecute this op in the interpreter,
-         * but js_IteratoreMore is idempotent so this is ok.
-         */
-        guard(false, w.eqi0(ok_ins), STATUS_EXIT);
-
+        pendingGuardCondition = w.eqi0(ok_ins);
         leaveDeepBailCall();
 
-        /*
-         * The interpreter will call js_IteratorMore again, but that's legal. We have to
-         * carefully protect ourselves against reentrancy.
-         */
-        JSContext *localCx = cx;
-        AutoValueRooter rooter(cx);
-        if (!js_IteratorMore(cx, iterobj, rooter.addr()))
-            RETURN_ERROR_A("error in js_IteratorMore");
-        if (!TRACE_RECORDER(localCx))
-            return ARECORD_ABORTED;
-
-        cond = (rooter.value().isTrue());
         cond_ins = w.eqi0(w.eqi0(is_boxed_true(AllocSlotsAddress(vp_ins))));
     }
 
-    jsbytecode* pc = cx->regs->pc;
-
-    if (pc[1] == JSOP_IFNE) {
-        fuseIf(pc + 1, cond, cond_ins);
-        return checkTraceEnd(pc + 1);
-    }
-
     stack(0, cond_ins);
 
     return ARECORD_CONTINUE;

js/src/jstracer.h

@@ -819,16 +819,13 @@ public:
  */
 typedef enum BuiltinStatus {
     BUILTIN_BAILED = 1,
-    BUILTIN_ERROR = 2,
-    BUILTIN_NO_FIXUP_NEEDED = 4,
-
-    BUILTIN_ERROR_NO_FIXUP_NEEDED = BUILTIN_ERROR | BUILTIN_NO_FIXUP_NEEDED
+    BUILTIN_ERROR = 2
 } BuiltinStatus;
 
 static JS_INLINE void
-SetBuiltinError(JSContext *cx, BuiltinStatus status = BUILTIN_ERROR)
+SetBuiltinError(JSContext *cx)
 {
-    cx->tracerState->builtinStatus |= status;
+    cx->tracerState->builtinStatus |= BUILTIN_ERROR;
 }
 
 #ifdef DEBUG_RECORDING_STATUS_NOT_BOOL