diff --git a/js/src/jit-test/tests/basic/testBug634590.js b/js/src/jit-test/tests/basic/testBug634590.js new file mode 100644 index 00000000000..da47ed0090d --- /dev/null +++ b/js/src/jit-test/tests/basic/testBug634590.js @@ -0,0 +1,7 @@ +this.name = "outer"; +var sb = evalcx(''); +sb.name = "inner"; +sb.parent = this; +function f() { return this.name; } +assertEq(evalcx('this.f = parent.f; var s = ""; for (i = 0; i < 10; ++i) s += f(); s', sb), + "innerinnerinnerinnerinnerinnerinnerinnerinnerinner"); diff --git a/js/src/jsfuninlines.h b/js/src/jsfuninlines.h index 4cbb744b236..d951e93c462 100644 --- a/js/src/jsfuninlines.h +++ b/js/src/jsfuninlines.h @@ -49,4 +49,30 @@ JSFunction::inStrictMode() const return script()->strictModeCode; } +namespace js { + +static inline bool +IsSafeForLazyThisCoercion(JSContext *cx, JSObject *callee) +{ + /* + * Look past any wrappers. If the callee is a strict function it is always + * safe because we won't do 'this' coercion in strict mode. Otherwise the + * callee is only safe to transform into the lazy 'this' token (undefined) + * if it is in the current scope. Without this restriction, lazy 'this' + * coercion would pick up the wrong global in the other scope. + */ + if (callee->isProxy()) { + callee = callee->unwrap(); + if (!callee->isFunction()) + return true; // treat any non-wrapped-function proxy as strict + + JSFunction *fun = callee->getFunctionPrivate(); + if (fun->isInterpreted() && fun->inStrictMode()) + return true; + } + return callee->getGlobal() == cx->fp()->scopeChain().getGlobal(); +} + +} + #endif /* jsfuninlines_h___ */ diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index fde50aec9f1..eab8a4ef6bb 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -2178,6 +2178,30 @@ ScriptPrologue(JSContext *cx, JSStackFrame *fp) return true; } +static inline bool +SlowThis(JSContext *cx, JSObject *obj, const Value &funval, Value *vp) +{ + if (!funval.isObject() || + ((obj->isGlobal() || IsCacheableNonGlobalScope(obj)) && + IsSafeForLazyThisCoercion(cx, &funval.toObject()))) { + /* + * We can avoid computing 'this' eagerly and push the implicit 'this' + * value (undefined), as long the scope is cachable and we are not + * crossing into another scope (in which case lazy calculation of 'this' + * would pick up the new and incorrect scope). 'strict' functions are an + * exception. We don't want to eagerly calculate 'this' for them even if + * the callee is in a different scope. + */ + *vp = UndefinedValue(); + return true; + } + + if (!(obj = obj->thisObject(cx))) + return false; + *vp = ObjectValue(*obj); + return true; +} + namespace js { JS_REQUIRES_STACK JS_NEVER_INLINE bool @@ -2203,8 +2227,8 @@ Interpret(JSContext *cx, JSStackFrame *entryFrame, uintN inlineCallCount, JSInte * expect from looking at the code. (We do omit POPs after SETs; * unfortunate, but not worth fixing.) */ -# define LOG_OPCODE(OP) JS_BEGIN_MACRO \ - if (JS_UNLIKELY(cx->logfp != NULL) && \ +# define LOG_OPCODE(OP) JS_BEGIN_MACRO \ + if (JS_UNLIKELY(cx->logfp != NULL) && \ (OP) == *regs.pc) \ js_LogOpcode(cx); \ JS_END_MACRO @@ -4792,26 +4816,13 @@ BEGIN_CASE(JSOP_SETCALL) } END_CASE(JSOP_SETCALL) -#define SLOW_PUSH_THISV(cx, obj) \ - JS_BEGIN_MACRO \ - Class *clasp; \ - JSObject *thisp = obj; \ - if (!thisp->getParent() || \ - (clasp = thisp->getClass()) == &js_CallClass || \ - clasp == &js_BlockClass || \ - clasp == &js_DeclEnvClass) { \ - /* Push the ImplicitThisValue for the Environment Record */ \ - /* associated with obj. See ES5 sections 10.2.1.1.6 and */ \ - /* 10.2.1.2.6 (ImplicitThisValue) and section 11.2.3 */ \ - /* (Function Calls). */ \ - PUSH_UNDEFINED(); \ - } else { \ - thisp = thisp->thisObject(cx); \ - if (!thisp) \ - goto error; \ - PUSH_OBJECT(*thisp); \ - } \ - JS_END_MACRO +#define SLOW_PUSH_THISV(cx, obj, funval) \ + JS_BEGIN_MACRO \ + Value v; \ + if (!SlowThis(cx, obj, funval, &v)) \ + goto error; \ + PUSH_COPY(v); \ + JS_END_MACRO \ BEGIN_CASE(JSOP_GETGNAME) BEGIN_CASE(JSOP_CALLGNAME) @@ -4841,20 +4852,17 @@ BEGIN_CASE(JSOP_CALLNAME) PUSH_COPY(rval); } - /* - * Push results, the same as below, but with a prop$ hit there - * is no need to test for the unusual and uncacheable case where - * the caller determines |this|. - */ -#if DEBUG - Class *clasp; - JS_ASSERT(!obj->getParent() || - (clasp = obj->getClass()) == &js_CallClass || - clasp == &js_BlockClass || - clasp == &js_DeclEnvClass); -#endif - if (op == JSOP_CALLNAME || op == JSOP_CALLGNAME) - PUSH_UNDEFINED(); + JS_ASSERT(obj->isGlobal() || IsCacheableNonGlobalScope(obj)); + if (op == JSOP_CALLNAME || op == JSOP_CALLGNAME) { + if (regs.sp[-1].isObject() && + !IsSafeForLazyThisCoercion(cx, ®s.sp[-1].toObject())) { + if (!(obj = obj->thisObject(cx))) + return false; + PUSH_OBJECT(*obj); + } else { + PUSH_UNDEFINED(); + } + } len = JSOP_NAME_LENGTH; DO_NEXT_OP(len); } @@ -4892,7 +4900,7 @@ BEGIN_CASE(JSOP_CALLNAME) /* obj must be on the scope chain, thus not a function. */ if (op == JSOP_CALLNAME || op == JSOP_CALLGNAME) - SLOW_PUSH_THISV(cx, obj); + SLOW_PUSH_THISV(cx, obj, rval); } END_CASE(JSOP_NAME) @@ -6395,7 +6403,7 @@ BEGIN_CASE(JSOP_XMLNAME) goto error; regs.sp[-1] = rval; if (op == JSOP_CALLXMLNAME) - SLOW_PUSH_THISV(cx, obj); + SLOW_PUSH_THISV(cx, obj, rval); } END_CASE(JSOP_XMLNAME) diff --git a/js/src/jsinterpinlines.h b/js/src/jsinterpinlines.h index c55c868b3fe..c976cdea8da 100644 --- a/js/src/jsinterpinlines.h +++ b/js/src/jsinterpinlines.h @@ -479,7 +479,7 @@ JSStackFrame::callObj() const JS_ASSERT(hasCallObj()); JSObject *pobj = &scopeChain(); while (JS_UNLIKELY(pobj->getClass() != &js_CallClass)) { - JS_ASSERT(js_IsCacheableNonGlobalScope(pobj) || pobj->isWith()); + JS_ASSERT(js::IsCacheableNonGlobalScope(pobj) || pobj->isWith()); pobj = pobj->getParent(); } return *pobj; diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index b2925942c7c..173921e21ba 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -4445,6 +4445,11 @@ JSObject::freeSlot(JSContext *cx, uint32 slot) return false; } +namespace js { + + +} + /* JSBOXEDWORD_INT_MAX as a string */ #define JSBOXEDWORD_INT_MAX_STRING "1073741823" @@ -5052,7 +5057,7 @@ js_FindPropertyHelper(JSContext *cx, jsid id, JSBool cacheResult, parent = obj->getParent(); for (scopeIndex = 0; parent - ? js_IsCacheableNonGlobalScope(obj) + ? IsCacheableNonGlobalScope(obj) : !obj->getOps()->lookupProperty; ++scopeIndex) { protoIndex = @@ -5162,11 +5167,11 @@ js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id) * farther checks or lookups. For details see the JSOP_BINDNAME case of * js_Interpret. * - * The test order here matters because js_IsCacheableNonGlobalScope + * The test order here matters because IsCacheableNonGlobalScope * must not be passed a global object (i.e. one with null parent). */ for (int scopeIndex = 0; - !obj->getParent() || js_IsCacheableNonGlobalScope(obj); + !obj->getParent() || IsCacheableNonGlobalScope(obj); scopeIndex++) { JSObject *pobj; JSProperty *prop; diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 27d59fa0212..176a91182fb 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -1661,16 +1661,19 @@ js_LookupPropertyWithFlags(JSContext *cx, JSObject *obj, jsid id, uintN flags, JSObject **objp, JSProperty **propp); +extern JS_FRIEND_DATA(js::Class) js_CallClass; +extern JS_FRIEND_DATA(js::Class) js_DeclEnvClass; + +namespace js { + /* * We cache name lookup results only for the global object or for native * non-global objects without prototype or with prototype that never mutates, * see bug 462734 and bug 487039. */ -inline bool -js_IsCacheableNonGlobalScope(JSObject *obj) +static inline bool +IsCacheableNonGlobalScope(JSObject *obj) { - extern JS_FRIEND_DATA(js::Class) js_CallClass; - extern JS_FRIEND_DATA(js::Class) js_DeclEnvClass; JS_ASSERT(obj->getParent()); js::Class *clasp = obj->getClass(); @@ -1682,6 +1685,8 @@ js_IsCacheableNonGlobalScope(JSObject *obj) return cacheable; } +} + /* * If cacheResult is false, return JS_NO_PROP_CACHE_FILL on success. */ diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index a536724cbf2..d60557c24e5 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -49,6 +49,7 @@ #include "jsobj.h" #include "jsprobes.h" #include "jspropertytree.h" +#include "jsproxy.h" #include "jsscope.h" #include "jsstaticcheck.h" #include "jsxml.h" @@ -60,6 +61,7 @@ #include "jsscopeinlines.h" #include "jsstr.h" +#include "jsfuninlines.h" #include "jsgcinlines.h" #include "jsprobes.h" diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 79fddf09990..6d540c5e461 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -6595,7 +6595,7 @@ ScopeChainCheck(JSContext* cx, TreeFragment* f) */ JSObject* child = &cx->fp()->scopeChain(); while (JSObject* parent = child->getParent()) { - if (!js_IsCacheableNonGlobalScope(child)) { + if (!IsCacheableNonGlobalScope(child)) { debug_only_print0(LC_TMTracer,"Blacklist: non-cacheable object on scope chain.\n"); Blacklist((jsbytecode*) f->root->ip); return false; @@ -15183,7 +15183,7 @@ TraceRecorder::traverseScopeChain(JSObject *obj, LIns *obj_ins, JSObject *target /* There was a call object, or should be a call object now. */ for (;;) { if (obj != globalObj) { - if (!js_IsCacheableNonGlobalScope(obj)) + if (!IsCacheableNonGlobalScope(obj)) RETURN_STOP("scope chain lookup crosses non-cacheable object"); // We must guard on the shape of all call objects for heavyweight functions diff --git a/js/src/methodjit/PolyIC.cpp b/js/src/methodjit/PolyIC.cpp index a954a753681..aa82dabf6db 100644 --- a/js/src/methodjit/PolyIC.cpp +++ b/js/src/methodjit/PolyIC.cpp @@ -1187,7 +1187,7 @@ class ScopeNameCompiler : public PICStubCompiler JS_ASSERT_IF(pic.kind == ic::PICInfo::XNAME, getprop.obj == tobj); while (tobj && tobj != getprop.holder) { - if (!js_IsCacheableNonGlobalScope(tobj)) + if (!IsCacheableNonGlobalScope(tobj)) return disable("non-cacheable scope chain object"); JS_ASSERT(tobj->isNative()); @@ -1539,7 +1539,7 @@ class BindNameCompiler : public PICStubCompiler JSObject *tobj = scopeChain; Address parent(pic.objReg, offsetof(JSObject, parent)); while (tobj && tobj != obj) { - if (!js_IsCacheableNonGlobalScope(tobj)) + if (!IsCacheableNonGlobalScope(tobj)) return disable("non-cacheable obj in scope chain"); masm.loadPtr(parent, pic.objReg); Jump nullTest = masm.branchTestPtr(Assembler::Zero, pic.objReg, pic.objReg);