diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index c4a13790aa1..801f927dea0 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -2057,7 +2057,10 @@ JS_PUBLIC_API(jsval) JS_ComputeThis(JSContext *cx, jsval *vp) { assertSameCompartment(cx, JSValueArray(vp, 2)); - return BoxThisForVp(cx, Valueify(vp)) ? vp[1] : JSVAL_NULL; + CallReceiver call = CallReceiverFromVp(Valueify(vp)); + if (!BoxNonStrictThis(cx, call)) + return JSVAL_NULL; + return Jsvalify(call.thisv()); } JS_PUBLIC_API(void *) diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 0ff3e7c3169..40c90902507 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -1475,7 +1475,7 @@ JS_GetFrameThis(JSContext *cx, JSStackFrame *fp, jsval *thisv) if (!ac.enter()) return false; - if (!fp->computeThis(cx)) + if (!ComputeThis(cx, fp)) return false; *thisv = Jsvalify(fp->thisValue()); return true; diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 30102a30472..cf1673f65cb 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -410,31 +410,6 @@ CallThisObjectHook(JSContext *cx, JSObject *obj, Value *argv) return thisp; } -/* - * ECMA requires "the global object", but in embeddings such as the browser, - * which have multiple top-level objects (windows, frames, etc. in the DOM), - * we prefer fun's parent. An example that causes this code to run: - * - * // in window w1 - * function f() { return this } - * function g() { return f } - * - * // in window w2 - * var h = w1.g() - * alert(h() == w1) - * - * The alert should display "true". - */ -JS_STATIC_INTERPRET bool -ComputeGlobalThis(JSContext *cx, Value *vp) -{ - JSObject *thisp = vp[0].toObject().getGlobal()->thisObject(cx); - if (!thisp) - return false; - vp[1].setObject(*thisp); - return true; -} - namespace js { void @@ -478,24 +453,46 @@ ReportIncompatibleMethod(JSContext *cx, Value *vp, Class *clasp) } } +/* + * ECMA requires "the global object", but in embeddings such as the browser, + * which have multiple top-level objects (windows, frames, etc. in the DOM), + * we prefer fun's parent. An example that causes this code to run: + * + * // in window w1 + * function f() { return this } + * function g() { return f } + * + * // in window w2 + * var h = w1.g() + * alert(h() == w1) + * + * The alert should display "true". + */ bool -BoxThisForVp(JSContext *cx, Value *vp) +BoxNonStrictThis(JSContext *cx, const CallReceiver &call) { /* * Check for SynthesizeFrame poisoning and fast constructors which - * didn't check their vp properly. + * didn't check their callee properly. */ - JS_ASSERT(!vp[1].isMagic()); + Value &thisv = call.thisv(); + JS_ASSERT(!thisv.isMagic()); + #ifdef DEBUG - JSFunction *fun = vp[0].toObject().isFunction() ? vp[0].toObject().getFunctionPrivate() : NULL; + JSFunction *fun = call.callee().isFunction() ? call.callee().getFunctionPrivate() : NULL; JS_ASSERT_IF(fun && fun->isInterpreted(), !fun->inStrictMode()); #endif - if (vp[1].isNullOrUndefined()) - return ComputeGlobalThis(cx, vp); + if (thisv.isNullOrUndefined()) { + JSObject *thisp = call.callee().getGlobal()->thisObject(cx); + if (!thisp) + return false; + call.thisv().setObject(*thisp); + return true; + } - if (!vp[1].isObject()) - return !!js_PrimitiveToObject(cx, &vp[1]); + if (!thisv.isObject()) + return !!js_PrimitiveToObject(cx, &thisv); return true; } @@ -4037,14 +4034,14 @@ BEGIN_CASE(JSOP_LOCALINC) } BEGIN_CASE(JSOP_THIS) - if (!regs.fp->computeThis(cx)) + if (!ComputeThis(cx, regs.fp)) goto error; PUSH_COPY(regs.fp->thisValue()); END_CASE(JSOP_THIS) BEGIN_CASE(JSOP_UNBRANDTHIS) { - if (!regs.fp->computeThis(cx)) + if (!ComputeThis(cx, regs.fp)) goto error; Value &thisv = regs.fp->thisValue(); if (thisv.isObject()) { @@ -4061,7 +4058,7 @@ END_CASE(JSOP_UNBRANDTHIS) jsint i; BEGIN_CASE(JSOP_GETTHISPROP) - if (!regs.fp->computeThis(cx)) + if (!ComputeThis(cx, regs.fp)) goto error; i = 0; PUSH_COPY(regs.fp->thisValue()); diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index d26f8dfed4e..60ddb8be9e6 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -458,8 +458,6 @@ struct JSStackFrame return formalArgs()[-1]; } - inline bool computeThis(JSContext *cx); - /* * Callee * @@ -483,6 +481,10 @@ struct JSStackFrame return isFunctionFrame() ? &callee() : NULL; } + js::CallReceiver callReceiver() const { + return js::CallReceiverFromArgv(formalArgs()); + } + /* * getValidCalleeObject is a fallible getter to compute the correct callee * function object, which may require deferred cloning due to the JSObject @@ -923,13 +925,22 @@ extern bool ScriptDebugEpilogue(JSContext *cx, JSStackFrame *fp, bool ok); /* - * For a call's vp (which necessarily includes callee at vp[0] and the original - * specified |this| at vp[1]), convert null/undefined |this| into the global - * object for the callee and replace other primitives with boxed versions. The - * callee must not be strict mode code. + * For a given |call|, convert null/undefined |this| into the global object for + * the callee and replace other primitives with boxed versions. This assumes + * that call.callee() is not strict mode code. This is the special/slow case of + * ComputeThis. */ extern bool -BoxThisForVp(JSContext *cx, js::Value *vp); +BoxNonStrictThis(JSContext *cx, const CallReceiver &call); + +/* + * Ensure that fp->thisValue() is the correct value of |this| for the scripted + * call represented by |fp|. ComputeThis is necessary because fp->thisValue() + * may be set to 'undefined' when 'this' should really be the global object (as + * an optimization to avoid global-this computation). + */ +inline bool +ComputeThis(JSContext *cx, JSStackFrame *fp); /* * The js::InvokeArgumentsGuard passed to js_Invoke must come from an diff --git a/js/src/jsinterpinlines.h b/js/src/jsinterpinlines.h index 346f8d6dca4..2b735c03157 100644 --- a/js/src/jsinterpinlines.h +++ b/js/src/jsinterpinlines.h @@ -352,29 +352,6 @@ JSStackFrame::clearMissingArgs() SetValueRangeToUndefined(formalArgs() + numActualArgs(), formalArgsEnd()); } -inline bool -JSStackFrame::computeThis(JSContext *cx) -{ - js::Value &thisv = thisValue(); - if (thisv.isObject()) - return true; - if (isFunctionFrame()) { - if (fun()->inStrictMode()) - return true; - /* - * Eval function frames have their own |this| slot, which is a copy of the function's - * |this| slot. If we lazily wrap a primitive |this| in an eval function frame, the - * eval's frame will get the wrapper, but the function's frame will not. To prevent - * this, we always wrap a function's |this| before pushing an eval frame, and should - * thus never see an unwrapped primitive in a non-strict eval function frame. - */ - JS_ASSERT(!isEvalFrame()); - } - if (!js::BoxThisForVp(cx, &thisv - 1)) - return false; - return true; -} - inline JSObject & JSStackFrame::varobj(js::StackSegment *seg) const { @@ -562,6 +539,9 @@ InvokeSessionGuard::invoke(JSContext *cx) const formals_[-2] = savedCallee_; formals_[-1] = savedThis_; + /* Prevent spurious accessing-callee-after-rval assert. */ + args_.calleeHasBeenReset(); + #ifdef JS_METHODJIT void *code; if (!optimized() || !(code = script_->getJIT(false /* !constructing */)->invokeEntry)) @@ -625,6 +605,27 @@ class PrimitiveBehavior { } // namespace detail +template +bool +GetPrimitiveThis(JSContext *cx, Value *vp, T *v) +{ + typedef detail::PrimitiveBehavior Behavior; + + const Value &thisv = vp[1]; + if (Behavior::isType(thisv)) { + *v = Behavior::extract(thisv); + return true; + } + + if (thisv.isObject() && thisv.toObject().getClass() == Behavior::getClass()) { + *v = Behavior::extract(thisv.toObject().getPrimitiveThis()); + return true; + } + + ReportIncompatibleMethod(cx, vp, Behavior::getClass()); + return false; +} + /* * Compute the implicit |this| parameter for a call expression where the callee * is an unqualified name reference. @@ -702,25 +703,25 @@ ComputeImplicitThis(JSContext *cx, JSObject *obj, const Value &funval, Value *vp return true; } -template -bool -GetPrimitiveThis(JSContext *cx, Value *vp, T *v) +inline bool +ComputeThis(JSContext *cx, JSStackFrame *fp) { - typedef detail::PrimitiveBehavior Behavior; - - const Value &thisv = vp[1]; - if (Behavior::isType(thisv)) { - *v = Behavior::extract(thisv); + Value &thisv = fp->thisValue(); + if (thisv.isObject()) return true; + if (fp->isFunctionFrame()) { + if (fp->fun()->inStrictMode()) + return true; + /* + * Eval function frames have their own |this| slot, which is a copy of the function's + * |this| slot. If we lazily wrap a primitive |this| in an eval function frame, the + * eval's frame will get the wrapper, but the function's frame will not. To prevent + * this, we always wrap a function's |this| before pushing an eval frame, and should + * thus never see an unwrapped primitive in a non-strict eval function frame. + */ + JS_ASSERT(!fp->isEvalFrame()); } - - if (thisv.isObject() && thisv.toObject().getClass() == Behavior::getClass()) { - *v = Behavior::extract(thisv.toObject().getPrimitiveThis()); - return true; - } - - ReportIncompatibleMethod(cx, vp, Behavior::getClass()); - return false; + return BoxNonStrictThis(cx, fp->callReceiver()); } /* diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 95dbcfacfbf..493693a5452 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -1216,7 +1216,7 @@ EvalKernel(JSContext *cx, const CallArgs &call, EvalType evalType, JSStackFrame * haven't wrapped that yet, do so now, before we make a copy of it for * the eval code to use. */ - if (evalType == DIRECT_EVAL && !caller->computeThis(cx)) + if (evalType == DIRECT_EVAL && !ComputeThis(cx, caller)) return false; EvalScriptGuard esg(cx, linearStr); @@ -1623,20 +1623,21 @@ const char js_lookupSetter_str[] = "__lookupSetter__"; JS_FRIEND_API(JSBool) js_obj_defineGetter(JSContext *cx, uintN argc, Value *vp) { - if (!BoxThisForVp(cx, vp)) + CallArgs call = CallArgsFromVp(argc, vp); + if (!BoxNonStrictThis(cx, call)) return false; - JSObject *obj = &vp[1].toObject(); + JSObject *obj = &call.thisv().toObject(); - if (argc <= 1 || !js_IsCallable(vp[3])) { + if (argc <= 1 || !js_IsCallable(call[1])) { JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_GETTER_OR_SETTER, js_getter_str); return JS_FALSE; } - PropertyOp getter = CastAsPropertyOp(&vp[3].toObject()); + PropertyOp getter = CastAsPropertyOp(&call[1].toObject()); jsid id; - if (!ValueToId(cx, vp[2], &id)) + if (!ValueToId(cx, call[0], &id)) return JS_FALSE; if (!CheckRedeclaration(cx, obj, id, JSPROP_GETTER)) return JS_FALSE; @@ -1648,7 +1649,7 @@ js_obj_defineGetter(JSContext *cx, uintN argc, Value *vp) uintN attrs; if (!CheckAccess(cx, obj, id, JSACC_WATCH, &junk, &attrs)) return JS_FALSE; - vp->setUndefined(); + call.rval().setUndefined(); return obj->defineProperty(cx, id, UndefinedValue(), getter, StrictPropertyStub, JSPROP_ENUMERATE | JSPROP_GETTER | JSPROP_SHARED); } @@ -1656,20 +1657,21 @@ js_obj_defineGetter(JSContext *cx, uintN argc, Value *vp) JS_FRIEND_API(JSBool) js_obj_defineSetter(JSContext *cx, uintN argc, Value *vp) { - if (!BoxThisForVp(cx, vp)) + CallArgs call = CallArgsFromVp(argc, vp); + if (!BoxNonStrictThis(cx, call)) return false; - JSObject *obj = &vp[1].toObject(); + JSObject *obj = &call.thisv().toObject(); - if (argc <= 1 || !js_IsCallable(vp[3])) { + if (argc <= 1 || !js_IsCallable(call[1])) { JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_GETTER_OR_SETTER, js_setter_str); return JS_FALSE; } - StrictPropertyOp setter = CastAsStrictPropertyOp(&vp[3].toObject()); + StrictPropertyOp setter = CastAsStrictPropertyOp(&call[1].toObject()); jsid id; - if (!ValueToId(cx, vp[2], &id)) + if (!ValueToId(cx, call[0], &id)) return JS_FALSE; if (!CheckRedeclaration(cx, obj, id, JSPROP_SETTER)) return JS_FALSE; @@ -1681,7 +1683,7 @@ js_obj_defineSetter(JSContext *cx, uintN argc, Value *vp) uintN attrs; if (!CheckAccess(cx, obj, id, JSACC_WATCH, &junk, &attrs)) return JS_FALSE; - vp->setUndefined(); + call.rval().setUndefined(); return obj->defineProperty(cx, id, UndefinedValue(), PropertyStub, setter, JSPROP_ENUMERATE | JSPROP_SETTER | JSPROP_SHARED); } diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index cf374937e48..4f8e6b5d4cb 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -9999,7 +9999,7 @@ TraceRecorder::getThis(LIns*& this_ins) * trace-constant. getThisObject writes back to fp->thisValue(), so do * the same on trace. */ - if (!fp->computeThis(cx)) + if (!ComputeThis(cx, fp)) RETURN_ERROR("computeThis failed"); /* thisv is a reference, so it'll see the newly computed |this|. */ diff --git a/js/src/jsvalue.h b/js/src/jsvalue.h index 0a5a2585bf3..4b990fce481 100644 --- a/js/src/jsvalue.h +++ b/js/src/jsvalue.h @@ -1241,28 +1241,74 @@ Debug_SetValueRangeToCrashOnTouch(Value *vec, size_t len) #endif } +/* + * Abstracts the layout of the (callee,this) receiver pair that is passed to + * natives and scripted functions. + */ +class CallReceiver +{ +#ifdef DEBUG + mutable bool usedRval_; +#endif + protected: + Value *argv_; + CallReceiver() {} + CallReceiver(Value *argv) : argv_(argv) { +#ifdef DEBUG + usedRval_ = false; +#endif + } + + public: + friend CallReceiver CallReceiverFromVp(Value *); + friend CallReceiver CallReceiverFromArgv(Value *); + Value *base() const { return argv_ - 2; } + JSObject &callee() const { JS_ASSERT(!usedRval_); return argv_[-2].toObject(); } + Value &calleev() const { JS_ASSERT(!usedRval_); return argv_[-2]; } + Value &thisv() const { return argv_[-1]; } + + Value &rval() const { +#ifdef DEBUG + usedRval_ = true; +#endif + return argv_[-2]; + } + + void calleeHasBeenReset() const { +#ifdef DEBUG + usedRval_ = false; +#endif + } +}; + +JS_ALWAYS_INLINE CallReceiver +CallReceiverFromVp(Value *vp) +{ + return CallReceiver(vp + 2); +} + +JS_ALWAYS_INLINE CallReceiver +CallReceiverFromArgv(Value *argv) +{ + return CallReceiver(argv); +} + /* * Abstracts the layout of the stack passed to natives from the engine and from * natives to js::Invoke. */ -class CallArgs +class CallArgs : public CallReceiver { uintN argc_; - Value *argv_; protected: CallArgs() {} - CallArgs(uintN argc, Value *argv) : argc_(argc), argv_(argv) {} + CallArgs(uintN argc, Value *argv) : CallReceiver(argv), argc_(argc) {} + public: friend CallArgs CallArgsFromVp(uintN, Value *); friend CallArgs CallArgsFromArgv(uintN, Value *); - public: - Value *base() const { return argv_ - 2; } - JSObject &callee() const { return argv_[-2].toObject(); } - Value &calleev() const { return argv_[-2]; } - Value &thisv() const { return argv_[-1]; } Value &operator[](unsigned i) const { JS_ASSERT(i < argc_); return argv_[i]; } Value *argv() const { return argv_; } uintN argc() const { return argc_; } - Value &rval() const { return argv_[-2]; } }; JS_ALWAYS_INLINE CallArgs diff --git a/js/src/methodjit/StubCalls.cpp b/js/src/methodjit/StubCalls.cpp index cf342d00714..f75be169f6a 100644 --- a/js/src/methodjit/StubCalls.cpp +++ b/js/src/methodjit/StubCalls.cpp @@ -1247,7 +1247,7 @@ stubs::Trap(VMFrame &f, uint32 trapTypes) void JS_FASTCALL stubs::This(VMFrame &f) { - if (!f.fp()->computeThis(f.cx)) + if (!ComputeThis(f.cx, f.fp())) THROW(); f.regs.sp[-1] = f.fp()->thisValue(); }