Bug 601733 - respect deleted arguments-object properties in JSOP_GETELEM (r=dvander,dmandelin)

js/src/jsfun.cpp

@@ -518,15 +518,11 @@ ArgGetter(JSContext *cx, JSObject *obj, jsid id, Value *vp)
          */
         uintN arg = uintN(JSID_TO_INT(id));
         if (arg < obj->getArgsInitialLength()) {
-            JSStackFrame *fp = (JSStackFrame *) obj->getPrivate();
-            if (fp) {
-                JS_ASSERT(fp->numActualArgs() == obj->getArgsInitialLength());
+            JS_ASSERT(!obj->getArgsElement(arg).isMagic(JS_ARGS_HOLE));
+            if (JSStackFrame *fp = (JSStackFrame *) obj->getPrivate())
                 *vp = fp->canonicalActualArg(arg);
-            } else {
-                const Value &v = obj->getArgsElement(arg);
-                if (!v.isMagic(JS_ARGS_HOLE))
-                    *vp = v;
-            }
+            else
+                *vp = obj->getArgsElement(arg);
         }
     } else if (JSID_IS_ATOM(id, cx->runtime->atomState.lengthAtom)) {
         if (!obj->isArgsLengthOverridden())

js/src/jsinterp.cpp

@@ -4378,24 +4378,17 @@ BEGIN_CASE(JSOP_GETELEM)
                 copyFrom = obj->addressOfDenseArrayElement(idx);
                 if (!copyFrom->isMagic())
                     goto end_getelem;
-
-                /* Reload retval from the stack in the rare hole case. */
-                copyFrom = &regs.sp[-1];
             }
         } else if (obj->isArguments()) {
             uint32 arg = uint32(i);
 
             if (arg < obj->getArgsInitialLength()) {
-                JSStackFrame *afp = (JSStackFrame *) obj->getPrivate();
-                if (afp) {
-                    copyFrom = &afp->canonicalActualArg(arg);
+                copyFrom = obj->addressOfArgsElement(arg);
+                if (!copyFrom->isMagic(JS_ARGS_HOLE)) {
+                    if (JSStackFrame *afp = (JSStackFrame *) obj->getPrivate())
+                        copyFrom = &afp->canonicalActualArg(arg);
                     goto end_getelem;
                 }
-
-                copyFrom = obj->addressOfArgsElement(arg);
-                if (!copyFrom->isMagic())
-                    goto end_getelem;
-                copyFrom = &regs.sp[-1];
             }
         }
         if (JS_LIKELY(INT_FITS_IN_JSID(i)))

js/src/jstracer.cpp

@@ -9732,6 +9732,13 @@ TraceRecorder::is_boxed_true(LIns *vaddr_ins, AccSet accSet)
     return lir->ins2(LIR_andi, bool_ins, payload_ins);
 }
 
+LIns*
+TraceRecorder::is_boxed_magic(LIns *vaddr_ins, JSWhyMagic why, AccSet accSet)
+{
+    LIns *tag_ins = lir->insLoad(LIR_ldi, vaddr_ins, sTagOffset, accSet);
+    return lir->ins2(LIR_eqi, tag_ins, INS_CONSTU(JSVAL_TAG_MAGIC));
+}
+
 void
 TraceRecorder::box_value_into(const Value &v, LIns *v_ins, LIns *dstaddr_ins, ptrdiff_t offset,
                               AccSet accSet)
@@ -9903,6 +9910,13 @@ TraceRecorder::is_boxed_true(LIns *vaddr_ins, AccSet accSet)
     return lir->ins2(LIR_eqq, v_ins, lir->insImmQ(JSVAL_BITS(JSVAL_TRUE)));
 }
 
+LIns*
+TraceRecorder::is_boxed_magic(LIns *vaddr_ins, JSWhyMagic why, AccSet accSet)
+{
+    LIns *v_ins = lir->insLoad(LIR_ldq, vaddr_ins, 0, accSet);
+    return lir->ins2(LIR_eqq, v_ins, INS_CONSTQWORD(BUILD_JSVAL(JSVAL_TAG_MAGIC, why)));
+}
+
 LIns*
 TraceRecorder::box_value_for_native_call(const Value &v, LIns *v_ins)
 {
@@ -10362,14 +10376,18 @@ TraceRecorder::record_EnterFrame()
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_LeaveFrame()
 {
+    JSStackFrame *fp = cx->fp();
+
     debug_only_stmt(
         debug_only_printf(LC_TMTracer,
                           "LeaveFrame (back to %s), callDepth=%d\n",
-                          js_AtomToPrintableString(cx, cx->fp()->fun()->atom),
+                          fp->isFunctionFrame()
+                          ? js_AtomToPrintableString(cx, fp->fun()->atom)
+                          : "global code",
                           callDepth);
         );
 
-    JS_ASSERT(js_CodeSpec[js_GetOpcode(cx, cx->fp()->script(),
+    JS_ASSERT(js_CodeSpec[js_GetOpcode(cx, fp->script(),
               cx->regs->pc)].length == JSOP_CALL_LENGTH);
 
     if (callDepth-- <= 0)
@@ -12547,6 +12565,21 @@ static bool OkToTraceTypedArrays = true;
 static bool OkToTraceTypedArrays = false;
 #endif
 
+JS_REQUIRES_STACK void
+TraceRecorder::guardNotHole(LIns *argsobj_ins, LIns *idx_ins)
+{
+    // vp = &argsobj->fslots[JSSLOT_ARGS_DATA].slots[idx]
+    LIns* argsData_ins = stobj_get_fslot_private_ptr(argsobj_ins, JSObject::JSSLOT_ARGS_DATA);
+    LIns* slotOffset_ins = lir->ins2(LIR_addp,
+                                     INS_CONSTWORD(offsetof(ArgumentsData, slots)),
+                                     lir->insUI2P(lir->ins2ImmI(LIR_muli, idx_ins, sizeof(Value))));
+    LIns* vp_ins = lir->ins2(LIR_addp, argsData_ins, slotOffset_ins);
+
+    guard(false,
+          addName(is_boxed_magic(vp_ins, JS_ARGS_HOLE, ACCSET_OTHER), "guard(not deleted arg)"),
+          MISMATCH_EXIT);
+}
+
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_JSOP_GETELEM()
 {
@@ -12587,14 +12620,21 @@ TraceRecorder::record_JSOP_GETELEM()
     }
 
     if (obj->isArguments()) {
+        // Don't even try to record if out of range or reading a deleted arg
+        int32 int_idx = idx.toInt32();
+        if (int_idx < 0 || int_idx >= (int32)obj->getArgsInitialLength())
+            RETURN_STOP_A("cannot trace arguments with out of range index");
+        if (obj->getArgsElement(int_idx).isMagic(JS_ARGS_HOLE))
+            RETURN_STOP_A("reading deleted args element");
+
+        // Only trace reading arguments out of active, tracked frame
         unsigned depth;
         JSStackFrame *afp = guardArguments(obj, obj_ins, &depth);
         if (afp) {
-            int32 int_idx = idx.toInt32();
-            if (int_idx < 0 || int_idx >= (int32)afp->numActualArgs())
-                RETURN_STOP_A("cannot trace arguments with out of range index");
             Value* vp = &afp->canonicalActualArg(int_idx);
             if (idx_ins->isImmD()) {
+                JS_ASSERT(int_idx == (int32)idx_ins->immD());
+                guardNotHole(obj_ins, INS_CONST(int_idx));
                 v_ins = get(vp);
             } else {
                 // If the index is not a constant expression, we generate LIR to load the value from
@@ -12611,6 +12651,8 @@ TraceRecorder::record_JSOP_GETELEM()
                               "guard(upvar index in range)"),
                       MISMATCH_EXIT);
 
+                guardNotHole(obj_ins, idx_ins);
+
                 JSValueType type = getCoercedType(*vp);
 
                 // Guard that the argument has the same type on trace as during recording.

js/src/jstracer.h

@@ -1285,6 +1285,7 @@ class TraceRecorder
     void unbox_any_object(nanojit::LIns* vaddr_ins, nanojit::LIns** obj_ins,
                           nanojit::LIns** is_obj_ins, nanojit::AccSet accSet);
     nanojit::LIns* is_boxed_true(nanojit::LIns* vaddr_ins, nanojit::AccSet accSet);
+    nanojit::LIns* is_boxed_magic(nanojit::LIns* vaddr_ins, JSWhyMagic why, nanojit::AccSet accSet);
 
     nanojit::LIns* is_string_id(nanojit::LIns* id_ins);
     nanojit::LIns* unbox_string_id(nanojit::LIns* id_ins);
@@ -1327,6 +1328,7 @@ class TraceRecorder
     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 void guardNotHole(nanojit::LIns *argsobj_ins, nanojit::LIns *ids_ins);
     JS_REQUIRES_STACK RecordingStatus getClassPrototype(JSObject* ctor,
                                                           nanojit::LIns*& proto_ins);
     JS_REQUIRES_STACK RecordingStatus getClassPrototype(JSProtoKey key,

js/src/methodjit/StubCalls.cpp

@@ -531,24 +531,17 @@ stubs::GetElem(VMFrame &f)
                 copyFrom = obj->addressOfDenseArrayElement(idx);
                 if (!copyFrom->isMagic())
                     goto end_getelem;
-
-                /* Reload retval from the stack in the rare hole case. */
-                copyFrom = &regs.sp[-1];
             }
         } else if (obj->isArguments()) {
             uint32 arg = uint32(i);
 
             if (arg < obj->getArgsInitialLength()) {
-                JSStackFrame *afp = (JSStackFrame *) obj->getPrivate();
-                if (afp) {
-                    copyFrom = &afp->canonicalActualArg(arg);
+                copyFrom = obj->addressOfArgsElement(arg);
+                if (!copyFrom->isMagic()) {
+                    if (JSStackFrame *afp = (JSStackFrame *) obj->getPrivate())
+                        copyFrom = &afp->canonicalActualArg(arg);
                     goto end_getelem;
                 }
-
-                copyFrom = obj->addressOfArgsElement(arg);
-                if (!copyFrom->isMagic())
-                    goto end_getelem;
-                /* Otherwise, fall to getProperty(). */
             }
         }
         if (JS_LIKELY(INT_FITS_IN_JSID(i)))

js/src/trace-test/tests/arguments/testDelArg1.js

@@ -0,0 +1,13 @@
+function f(x,y,z) {
+  z = 9;
+  delete arguments[2];
+  assertEq(arguments[2], undefined);
+  o = arguments;
+  assertEq(o[2], undefined);
+  assertEq(o[2] == undefined, true);
+}
+
+for (var i = 0; i < HOTLOOP+2; ++i) {
+    print(i);
+    f(1,2,3)
+}

js/src/trace-test/tests/arguments/testDelArg2.js

@@ -0,0 +1,13 @@
+function f(del) {
+    o = arguments;
+    if (del)
+        delete o[2];
+    for (var i = 0; i < HOTLOOP+2; ++i)
+        assertEq(o[2] == undefined, del);
+}
+
+// record without arg deleted
+f(false, 1,2,3,4);
+
+// run with arg deleted
+f(true, 1,2,3,4);