Unqualified function invocation doesn't use the global object the property was gotten from as |this| (bug 634590, r=brendan).

This commit is contained in:
Andreas Gal 2011-02-17 17:52:55 -08:00
parent 66d95c44a9
commit 2e75eef92d
8 changed files with 104 additions and 35 deletions

View File

@ -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");

View File

@ -2178,6 +2178,29 @@ 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)) && IsCacheableCallee(cx, funval)) {
/*
* 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 +2226,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 +4815,28 @@ 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
/*
* Eager 'this' coercion slow path. If we end up calculating 'this' eagerly,
* purge the property cache since we don't perform eager 'this' coercion in
* the property cache hit fast path.
*/
#define SLOW_PUSH_THISV(cx, obj, funval) \
JS_BEGIN_MACRO \
Value v; \
if (!SlowThis(cx, obj, funval, &v)) \
goto error; \
if (!v.isUndefined()) { \
PropertyCacheEntry *entry; \
JSObject *obj2; \
JSAtom *atom; \
JS_PROPERTY_CACHE(cx).test(cx, regs.pc, obj, obj2, entry, atom); \
if (!atom) { \
ASSERT_VALID_PROPERTY_CACHE_HIT(0, obj, obj2, entry); \
memset(entry, 0, sizeof(*entry)); \
} \
} \
PUSH_COPY(v); \
JS_END_MACRO \
BEGIN_CASE(JSOP_GETGNAME)
BEGIN_CASE(JSOP_CALLGNAME)
@ -4892,7 +4917,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 +6420,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)

View File

@ -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;

View File

@ -4445,6 +4445,31 @@ JSObject::freeSlot(JSContext *cx, uint32 slot)
return false;
}
namespace js {
bool
IsCacheableCallee(JSContext *cx, const Value &funval)
{
JS_ASSERT(funval.isObject());
/*
* Look past any wrappers. If the callee is a strict function it is always
* cacheable because we won't do 'this' coercion in strict mode. Otherwise
* the callee is only cacheable if it is in the current scope. Without this
* restriction lazy 'this' computation would pick up the wrong global in
* the other scope.
*/
JSObject *funobj = funval.toObject().unwrap();
if (funobj->isFunction()) {
JSFunction *fun = funobj->getFunctionPrivate();
if (fun->isInterpreted() && fun->inStrictMode())
return true;
}
return funobj->getGlobal() == cx->fp()->scopeChain().getGlobal();
}
}
/* JSBOXEDWORD_INT_MAX as a string */
#define JSBOXEDWORD_INT_MAX_STRING "1073741823"
@ -5052,7 +5077,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 +5187,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;

View File

@ -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)
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,11 @@ js_IsCacheableNonGlobalScope(JSObject *obj)
return cacheable;
}
bool
IsCacheableCallee(JSContext *cx, const Value &funval);
}
/*
* If cacheResult is false, return JS_NO_PROP_CACHE_FILL on success.
*/

View File

@ -155,6 +155,8 @@ PropertyCache::fill(JSContext *cx, JSObject *obj, uintN scopeIndex, uintN protoI
JS_ASSERT(pobj->hasMethodBarrier());
JSObject &funobj = shape->methodObject();
JS_ASSERT(&funobj == &pobj->nativeGetSlot(shape->slot).toObject());
if (!IsCacheableCallee(cx, ObjectValue(funobj)))
return JS_NO_PROP_CACHE_FILL;
vword.setFunObj(funobj);
break;
}
@ -191,6 +193,8 @@ PropertyCache::fill(JSContext *cx, JSObject *obj, uintN scopeIndex, uintN protoI
if (!pobj->brand(cx))
return JS_NO_PROP_CACHE_FILL;
}
if (!IsCacheableCallee(cx, v))
return JS_NO_PROP_CACHE_FILL;
vword.setFunObj(*funobj);
break;
}

View File

@ -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;
@ -15189,7 +15189,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

View File

@ -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);