[INFER] Never do a lookup on the current scope chain for GNAME opcodes, bug 647695.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 08 Jun 2011 09:20:01 -0700
changeset 75153 a53db4f2d235f538b283d85bfba2163816e13e30
parent 75152 afe33041f4819e18a263ce778ceafed1dd262644
child 75154 73d2e2357b5be8928900f363bf6c74631a0789c6
child 75155 94ae102189f061a9afb553363b7414c54351b845
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs647695
milestone7.0a1
[INFER] Never do a lookup on the current scope chain for GNAME opcodes, bug 647695.
js/src/jit-test/tests/basic/bug647695.js
js/src/jit-test/tests/jaeger/bug563000/eif-call-newvar.js
js/src/jit-test/tests/jaeger/bug563000/eif-getter-newvar.js
js/src/jit-test/tests/jaeger/bug563000/eif-trap-newvar.js
js/src/jsinterp.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jstracer.cpp
js/src/methodjit/PolyIC.cpp
js/src/methodjit/StubCalls.cpp
js/src/shell/js.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug647695.js
@@ -0,0 +1,15 @@
+try { let(x = XMLList(7), y = let(a = x)("")) {} } catch (e) {}
+
+try {
+with({
+    y: Float64Array
+})
+for each(let y in [y]) {}
+} catch (e) {}
+
+try { test(); } catch (e) {}
+function test() {
+    try {
+        var result, arguments = 'adddb', arguments;
+    } catch (ex) {}
+}
--- a/js/src/jit-test/tests/jaeger/bug563000/eif-call-newvar.js
+++ b/js/src/jit-test/tests/jaeger/bug563000/eif-call-newvar.js
@@ -1,14 +1,14 @@
 // |jit-test| mjitalways;debug
 setDebug(true);
 
 function callee() {
   assertJit();
   evalInFrame(1, "var x = 'success'");
 }
 function caller() {
-  assertJit();
+  eval();
   callee();
   return x;
 }
 assertEq(caller(), "success");
 assertEq(typeof x, "undefined");
--- a/js/src/jit-test/tests/jaeger/bug563000/eif-getter-newvar.js
+++ b/js/src/jit-test/tests/jaeger/bug563000/eif-getter-newvar.js
@@ -1,10 +1,10 @@
 // |jit-test| mjitalways;debug
 setDebug(true);
 
 this.__defineGetter__("someProperty", function () { evalInFrame(1, "var x = 'success'"); });
 function caller(obj) {
-  assertJit();
+  eval();
   obj.someProperty;
   return x;
 }
 assertEq(caller(this), "success");
--- a/js/src/jit-test/tests/jaeger/bug563000/eif-trap-newvar.js
+++ b/js/src/jit-test/tests/jaeger/bug563000/eif-trap-newvar.js
@@ -1,10 +1,10 @@
 // |jit-test| mjitalways;debug
 setDebug(true);
 
 function nop(){}
 function caller(obj) {
-  assertJit();
+  eval();
   return x;
 }
-trap(caller, 9, "var x = 'success'; nop()");
+trap(caller, 13, "var x = 'success'; nop()");
 assertEq(caller(this), "success");
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -2042,17 +2042,17 @@ AssertValidPropertyCacheHit(JSContext *c
     else
         atom = cx->runtime->atomState.lengthAtom;
 
     JSObject *obj, *pobj;
     JSProperty *prop;
     JSBool ok;
 
     if (JOF_OPMODE(*regs.pc) == JOF_NAME) {
-        ok = js_FindProperty(cx, ATOM_TO_JSID(atom), &obj, &pobj, &prop);
+        ok = js_FindProperty(cx, ATOM_TO_JSID(atom), false, &obj, &pobj, &prop);
     } else {
         obj = start;
         ok = js_LookupProperty(cx, obj, ATOM_TO_JSID(atom), &pobj, &prop);
     }
     if (!ok)
         return false;
     if (cx->runtime->gcNumber != sample)
         JS_PROPERTY_CACHE(cx).restore(&savedEntry);
@@ -3161,17 +3161,17 @@ BEGIN_CASE(JSOP_FORNAME)
 BEGIN_CASE(JSOP_FORGNAME)
 {
     JS_ASSERT(regs.sp - 1 >= regs.fp()->base());
     JSAtom *atom;
     LOAD_ATOM(0, atom);
     jsid id = ATOM_TO_JSID(atom);
     JSObject *obj, *obj2;
     JSProperty *prop;
-    if (!js_FindProperty(cx, id, &obj, &obj2, &prop))
+    if (!js_FindProperty(cx, id, op == JSOP_FORGNAME, &obj, &obj2, &prop))
         goto error;
 
     {
         AutoValueRooter tvr(cx);
         JS_ASSERT(regs.sp[-1].isObject());
         if (!IteratorNext(cx, &regs.sp[-1].toObject(), tvr.addr()))
             goto error;
         if (!obj->setProperty(cx, id, tvr.addr(), script->strictModeCode))
@@ -3801,17 +3801,17 @@ END_CASE(JSOP_POS)
 
 BEGIN_CASE(JSOP_DELNAME)
 {
     JSAtom *atom;
     LOAD_ATOM(0, atom);
     jsid id = ATOM_TO_JSID(atom);
     JSObject *obj, *obj2;
     JSProperty *prop;
-    if (!js_FindProperty(cx, id, &obj, &obj2, &prop))
+    if (!js_FindProperty(cx, id, false, &obj, &obj2, &prop))
         goto error;
 
     /* Strict mode code should never contain JSOP_DELNAME opcodes. */
     JS_ASSERT(!script->strictModeCode);
 
     /* ECMA says to return true if name is undefined or inherited. */
     PUSH_BOOLEAN(true);
     if (prop) {
@@ -3911,17 +3911,19 @@ BEGIN_CASE(JSOP_DECNAME)
 BEGIN_CASE(JSOP_NAMEINC)
 BEGIN_CASE(JSOP_NAMEDEC)
 BEGIN_CASE(JSOP_INCGNAME)
 BEGIN_CASE(JSOP_DECGNAME)
 BEGIN_CASE(JSOP_GNAMEINC)
 BEGIN_CASE(JSOP_GNAMEDEC)
 {
     obj = &regs.fp()->scopeChain();
-    if (js_CodeSpec[op].format & JOF_GNAME)
+
+    bool global = (js_CodeSpec[op].format & JOF_GNAME);
+    if (global)
         obj = obj->getGlobal();
 
     JSObject *obj2;
     PropertyCacheEntry *entry;
     JS_PROPERTY_CACHE(cx).test(cx, regs.pc, obj, obj2, entry, atom);
     if (!atom) {
         ASSERT_VALID_PROPERTY_CACHE_HIT(0, obj, obj2, entry);
         if (obj == obj2 && entry->vword.isSlot()) {
@@ -3938,17 +3940,17 @@ BEGIN_CASE(JSOP_GNAMEDEC)
                 DO_NEXT_OP(len);
             }
         }
         LOAD_ATOM(0, atom);
     }
 
     id = ATOM_TO_JSID(atom);
     JSProperty *prop;
-    if (!js_FindPropertyHelper(cx, id, true, &obj, &obj2, &prop))
+    if (!js_FindPropertyHelper(cx, id, true, global, &obj, &obj2, &prop))
         goto error;
     if (!prop) {
         atomNotDefined = atom;
         goto atom_not_defined;
     }
 }
 
 do_incop:
@@ -4808,16 +4810,20 @@ END_CASE(JSOP_SETCALL)
 
 BEGIN_CASE(JSOP_GETGNAME)
 BEGIN_CASE(JSOP_CALLGNAME)
 BEGIN_CASE(JSOP_NAME)
 BEGIN_CASE(JSOP_CALLNAME)
 {
     JSObject *obj = &regs.fp()->scopeChain();
 
+    bool global = js_CodeSpec[op].format & JOF_GNAME;
+    if (global)
+        obj = obj->getGlobal();
+
     const Shape *shape;
     Value rval;
 
     PropertyCacheEntry *entry;
     JSObject *obj2;
     JSAtom *atom;
     JS_PROPERTY_CACHE(cx).test(cx, regs.pc, obj, obj2, entry, atom);
     if (!atom) {
@@ -4838,20 +4844,19 @@ BEGIN_CASE(JSOP_CALLNAME)
 
         JS_ASSERT(obj->isGlobal() || IsCacheableNonGlobalScope(obj));
         if (op == JSOP_CALLNAME || op == JSOP_CALLGNAME)
             PUSH_IMPLICIT_THIS(cx, obj, regs.sp[-1]);
         len = JSOP_NAME_LENGTH;
         DO_NEXT_OP(len);
     }
 
-    jsid id;
-    id = ATOM_TO_JSID(atom);
+    jsid id = ATOM_TO_JSID(atom);
     JSProperty *prop;
-    if (!js_FindPropertyHelper(cx, id, true, &obj, &obj2, &prop))
+    if (!js_FindPropertyHelper(cx, id, true, global, &obj, &obj2, &prop))
         goto error;
     if (!prop) {
         /* Kludge to allow (typeof foo == "undefined") tests. */
         JSOp op2 = js_GetOpcode(cx, script, regs.pc + JSOP_NAME_LENGTH);
         if (op2 == JSOP_TYPEOF) {
             PUSH_UNDEFINED();
             script->types.monitor(cx, regs.pc, regs.sp[-1]);
             len = JSOP_NAME_LENGTH;
@@ -5280,17 +5285,17 @@ BEGIN_CASE(JSOP_CALLUPVAR_DBG)
         AutoLocalNameArray names(cx, fun);
         if (!names)
             goto error;
 
         uintN index = fun->script()->bindings.countArgsAndVars() + GET_UINT16(regs.pc);
         atom = JS_LOCAL_NAME_TO_ATOM(names[index]);
         id = ATOM_TO_JSID(atom);
 
-        if (!js_FindProperty(cx, id, &obj, &obj2, &prop))
+        if (!js_FindProperty(cx, id, false, &obj, &obj2, &prop))
             goto error;
     }
 
     if (!prop) {
         atomNotDefined = atom;
         goto atom_not_defined;
     }
 
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -5440,27 +5440,40 @@ LookupPropertyWithFlags(JSContext *cx, J
     id = js_CheckForStringIndex(id);
 
     return LookupPropertyWithFlagsInline(cx, obj, id, flags, objp, propp);
 }
 
 } /* namespace js */
 
 PropertyCacheEntry *
-js_FindPropertyHelper(JSContext *cx, jsid id, JSBool cacheResult,
+js_FindPropertyHelper(JSContext *cx, jsid id, bool cacheResult, bool global,
                       JSObject **objp, JSObject **pobjp, JSProperty **propp)
 {
     JSObject *scopeChain, *obj, *parent, *pobj;
     PropertyCacheEntry *entry;
     int scopeIndex;
     JSProperty *prop;
 
     JS_ASSERT_IF(cacheResult, !JS_ON_TRACE(cx));
     scopeChain = cx->stack.currentScriptedScopeChain();
 
+    if (global) {
+        /*
+         * Skip along the scope chain to the enclosing global object. This is
+         * used for GNAME opcodes where the bytecode emitter has determined a
+         * name access must be on the global. It also insulates us from bugs
+         * in the emitter: type inference will assume that GNAME opcodes are
+         * accessing the global object, and the inferred behavior should match
+         * the actual behavior even if the id could be found on the scope chain
+         * before the global object.
+         */
+        scopeChain = scopeChain->getGlobal();
+    }
+
     /* Scan entries on the scope chain that we can cache across. */
     entry = JS_NO_PROP_CACHE_FILL;
     obj = scopeChain;
     parent = obj->getParent();
     for (scopeIndex = 0;
          parent
          ? IsCacheableNonGlobalScope(obj)
          : !obj->getOps()->lookupProperty;
@@ -5538,20 +5551,20 @@ js_FindPropertyHelper(JSContext *cx, jsi
     return entry;
 }
 
 /*
  * On return, if |*pobjp| is a native object, then |*propp| is a |Shape *|.
  * Otherwise, its type and meaning depends on the host object's implementation.
  */
 JS_FRIEND_API(JSBool)
-js_FindProperty(JSContext *cx, jsid id, JSObject **objp, JSObject **pobjp,
-                JSProperty **propp)
-{
-    return !!js_FindPropertyHelper(cx, id, false, objp, pobjp, propp);
+js_FindProperty(JSContext *cx, jsid id, bool global,
+                JSObject **objp, JSObject **pobjp, JSProperty **propp)
+{
+    return !!js_FindPropertyHelper(cx, id, false, global, objp, pobjp, propp);
 }
 
 JSObject *
 js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id)
 {
     /*
      * This function should not be called for a global object or from the
      * trace and should have a valid cache entry for native scopeChain.
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1717,26 +1717,26 @@ IsCacheableNonGlobalScope(JSObject *obj)
 }
 
 }
 
 /*
  * If cacheResult is false, return JS_NO_PROP_CACHE_FILL on success.
  */
 extern js::PropertyCacheEntry *
-js_FindPropertyHelper(JSContext *cx, jsid id, JSBool cacheResult,
+js_FindPropertyHelper(JSContext *cx, jsid id, bool cacheResult, bool global,
                       JSObject **objp, JSObject **pobjp, JSProperty **propp);
 
 /*
- * Return the index along the scope chain in which id was found, or the last
- * index if not found, or -1 on error.
+ * Search for id either on the current scope chain or on the scope chain's
+ * global object, per the global parameter.
  */
 extern JS_FRIEND_API(JSBool)
-js_FindProperty(JSContext *cx, jsid id, JSObject **objp, JSObject **pobjp,
-                JSProperty **propp);
+js_FindProperty(JSContext *cx, jsid id, bool global,
+                JSObject **objp, JSObject **pobjp, JSProperty **propp);
 
 extern JS_REQUIRES_STACK JSObject *
 js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id);
 
 extern JSObject *
 js_FindVariableScope(JSContext *cx, JSFunction **funp);
 
 /*
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -8070,21 +8070,23 @@ JS_REQUIRES_STACK AbortableRecordingStat
 TraceRecorder::scopeChainProp(JSObject* chainHead, Value*& vp, LIns*& ins, NameResult& nr,
                               JSObject** scopeObjp)
 {
     JS_ASSERT(chainHead == &cx->fp()->scopeChain());
     JS_ASSERT(chainHead != globalObj);
 
     TraceMonitor &localtm = *traceMonitor;
 
-    JSAtom* atom = atoms[GET_INDEX(cx->regs().pc)];
+    jsbytecode *pc = cx->regs().pc;
+    JSAtom* atom = atoms[GET_INDEX(pc)];
+    bool global = (js_CodeSpec[*pc].format & JOF_GNAME);
     JSObject* obj2;
     JSProperty* prop;
     JSObject *obj = chainHead;
-    if (!js_FindProperty(cx, ATOM_TO_JSID(atom), &obj, &obj2, &prop))
+    if (!js_FindProperty(cx, ATOM_TO_JSID(atom), global, &obj, &obj2, &prop))
         RETURN_ERROR_A("error in js_FindProperty");
 
     /* js_FindProperty can reenter the interpreter and kill |this|. */
     if (!localtm.recorder)
         return ARECORD_ABORTED;
 
     if (!prop)
         RETURN_STOP_A("failed to find name in non-global scope chain");
@@ -9503,17 +9505,18 @@ TraceRecorder::test_property_cache(JSObj
         // The lookup below may change object shapes.
         forgetGuardedShapes();
 
         JSProperty* prop;
         if (JOF_OPMODE(*pc) == JOF_NAME) {
             JS_ASSERT(aobj == obj);
 
             TraceMonitor &localtm = *traceMonitor;
-            entry = js_FindPropertyHelper(cx, id, true, &obj, &obj2, &prop);
+            bool global = (js_CodeSpec[*pc].format & JOF_GNAME);
+            entry = js_FindPropertyHelper(cx, id, true, global, &obj, &obj2, &prop);
             if (!entry)
                 RETURN_ERROR_A("error in js_FindPropertyHelper");
 
             /* js_FindPropertyHelper can reenter the interpreter and kill |this|. */
             if (!localtm.recorder)
                 return ARECORD_ABORTED;
 
             if (entry == JS_NO_PROP_CACHE_FILL)
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -702,17 +702,18 @@ struct GetPropertyHelper {
 
     GetPropertyHelper(JSContext *cx, JSObject *obj, JSAtom *atom, IC &ic, VMFrame &f)
       : cx(cx), obj(obj), atom(atom), ic(ic), f(f), holder(NULL), prop(NULL), shape(NULL)
     { }
 
   public:
     LookupStatus bind() {
         RecompilationMonitor monitor(cx);
-        if (!js_FindProperty(cx, ATOM_TO_JSID(atom), &obj, &holder, &prop))
+        bool global = (js_CodeSpec[*f.pc()].format & JOF_GNAME);
+        if (!js_FindProperty(cx, ATOM_TO_JSID(atom), global, &obj, &holder, &prop))
             return ic.error(cx);
         if (monitor.recompiled())
             return Lookup_Uncacheable;
         if (!prop)
             return ic.disable(cx, "lookup failed");
         if (!obj->isNative())
             return ic.disable(cx, "non-native");
         if (!IsCacheableProtoChain(obj, holder))
--- a/js/src/methodjit/StubCalls.cpp
+++ b/js/src/methodjit/StubCalls.cpp
@@ -357,17 +357,18 @@ NameOp(VMFrame &f, JSObject *obj, bool c
             shape = entry->vword.toShape();
             NATIVE_GET(cx, obj, obj2, shape, JSGET_METHOD_BARRIER, &rval, return NULL);
         }
 
         JS_ASSERT(obj->isGlobal() || IsCacheableNonGlobalScope(obj));
     } else {
         id = ATOM_TO_JSID(atom);
         JSProperty *prop;
-        if (!js_FindPropertyHelper(cx, id, true, &obj, &obj2, &prop))
+        bool global = (js_CodeSpec[*f.pc()].format & JOF_GNAME);
+        if (!js_FindPropertyHelper(cx, id, true, global, &obj, &obj2, &prop))
             return NULL;
         if (!prop) {
             /* Kludge to allow (typeof foo == "undefined") tests. */
             JSOp op2 = js_GetOpcode(cx, f.script(), f.pc() + JSOP_NAME_LENGTH);
             if (op2 == JSOP_TYPEOF) {
                 f.regs.sp++;
                 f.regs.sp[-1].setUndefined();
                 f.script()->types.monitor(cx, f.pc(), f.regs.sp[-1]);
@@ -1715,17 +1716,18 @@ NameIncDec(VMFrame &f, JSObject *obj, JS
                 f.regs.sp[0].setInt32(tmp);
                 return true;
             }
         }
         atom = origAtom;
     }
 
     jsid id = ATOM_TO_JSID(atom);
-    if (!js_FindPropertyHelper(cx, id, true, &obj, &obj2, &prop))
+    bool global = (js_CodeSpec[*f.pc()].format & JOF_GNAME);
+    if (!js_FindPropertyHelper(cx, id, true, global, &obj, &obj2, &prop))
         return false;
     if (!prop) {
         ReportAtomNotDefined(cx, atom);
         return false;
     }
     return ObjIncOp<N, POST, strict, false>(f, obj, id);
 }
 
@@ -2615,17 +2617,17 @@ stubs::ArgSub(VMFrame &f, uint32 n)
 }
 
 void JS_FASTCALL
 stubs::DelName(VMFrame &f, JSAtom *atom)
 {
     jsid id = ATOM_TO_JSID(atom);
     JSObject *obj, *obj2;
     JSProperty *prop;
-    if (!js_FindProperty(f.cx, id, &obj, &obj2, &prop))
+    if (!js_FindProperty(f.cx, id, false, &obj, &obj2, &prop))
         THROW();
 
     /* Strict mode code should never contain JSOP_DELNAME opcodes. */
     JS_ASSERT(!f.script()->strictModeCode);
 
     /* ECMA says to return true if name is undefined or inherited. */
     f.regs.sp++;
     f.regs.sp[-1] = BooleanValue(true);
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2704,17 +2704,17 @@ DumpStats(JSContext *cx, uintN argc, jsv
         } else if (JS_FlatStringEqualsAscii(flatStr, "atom")) {
             js_DumpAtoms(cx, gOutFile);
         } else if (JS_FlatStringEqualsAscii(flatStr, "global")) {
             DumpScope(cx, cx->globalObject, stdout);
         } else {
             if (!JS_ValueToId(cx, STRING_TO_JSVAL(str), &id))
                 return JS_FALSE;
             JSObject *obj;
-            if (!js_FindProperty(cx, id, &obj, &obj2, &prop))
+            if (!js_FindProperty(cx, id, false, &obj, &obj2, &prop))
                 return JS_FALSE;
             if (prop) {
                 if (!obj->getProperty(cx, id, &value))
                     return JS_FALSE;
             }
             if (!prop || !value.isObjectOrNull()) {
                 fputs("js: invalid stats argument ", gErrFile);
                 JS_FileEscapedString(gErrFile, str, 0);