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

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

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___ */

js/src/jsinterp.cpp

@@ -2151,6 +2151,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
@@ -4765,26 +4789,13 @@ BEGIN_CASE(JSOP_SETCALL)
 }
 END_CASE(JSOP_SETCALL)
 
-#define SLOW_PUSH_THISV(cx, obj)                                            \
+#define SLOW_PUSH_THISV(cx, obj, funval)                                      \
     JS_BEGIN_MACRO                                                            \
-        Class *clasp;                                                       \
+        Value v;                                                              \
-        JSObject *thisp = obj;                                              \
+        if (!SlowThis(cx, obj, funval, &v))                                   \
-        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);                                            \
+        PUSH_COPY(v);                                                         \
-        }                                                                   \
+    JS_END_MACRO                                                              \
-    JS_END_MACRO
 
 BEGIN_CASE(JSOP_GETGNAME)
 BEGIN_CASE(JSOP_CALLGNAME)
@@ -4814,20 +4825,17 @@ BEGIN_CASE(JSOP_CALLNAME)
             PUSH_COPY(rval);
         }
 
-        /*
+        JS_ASSERT(obj->isGlobal() || IsCacheableNonGlobalScope(obj));
-         * Push results, the same as below, but with a prop$ hit there
+        if (op == JSOP_CALLNAME || op == JSOP_CALLGNAME) {
-         * is no need to test for the unusual and uncacheable case where
+            if (regs.sp[-1].isObject() &&
-         * the caller determines |this|.
+                !IsSafeForLazyThisCoercion(cx, &regs.sp[-1].toObject())) {
-         */
+                if (!(obj = obj->thisObject(cx)))
-#if DEBUG
+                    return false;
-        Class *clasp;
+                PUSH_OBJECT(*obj);
-        JS_ASSERT(!obj->getParent() ||
+            } else {
-                  (clasp = obj->getClass()) == &js_CallClass ||
-                  clasp == &js_BlockClass ||
-                  clasp == &js_DeclEnvClass);
-#endif
-        if (op == JSOP_CALLNAME || op == JSOP_CALLGNAME)
                 PUSH_UNDEFINED();
+            }
+        }
         len = JSOP_NAME_LENGTH;
         DO_NEXT_OP(len);
     }
@@ -4865,7 +4873,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)
 
@@ -6368,7 +6376,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)
 

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;

js/src/jsobj.cpp

@@ -4429,6 +4429,11 @@ JSObject::freeSlot(JSContext *cx, uint32 slot)
     return false;
 }
 
+namespace js {
+
+
+}
+
 /* JSBOXEDWORD_INT_MAX as a string */
 #define JSBOXEDWORD_INT_MAX_STRING "1073741823"
 
@@ -5036,7 +5041,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 =
@@ -5146,11 +5151,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;

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
+static 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,8 @@ js_IsCacheableNonGlobalScope(JSObject *obj)
     return cacheable;
 }
 
+}
+
 /*
  * If cacheResult is false, return JS_NO_PROP_CACHE_FILL on success.
  */

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"
 

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

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