diff --git a/js/src/jit-test/tests/arguments/testDelArg3.js b/js/src/jit-test/tests/arguments/testDelArg3.js new file mode 100644 index 000000000000..4931870bbcdd --- /dev/null +++ b/js/src/jit-test/tests/arguments/testDelArg3.js @@ -0,0 +1,42 @@ +function assertGood(x) { + assertEq(x, "good"); +} + +(function() { + var a = arguments; + return function() { + assertGood.apply(null, a); + } +})("good")(); + +(function() { + var a = arguments; + return function() { + a[0] = "good"; + assertGood.apply(null, a); + } +})("bad")(); + +Object.prototype[0] = "good"; + +(function() { + var a = arguments; + return function() { + delete a[0]; + assertGood.apply(null, a); + } +})("bad")(); + +delete Object.prototype[0]; + +function assertUndefined(x) { + assertEq(x, undefined); +} + +(function() { + var a = arguments; + return function() { + a[0] = "bad"; + assertUndefined.apply(null, a); + } +})()(); diff --git a/js/src/jit-test/tests/arguments/testDelArg3Strict.js b/js/src/jit-test/tests/arguments/testDelArg3Strict.js new file mode 100644 index 000000000000..77d247a4d86b --- /dev/null +++ b/js/src/jit-test/tests/arguments/testDelArg3Strict.js @@ -0,0 +1,44 @@ +"use strict"; + +function assertGood(x) { + assertEq(x, "good"); +} + +(function() { + var a = arguments; + return function() { + assertGood.apply(null, a); + } +})("good")(); + +(function() { + var a = arguments; + return function() { + a[0] = "good"; + assertGood.apply(null, a); + } +})("bad")(); + +Object.prototype[0] = "good"; + +(function() { + var a = arguments; + return function() { + delete a[0]; + assertGood.apply(null, a); + } +})("bad")(); + +delete Object.prototype[0]; + +function assertUndefined(x) { + assertEq(x, undefined); +} + +(function() { + var a = arguments; + return function() { + a[0] = "bad"; + assertUndefined.apply(null, a); + } +})()(); diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 7ca6192d5aa1..9734534c186f 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -400,6 +400,19 @@ GetElement(JSContext *cx, JSObject *obj, jsdouble index, JSBool *hole, Value *vp namespace js { +struct STATIC_SKIP_INFERENCE CopyNonHoleArgsTo +{ + CopyNonHoleArgsTo(JSObject *aobj, Value *dst) : aobj(aobj), dst(dst) {} + JSObject *aobj; + Value *dst; + bool operator()(uintN argi, Value *src) { + if (aobj->getArgsElement(argi).isMagic(JS_ARGS_HOLE)) + return false; + *dst++ = *src; + return true; + } +}; + bool GetElements(JSContext *cx, JSObject *aobj, jsuint length, Value *vp) { @@ -413,21 +426,28 @@ GetElements(JSContext *cx, JSObject *aobj, jsuint length, Value *vp) } else if (aobj->isArguments() && !aobj->isArgsLengthOverridden() && !js_PrototypeHasIndexedProperties(cx, aobj)) { /* - * Two cases, two loops: note how in the case of an active stack frame - * backing aobj, even though we copy from fp->argv, we still must check - * aobj->getArgsElement(i) for a hole, to handle a delete on the - * corresponding arguments element. See args_delProperty. + * If the argsobj is for an active call, then the elements are the + * live args on the stack. Otherwise, the elements are the args that + * were copied into the argsobj by PutActivationObjects when the + * function returned. In both cases, it is necessary to fall off the + * fast path for deleted properties (MagicValue(JS_ARGS_HOLE) since + * this requires general-purpose property lookup. */ if (JSStackFrame *fp = (JSStackFrame *) aobj->getPrivate()) { JS_ASSERT(fp->numActualArgs() <= JS_ARGS_LENGTH_MAX); - fp->forEachCanonicalActualArg(CopyNonHoleArgsTo(aobj, vp)); + if (!fp->forEachCanonicalActualArg(CopyNonHoleArgsTo(aobj, vp))) + goto found_deleted_prop; } else { Value *srcbeg = aobj->getArgsElements(); Value *srcend = srcbeg + length; - for (Value *dst = vp, *src = srcbeg; src < srcend; ++dst, ++src) - *dst = src->isMagic(JS_ARGS_HOLE) ? UndefinedValue() : *src; + for (Value *dst = vp, *src = srcbeg; src < srcend; ++dst, ++src) { + if (src->isMagic(JS_ARGS_HOLE)) + goto found_deleted_prop; + *dst = *src; + } } } else { + found_deleted_prop: for (uintN i = 0; i < length; i++) { if (!aobj->getProperty(cx, INT_TO_JSID(jsint(i)), &vp[i])) return JS_FALSE; diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 11348d0d1ab0..21fd2f44a44c 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -223,10 +223,11 @@ struct STATIC_SKIP_INFERENCE PutArg { PutArg(Value *dst) : dst(dst) {} Value *dst; - void operator()(uintN, Value *src) { + bool operator()(uintN, Value *src) { if (!dst->isMagic(JS_ARGS_HOLE)) *dst = *src; ++dst; + return true; } }; diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 27670d9dcd1a..a5b1d6c6f0d2 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -396,8 +396,13 @@ struct JSStackFrame inline js::Value *actualArgsEnd() const; inline js::Value &canonicalActualArg(uintN i) const; - template inline void forEachCanonicalActualArg(Op op); - template inline void forEachFormalArg(Op op); + + /* + * Apply 'op' to each arg of the specified type. Stop if 'op' returns + * false. Return 'true' iff all 'op' calls returned true. + */ + template inline bool forEachCanonicalActualArg(Op op); + template inline bool forEachFormalArg(Op op); inline void clearMissingArgs(); diff --git a/js/src/jsinterpinlines.h b/js/src/jsinterpinlines.h index 0fd3dfd6ea6d..ebb1285fbd7e 100644 --- a/js/src/jsinterpinlines.h +++ b/js/src/jsinterpinlines.h @@ -287,7 +287,7 @@ JSStackFrame::canonicalActualArg(uintN i) const } template -inline void +inline bool JSStackFrame::forEachCanonicalActualArg(Op op) { uintN nformal = fun()->nargs; @@ -296,53 +296,50 @@ JSStackFrame::forEachCanonicalActualArg(Op op) if (nactual <= nformal) { uintN i = 0; js::Value *actualsEnd = formals + nactual; - for (js::Value *p = formals; p != actualsEnd; ++p, ++i) - op(i, p); + for (js::Value *p = formals; p != actualsEnd; ++p, ++i) { + if (!op(i, p)) + return false; + } } else { uintN i = 0; js::Value *formalsEnd = formalArgsEnd(); - for (js::Value *p = formals; p != formalsEnd; ++p, ++i) - op(i, p); + for (js::Value *p = formals; p != formalsEnd; ++p, ++i) { + if (!op(i, p)) + return false; + } js::Value *actuals = formalsEnd - (nactual + 2); js::Value *actualsEnd = formals - 2; - for (js::Value *p = actuals; p != actualsEnd; ++p, ++i) - op(i, p); + for (js::Value *p = actuals; p != actualsEnd; ++p, ++i) { + if (!op(i, p)) + return false; + } } + return true; } template -inline void +inline bool JSStackFrame::forEachFormalArg(Op op) { js::Value *formals = formalArgsEnd() - fun()->nargs; js::Value *formalsEnd = formalArgsEnd(); uintN i = 0; - for (js::Value *p = formals; p != formalsEnd; ++p, ++i) - op(i, p); + for (js::Value *p = formals; p != formalsEnd; ++p, ++i) { + if (!op(i, p)) + return false; + } + return true; } namespace js { -struct STATIC_SKIP_INFERENCE CopyNonHoleArgsTo -{ - CopyNonHoleArgsTo(JSObject *aobj, Value *dst) : aobj(aobj), dst(dst) {} - JSObject *aobj; - Value *dst; - void operator()(uintN argi, Value *src) { - if (aobj->getArgsElement(argi).isMagic(JS_ARGS_HOLE)) - dst->setUndefined(); - else - *dst = *src; - ++dst; - } -}; - struct CopyTo { Value *dst; CopyTo(Value *dst) : dst(dst) {} - void operator()(uintN, Value *src) { + bool operator()(uintN, Value *src) { *dst++ = *src; + return true; } }; diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index fc65c16f9dd9..7efc8a10d742 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -10155,8 +10155,9 @@ class BoxArg : tr(tr), addr(addr) {} TraceRecorder *tr; Address addr; - void operator()(uintN argi, Value *src) { + bool operator()(uintN argi, Value *src) { tr->box_value_into(*src, tr->get(src), OffsetAddress(addr, argi * sizeof(Value))); + return true; } };