Bug 539553 - Correctness regression on the r-tree benchmark. r=dmandelin.
authorJason Orendorff <jorendorff@mozilla.com>
Thu, 14 Jan 2010 18:23:05 -0600
changeset 37691 aba69ed5c41d80d5344475dd3a42eb92a3d16efe
parent 37690 3036013432ce47875bce99e7c1ab54cdc1532ce4
child 37692 a39e0b5578645dd64e11a4939a9de3ca9ff8b08b
push idunknown
push userunknown
push dateunknown
reviewersdmandelin
bugs539553
milestone1.9.3a1pre
Bug 539553 - Correctness regression on the r-tree benchmark. r=dmandelin.
js/src/jstracer.cpp
js/src/jstracer.h
js/src/trace-test/tests/basic/bug539553-2.js
js/src/trace-test/tests/basic/bug539553.js
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -13976,16 +13976,27 @@ TraceRecorder::record_JSOP_ARGSUB()
             stack(0, get(&cx->fp->argv[slot]));
         else
             stack(0, INS_VOID());
         return ARECORD_CONTINUE;
     }
     RETURN_STOP_A("can't trace JSOP_ARGSUB hard case");
 }
 
+JS_REQUIRES_STACK LIns*
+TraceRecorder::guardArgsLengthNotAssigned(LIns* argsobj_ins)
+{
+    // The following implements js_IsOverriddenArgsLength on trace.
+    // The '2' bit is set if length was overridden.
+    LIns *len_ins = stobj_get_fslot(argsobj_ins, JSSLOT_ARGS_LENGTH);
+    LIns *ovr_ins = lir->ins2(LIR_piand, len_ins, INS_CONSTWORD(2));
+    guard(true, lir->ins_peq0(ovr_ins), snapshot(BRANCH_EXIT));
+    return len_ins;
+}
+
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_JSOP_ARGCNT()
 {
     if (cx->fp->fun->flags & JSFUN_HEAVYWEIGHT)
         RETURN_STOP_A("can't trace heavyweight JSOP_ARGCNT");
 
     // argc is fixed on trace, so ideally we would simply generate LIR for
     // constant argc. But the user can mutate arguments.length in the
@@ -13993,23 +14004,17 @@ TraceRecorder::record_JSOP_ARGCNT()
     // 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().
     if (cx->fp->argsobj && js_IsOverriddenArgsLength(JSVAL_TO_OBJECT(cx->fp->argsobj)))
         RETURN_STOP_A("can't trace JSOP_ARGCNT if arguments.length has been modified");
     LIns *a_ins = get(&cx->fp->argsobj);
     if (callDepth == 0) {
         LIns *br = lir->insBranch(LIR_jt, lir->ins_peq0(a_ins), NULL);
-
-        // The following implements js_IsOverriddenArgsLength on trace.
-        // The '2' bit is set if length was overridden.
-        LIns *len_ins = stobj_get_fslot(a_ins, JSSLOT_ARGS_LENGTH);
-        LIns *ovr_ins = lir->ins2(LIR_piand, len_ins, INS_CONSTWORD(2));
-
-        guard(true, lir->ins_peq0(ovr_ins), snapshot(BRANCH_EXIT));
+        guardArgsLengthNotAssigned(a_ins);
         LIns *label = lir->ins0(LIR_label);
         br->setTarget(label);
     }
     stack(0, lir->insImmf(cx->fp->argc));
     return ARECORD_CONTINUE;
 }
 
 JS_REQUIRES_STACK AbortableRecordingStatus
@@ -14854,17 +14859,25 @@ TraceRecorder::record_JSOP_LENGTH()
     LIns* obj_ins = get(&l);
 
     if (STOBJ_GET_CLASS(obj) == &js_ArgumentsClass) {
         unsigned depth;
         JSStackFrame *afp = guardArguments(obj, obj_ins, &depth);
         if (!afp)
             RETURN_STOP_A("can't reach arguments object's frame");
 
-        LIns* v_ins = lir->ins1(LIR_i2f, INS_CONST(afp->argc));
+        // We must both check at record time and guard at run time that
+        // arguments.length has not been reassigned, redefined or deleted.
+        if (js_IsOverriddenArgsLength(obj))
+            RETURN_STOP_A("can't trace JSOP_ARGCNT if arguments.length has been modified");
+        LIns* slot_ins = guardArgsLengthNotAssigned(obj_ins);
+
+        // slot_ins is the value from the slot; right-shift by 2 bits to get
+        // the length (see GetArgsLength in jsfun.cpp).
+        LIns* v_ins = lir->ins1(LIR_i2f, lir->ins2i(LIR_rsh, slot_ins, 2));
         set(&l, v_ins);
         return ARECORD_CONTINUE;
     }
 
     LIns* v_ins;
     if (OBJ_IS_ARRAY(cx, obj)) {
         if (OBJ_IS_DENSE_ARRAY(cx, obj)) {
             if (!guardDenseArray(obj, obj_ins, BRANCH_EXIT)) {
--- a/js/src/jstracer.h
+++ b/js/src/jstracer.h
@@ -1297,16 +1297,17 @@ class TraceRecorder
     JS_REQUIRES_STACK JSStackFrame* entryFrame() const;
     JS_REQUIRES_STACK void clearEntryFrameSlotsFromTracker(Tracker& which);
     JS_REQUIRES_STACK void clearCurrentFrameSlotsFromTracker(Tracker& which);
     JS_REQUIRES_STACK void clearFrameSlotsFromTracker(Tracker& which, JSStackFrame* fp, unsigned nslots);
     JS_REQUIRES_STACK void putArguments();
     JS_REQUIRES_STACK RecordingStatus guardCallee(jsval& callee);
     JS_REQUIRES_STACK JSStackFrame      *guardArguments(JSObject *obj, nanojit::LIns* obj_ins,
                                                         unsigned *depthp);
+    JS_REQUIRES_STACK nanojit::LIns* guardArgsLengthNotAssigned(nanojit::LIns* argsobj_ins);
     JS_REQUIRES_STACK RecordingStatus getClassPrototype(JSObject* ctor,
                                                           nanojit::LIns*& proto_ins);
     JS_REQUIRES_STACK RecordingStatus getClassPrototype(JSProtoKey key,
                                                           nanojit::LIns*& proto_ins);
     JS_REQUIRES_STACK RecordingStatus newArray(JSObject* ctor, uint32 argc, jsval* argv,
                                                  jsval* rval);
     JS_REQUIRES_STACK RecordingStatus newString(JSObject* ctor, uint32 argc, jsval* argv,
                                                   jsval* rval);
new file mode 100644
--- /dev/null
+++ b/js/src/trace-test/tests/basic/bug539553-2.js
@@ -0,0 +1,7 @@
+function f() {
+    var x = arguments;
+    arguments.length = {};
+    for (var i = 0; i < 9; i++)
+        x.length.toSource();
+}
+f();
new file mode 100644
--- /dev/null
+++ b/js/src/trace-test/tests/basic/bug539553.js
@@ -0,0 +1,11 @@
+function g(x) {
+    assertEq(x.length, 1);
+}
+
+function f() {
+    arguments.length = 1;
+    for (var i = 0; i < 9; i++)
+        g(arguments);
+}
+
+f();