bug 462734 - fixing JSOP_BINDNAME caching issues. r=brendan

js/src/jsemit.h

@@ -261,7 +261,7 @@ struct JSTreeContext {              /* tree context for semantic checks */
 #define TCF_HAS_SHARPS        0x8000 /* source contains sharp defs or uses */
 
 /*
- * Sticky deoptimization flags to propagate from FunctionBody.
+ * Flags to propagate from FunctionBody.
  */
 #define TCF_FUN_FLAGS           (TCF_FUN_SETS_OUTER_NAME |                    \
                                  TCF_FUN_USES_ARGUMENTS  |                    \

js/src/jsinterp.cpp

@@ -3639,7 +3639,25 @@ js_Interpret(JSContext *cx)
             do {
                 JSPropCacheEntry *entry;
 
+                /*
+                 * We can skip the property lookup for the global object. If
+                 * the property does not exist anywhere on the scope chain,
+                 * JSOP_SETNAME adds the property to the global.
+                 *
+                 * As a consequence of this optimization for the global object
+                 * we run its JSRESOLVE_ASSIGNING-tolerant resolve hooks only
+                 * in JSOP_SETNAME, after the interpreter evaluates the right-
+                 * hand-side of the assignment, and not here.
+                 *
+                 * This should be transparent to the hooks because the script,
+                 * instead of name = rhs, could have used global.name = rhs
+                 * given a global object reference, which also calls the hooks
+                 * only after evaluating the rhs. We desire such resolve hook
+                 * equivalence between the two forms.
+                 */
                 obj = fp->scopeChain;
+                if (!OBJ_GET_PARENT(cx, obj))
+                    break;
                 if (JS_LIKELY(OBJ_IS_NATIVE(obj))) {
                     PROPERTY_CACHE_TEST(cx, regs.pc, obj, obj2, entry, atom);
                     if (!atom) {
@@ -3652,7 +3670,7 @@ js_Interpret(JSContext *cx)
                     LOAD_ATOM(0);
                 }
                 id = ATOM_TO_JSID(atom);
-                obj = js_FindIdentifierBase(cx, id, entry);
+                obj = js_FindIdentifierBase(cx, fp->scopeChain, id, entry);
                 if (!obj)
                     goto error;
             } while (0);
@@ -4758,8 +4776,10 @@ js_Interpret(JSContext *cx)
                     LOAD_ATOM(0);
                 id = ATOM_TO_JSID(atom);
                 if (entry) {
-                    if (!js_SetPropertyHelper(cx, obj, id, &rval, &entry))
+                    if (!js_SetPropertyHelper(cx, obj, id, op == JSOP_SETNAME,
+                                              &rval, &entry)) {
                         goto error;
+                    }
 #ifdef JS_TRACER
                     if (entry)
                         TRACE_1(SetPropMiss, entry);
@@ -6413,7 +6433,7 @@ js_Interpret(JSContext *cx)
                     goto error;
                 }
                 if (JS_UNLIKELY(atom == cx->runtime->atomState.protoAtom)
-                    ? !js_SetPropertyHelper(cx, obj, id, &rval, &entry)
+                    ? !js_SetPropertyHelper(cx, obj, id, false, &rval, &entry)
                     : !js_DefineNativeProperty(cx, obj, id, rval, NULL, NULL,
                                                JSPROP_ENUMERATE, 0, 0, NULL,
                                                &entry)) {

js/src/jsobj.cpp

@@ -4070,45 +4070,68 @@ js_FindProperty(JSContext *cx, jsid id, JSObject **objp, JSObject **pobjp,
 }
 
 JS_REQUIRES_STACK JSObject *
-js_FindIdentifierBase(JSContext *cx, jsid id, JSPropCacheEntry *entry)
+js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id,
+                      JSPropCacheEntry *entry)
 {
-    JSObject *obj, *pobj;
-    JSProperty *prop;
+    /*
+     * This function should not be called for a global object or from the
+     * trace and should have a valid cache entry for native scopeChain.
+     */
+    JSObject *parent = OBJ_GET_PARENT(cx, scopeChain);
+    JS_ASSERT(parent);
+    JS_ASSERT(!JS_ON_TRACE(cx));
+    JS_ASSERT_IF(OBJ_IS_NATIVE(scopeChain), entry);
 
     /*
-     * Look for id's property along the "with" statement chain and the
-     * statically-linked scope chain.
+     * Optimize and cache only for classes that do not have resolve hooks and
+     * where the prototype is used only to implement a shared scope, bug 462734
+     * and bug 487039.
      */
-    if (js_FindPropertyHelper(cx, id, &obj, &pobj, &prop, &entry) < 0)
-        return NULL;
-    if (prop) {
-        OBJ_DROP_PROPERTY(cx, pobj, prop);
-        return obj;
-    }
+    JSObject *obj = scopeChain;
+    for (int scopeIndex = 0; ; scopeIndex++) {
+        JSClass *clasp = OBJ_GET_CLASS(cx, obj);
+        if (clasp != &js_CallClass && clasp != &js_BlockClass)
+            break;
 
-    /*
-     * Use the top-level scope from the scope chain, which won't end in the
-     * same scope as cx->globalObject for cross-context function calls.
-     */
-    JS_ASSERT(obj);
-
-    /*
-     * Property not found.  Give a strict warning if binding an undeclared
-     * top-level variable.
-     */
-    if (JS_HAS_STRICT_OPTION(cx)) {
-        JSString *str = JSVAL_TO_STRING(ID_TO_VALUE(id));
-        const char *bytes = js_GetStringBytes(cx, str);
-
-        if (!bytes ||
-            !JS_ReportErrorFlagsAndNumber(cx,
-                                          JSREPORT_WARNING | JSREPORT_STRICT,
-                                          js_GetErrorMessage, NULL,
-                                          JSMSG_UNDECLARED_VAR, bytes)) {
+        JSObject *pobj;
+        JSProperty *prop;
+        int protoIndex = js_LookupPropertyWithFlags(cx, obj, id, 0,
+                                                    &pobj, &prop);
+        if (protoIndex < 0)
             return NULL;
+        if (prop) {
+            JS_ASSERT(OBJ_IS_NATIVE(pobj));
+            JS_ASSERT(OBJ_GET_CLASS(cx, pobj) == clasp);
+            js_FillPropertyCache(cx, scopeChain, OBJ_SHAPE(scopeChain),
+                                 scopeIndex, protoIndex, pobj,
+                                 (JSScopeProperty *) prop, &entry);
+            JS_UNLOCK_OBJ(cx, pobj);
+            return obj;
+        }
+
+        obj = parent;
+        parent = OBJ_GET_PARENT(cx, parent);
+        if (!parent) {
+            /*
+             * Here obj is the global one and we can skip any checks for it,
+             * see comments in the JSOP_BINDNAME case of js_Interpret.
+             */
+            return obj;
         }
     }
 
+    do {
+        JSObject *pobj;
+        JSProperty *prop;
+        if (!OBJ_LOOKUP_PROPERTY(cx, obj, id, &pobj, &prop))
+            return NULL;
+        if (prop) {
+            OBJ_DROP_PROPERTY(cx, pobj, prop);
+            break;
+        }
+        obj = parent;
+        parent = OBJ_GET_PARENT(cx, parent);
+    } while (parent);
     return obj;
 }
 
@@ -4337,8 +4360,8 @@ js_GetMethod(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
 }
 
 JSBool
-js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
-                     JSPropCacheEntry **entryp)
+js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id,
+                     JSBool unqualified, jsval *vp, JSPropCacheEntry **entryp)
 {
     uint32 shape;
     int protoIndex;
@@ -4368,10 +4391,28 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
                                             &pobj, &prop);
     if (protoIndex < 0)
         return JS_FALSE;
+    if (prop) {
+        if (!OBJ_IS_NATIVE(pobj)) {
+            OBJ_DROP_PROPERTY(cx, pobj, prop);
+            prop = NULL;
+        }
+    } else {
+        /* We should never add properties to lexical blocks.  */
+        JS_ASSERT(OBJ_GET_CLASS(cx, obj) != &js_BlockClass);
 
-    if (prop && !OBJ_IS_NATIVE(pobj)) {
-        OBJ_DROP_PROPERTY(cx, pobj, prop);
-        prop = NULL;
+        if (unqualified && !OBJ_GET_PARENT(cx, obj) &&
+            JS_HAS_STRICT_OPTION(cx)) {
+            JSString *str = JSVAL_TO_STRING(ID_TO_VALUE(id));
+            const char *bytes = js_GetStringBytes(cx, str);
+            if (!bytes ||
+                !JS_ReportErrorFlagsAndNumber(cx,
+                                              JSREPORT_WARNING |
+                                              JSREPORT_STRICT,
+                                              js_GetErrorMessage, NULL,
+                                              JSMSG_UNDECLARED_VAR, bytes)) {
+                return NULL;
+            }
+        }
     }
     sprop = (JSScopeProperty *) prop;
 
@@ -4487,9 +4528,6 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
     }
 
     if (!sprop) {
-        /* We should never add properties to lexical blocks.  */
-        JS_ASSERT(OBJ_GET_CLASS(cx, obj) != &js_BlockClass);
-
         /*
          * Purge the property cache of now-shadowed id in obj's scope chain.
          * Do this early, before locking obj to avoid nesting locks.
@@ -4549,7 +4587,7 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
 JSBool
 js_SetProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
 {
-    return js_SetPropertyHelper(cx, obj, id, vp, NULL);
+    return js_SetPropertyHelper(cx, obj, id, false, vp, NULL);
 }
 
 JSBool

js/src/jsobj.h

@@ -681,7 +681,8 @@ js_FindProperty(JSContext *cx, jsid id, JSObject **objp, JSObject **pobjp,
                 JSProperty **propp);
 
 extern JS_REQUIRES_STACK JSObject *
-js_FindIdentifierBase(JSContext *cx, jsid id, JSPropCacheEntry *entry);
+js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id,
+                      JSPropCacheEntry *entry);
 
 extern JSObject *
 js_FindVariableScope(JSContext *cx, JSFunction **funp);
@@ -711,8 +712,8 @@ js_GetMethod(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
              JSPropCacheEntry **entryp);
 
 extern JSBool
-js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
-                     JSPropCacheEntry **entryp);
+js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id,
+                     JSBool unqualified, jsval *vp, JSPropCacheEntry **entryp);
 
 extern JSBool
 js_SetProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp);

js/src/jsparse.cpp

@@ -6405,15 +6405,9 @@ GeneratorExpr(JSParseNode *pn, JSParseNode *kid, JSTreeContext *tc)
         if (!funbox)
             return NULL;
 
-        /*
-         * We assume conservatively that any deoptimization flag in tc->flags
-         * besides TCF_FUN_PARAM_ARGUMENTS can come from the kid. So we
-         * propagate these flags into genfn. For code simplicity we also do
-         * not detect if the flags were only set in the kid and could be
-         * removed from tc->flags.
-         */
-        gentc.flags |= TCF_FUN_IS_GENERATOR | TCF_GENEXP_LAMBDA |
-                       (tc->flags & (TCF_FUN_FLAGS & ~TCF_FUN_PARAM_ARGUMENTS));
+        gentc.flags |= TCF_FUN_IS_GENERATOR | TCF_GENEXP_LAMBDA;
+        if (tc->flags & TCF_HAS_SHARPS)
+            gentc.flags |= TCF_HAS_SHARPS;
         funbox->tcflags |= gentc.flags;
         genfn->pn_funbox = funbox;
         genfn->pn_blockid = gentc.bodyid;