Bug 514568 - Use a fresh variable environment for strict mode code run by eval, and give strict mode eval code frames a Call object backed by those variables. r=igor

js/src/jsemit.cpp

@@ -2071,8 +2071,22 @@ MakeUpvarForEval(JSParseNode *pn, JSCodeGenerator *cg)
  * Try to convert a *NAME op to a *GNAME op, which optimizes access to
  * undeclared globals. Return true if a conversion was made.
  *
- * This conversion is not made if we are in strict mode, because the
+ * This conversion is not made if we are in strict mode.  In eval code nested
- * access to an undeclared global would be an error.
+ * within (strict mode) eval code, access to an undeclared "global" might
+ * merely be to a binding local to that outer eval:
+ *
+ *   "use strict";
+ *   var x = "global";
+ *   eval('var x = "eval"; eval("x");'); // 'eval', not 'global'
+ *
+ * Outside eval code, access to an undeclared global is a strict mode error:
+ *
+ *   "use strict";
+ *   function foo()
+ *   {
+ *     undeclared = 17; // throws ReferenceError
+ *   }
+ *   foo();
  */
 static bool
 TryConvertToGname(JSCodeGenerator *cg, JSParseNode *pn, JSOp *op)

js/src/jsfun.cpp

@@ -941,12 +941,16 @@ CalleeGetter(JSContext *cx, JSObject *obj, jsid id, Value *vp)
     return CheckForEscapingClosure(cx, obj, vp);
 }
 
-static JSObject *
+namespace js {
-NewCallObject(JSContext *cx, JSFunction *fun, JSObject &scopeChain, JSObject &callee)
-{
-    Bindings &bindings = fun->script()->bindings;
 
-    size_t argsVars = bindings.countArgsAndVars();
+/*
+ * Construct a call object for the given bindings.  The callee is the function
+ * on behalf of which the call object is being created.
+ */
+JSObject *
+NewCallObject(JSContext *cx, Bindings *bindings, JSObject &scopeChain, JSObject *callee)
+{
+    size_t argsVars = bindings->countArgsAndVars();
     size_t slots = JSObject::CALL_RESERVED_SLOTS + argsVars;
     gc::FinalizeKind kind = gc::GetGCObjectKind(slots);
 
@@ -956,7 +960,7 @@ NewCallObject(JSContext *cx, JSFunction *fun, JSObject &scopeChain, JSObject &ca
 
     /* Init immediately to avoid GC seeing a half-init'ed object. */
     callobj->init(cx, &js_CallClass, NULL, &scopeChain, NULL, false);
-    callobj->setMap(bindings.lastShape());
+    callobj->setMap(bindings->lastShape());
 
     /* This must come after callobj->lastProp has been set. */
     if (!callobj->ensureInstanceReservedSlots(cx, argsVars))
@@ -976,6 +980,8 @@ NewCallObject(JSContext *cx, JSFunction *fun, JSObject &scopeChain, JSObject &ca
     return callobj;
 }
 
+} // namespace js
+
 static inline JSObject *
 NewDeclEnvObject(JSContext *cx, JSStackFrame *fp)
 {
@@ -1029,7 +1035,8 @@ js_GetCallObject(JSContext *cx, JSStackFrame *fp)
         }
     }
 
-    JSObject *callobj = NewCallObject(cx, fp->fun(), fp->scopeChain(), fp->callee());
+    JSObject *callobj =
+        NewCallObject(cx, &fp->fun()->script()->bindings, fp->scopeChain(), &fp->callee());
     if (!callobj)
         return NULL;
 
@@ -1049,7 +1056,8 @@ js_CreateCallObjectOnTrace(JSContext *cx, JSFunction *fun, JSObject *callee, JSO
 {
     JS_ASSERT(!js_IsNamedLambda(fun));
     JS_ASSERT(scopeChain);
-    return NewCallObject(cx, fun, *scopeChain, *callee);
+    JS_ASSERT(callee);
+    return NewCallObject(cx, &fun->script()->bindings, *scopeChain, callee);
 }
 
 JS_DEFINE_CALLINFO_4(extern, OBJECT, js_CreateCallObjectOnTrace, CONTEXT, FUNCTION, OBJECT, OBJECT,
@@ -1208,7 +1216,10 @@ GetFlatUpvar(JSContext *cx, JSObject *obj, jsid id, Value *vp)
     JS_ASSERT((int16) JSID_TO_INT(id) == JSID_TO_INT(id));
     uintN i = (uint16) JSID_TO_INT(id);
 
-    *vp = obj->getCallObjCallee().getFlatClosureUpvar(i);
+    JSObject *callee = obj->getCallObjCallee();
+    JS_ASSERT(callee);
+
+    *vp = callee->getFlatClosureUpvar(i);
     return true;
 }
 
@@ -1218,7 +1229,10 @@ SetFlatUpvar(JSContext *cx, JSObject *obj, jsid id, Value *vp)
     JS_ASSERT((int16) JSID_TO_INT(id) == JSID_TO_INT(id));
     uintN i = (uint16) JSID_TO_INT(id);
 
-    Value *upvarp = &obj->getCallObjCallee().getFlatClosureUpvar(i);
+    JSObject *callee = obj->getCallObjCallee();
+    JS_ASSERT(callee);
+
+    Value *upvarp = &callee->getFlatClosureUpvar(i);
     GC_POKE(cx, *upvarp);
     *upvarp = *vp;
     return true;
@@ -1296,9 +1310,15 @@ call_resolve(JSContext *cx, JSObject *obj, jsid id, uintN flags,
     JS_ASSERT(!obj->getProto());
 
     if (!JSID_IS_ATOM(id))
-        return JS_TRUE;
+        return true;
 
-    JS_ASSERT(!obj->getCallObjCalleeFunction()->script()->bindings.hasBinding(cx, JSID_TO_ATOM(id)));
+    JSObject *callee = obj->getCallObjCallee();
+#ifdef DEBUG
+    if (callee) {
+        JSScript *script = callee->getFunctionPrivate()->script();
+        JS_ASSERT(!script->bindings.hasBinding(cx, JSID_TO_ATOM(id)));
+    }
+#endif
 
     /*
      * Resolve arguments so that we never store a particular Call object's
@@ -1308,19 +1328,19 @@ call_resolve(JSContext *cx, JSObject *obj, jsid id, uintN flags,
      * properties; see js::Bindings::add and js::Interpret's JSOP_DEFFUN
      * rebinding-Call-property logic.
      */
-    if (JSID_IS_ATOM(id, cx->runtime->atomState.argumentsAtom)) {
+    if (callee && id == ATOM_TO_JSID(cx->runtime->atomState.argumentsAtom)) {
         if (!js_DefineNativeProperty(cx, obj, id, UndefinedValue(),
                                      GetCallArguments, SetCallArguments,
                                      JSPROP_PERMANENT | JSPROP_SHARED | JSPROP_ENUMERATE,
                                      0, 0, NULL, JSDNP_DONT_PURGE)) {
-            return JS_FALSE;
+            return false;
         }
         *objp = obj;
-        return JS_TRUE;
+        return true;
     }
 
     /* Control flow reaches here only if id was not resolved. */
-    return JS_TRUE;
+    return true;
 }
 
 static void

js/src/jsfun.h

@@ -321,6 +321,15 @@ JSObject::getFunctionPrivate() const
 
 namespace js {
 
+/*
+ * Construct a call object for the given bindings.  If this is a call object
+ * for a function invocation, callee should be the function being called.
+ * Otherwise it must be a call object for eval of strict mode code, and callee
+ * must be null.
+ */
+extern JSObject *
+NewCallObject(JSContext *cx, js::Bindings *bindings, JSObject &scopeChain, JSObject *callee);
+
 /*
  * NB: jsapi.h and jsobj.h must be included before any call to this macro.
  */

js/src/jsinterp.cpp

@@ -921,6 +921,9 @@ Execute(JSContext *cx, JSObject *chain, JSScript *script,
     if (!cx->stack().getExecuteFrame(cx, script, &frame))
         return false;
 
+    /* Initialize fixed slots (GVAR ops expect NULL). */
+    SetValueRangeToNull(frame.fp()->slots(), script->nfixed);
+
     /* Initialize frame and locals. */
     JSObject *initialVarObj;
     if (prev) {
@@ -953,15 +956,26 @@ Execute(JSContext *cx, JSObject *chain, JSScript *script,
             return false;
         frame.fp()->globalThis().setObject(*thisp);
 
-        initialVarObj = (cx->options & JSOPTION_VAROBJFIX)
+        initialVarObj = (cx->options & JSOPTION_VAROBJFIX) ? chain->getGlobal() : chain;
-                        ? chain->getGlobal()
+    }
-                        : chain;
+
+    /*
+     * Strict mode eval code receives its own, fresh lexical environment; thus
+     * strict mode eval can't mutate its calling frame's binding set.
+     */
+    if (script->strictModeCode) {
+        initialVarObj = NewCallObject(cx, &script->bindings, *initialVarObj, NULL);
+        if (!initialVarObj)
+            return false;
+        initialVarObj->setPrivate(frame.fp());
+
+        /* Clear the Call object propagated from the previous frame, if any. */
+        if (frame.fp()->hasCallObj())
+            frame.fp()->clearCallObj();
+        frame.fp()->setScopeChainAndCallObj(*initialVarObj);
     }
     JS_ASSERT(!initialVarObj->getOps()->defineProperty);
 
-    /* Initialize fixed slots (GVAR ops expect NULL). */
-    SetValueRangeToNull(frame.fp()->slots(), script->nfixed);
-
 #if JS_HAS_SHARP_VARS
     JS_STATIC_ASSERT(SHARP_NSLOTS == 2);
     if (script->hasSharps) {

js/src/jsinterpinlines.h

@@ -188,9 +188,7 @@ JSStackFrame::initEvalFrame(JSContext *cx, JSScript *script, JSStackFrame *prev,
 
     /* Initialize stack frame members. */
     flags_ = flagsArg | JSFRAME_HAS_PREVPC | JSFRAME_HAS_SCOPECHAIN |
-             (prev->flags_ & (JSFRAME_FUNCTION |
+             (prev->flags_ & (JSFRAME_FUNCTION | JSFRAME_GLOBAL | JSFRAME_HAS_CALL_OBJ));
-                              JSFRAME_GLOBAL |
-                              JSFRAME_HAS_CALL_OBJ));
     if (isFunctionFrame()) {
         exec = prev->exec;
         args.script = script;

js/src/jsobj.h

@@ -880,7 +880,15 @@ struct JSObject : js::gc::Cell {
 
   private:
     /*
-     * Reserved slot structure for Arguments objects:
+     * Reserved slot structure for Call objects:
+     *
+     * private               - the stack frame corresponding to the Call object
+     *                         until js_PutCallObject or its on-trace analog
+     *                         is called, null thereafter
+     * JSSLOT_CALL_CALLEE    - callee function for the stack frame, or null if
+     *                         the stack frame is for strict mode eval code
+     * JSSLOT_CALL_ARGUMENTS - arguments object for non-strict mode eval stack
+     *                         frames (not valid for strict mode eval frames)
      */
     static const uint32 JSSLOT_CALL_CALLEE = 0;
     static const uint32 JSSLOT_CALL_ARGUMENTS = 1;
@@ -892,9 +900,13 @@ struct JSObject : js::gc::Cell {
     /* The stack frame for this Call object, if the frame is still active. */
     inline JSStackFrame *maybeCallObjStackFrame() const;
 
-    inline JSObject &getCallObjCallee() const;
+    /*
+     * The callee function if this Call object was created for a function
+     * invocation, or null if it was created for a strict mode eval frame.
+     */
+    inline JSObject *getCallObjCallee() const;
     inline JSFunction *getCallObjCalleeFunction() const; 
-    inline void setCallObjCallee(JSObject &callee);
+    inline void setCallObjCallee(JSObject *callee);
 
     inline const js::Value &getCallObjArguments() const;
     inline void setCallObjArguments(const js::Value &v);

js/src/jsobjinlines.h

@@ -440,18 +440,18 @@ JSObject::maybeCallObjStackFrame() const
 }
 
 inline void
-JSObject::setCallObjCallee(JSObject &callee)
+JSObject::setCallObjCallee(JSObject *callee)
 {
     JS_ASSERT(isCall());
-    JS_ASSERT(callee.isFunction());
+    JS_ASSERT_IF(callee, callee->isFunction());
-    return getSlotRef(JSSLOT_CALL_CALLEE).setObject(callee);
+    return getSlotRef(JSSLOT_CALL_CALLEE).setObjectOrNull(callee);
 }
 
-inline JSObject &
+inline JSObject *
 JSObject::getCallObjCallee() const
 {
     JS_ASSERT(isCall());
-    return getSlot(JSSLOT_CALL_CALLEE).toObject();
+    return getSlot(JSSLOT_CALL_CALLEE).toObjectOrNull();
 }
 
 inline JSFunction *
@@ -465,6 +465,7 @@ inline const js::Value &
 JSObject::getCallObjArguments() const
 {
     JS_ASSERT(isCall());
+    JS_ASSERT(getCallObjCallee() != NULL);
     return getSlot(JSSLOT_CALL_ARGUMENTS);
 }
 
@@ -472,6 +473,7 @@ inline void
 JSObject::setCallObjArguments(const js::Value &v)
 {
     JS_ASSERT(isCall());
+    JS_ASSERT(getCallObjCallee() != NULL);
     setSlot(JSSLOT_CALL_ARGUMENTS, v);
 }
 

js/src/jstracer.cpp

@@ -3081,7 +3081,7 @@ public:
             JS_ASSERT(p == fp->addressOfScopeChain());
             if (frameobj->isCall() &&
                 !frameobj->getPrivate() &&
-                &fp->callee() == &frameobj->getCallObjCallee())
+                fp->maybeCallee() == frameobj->getCallObjCallee())
             {
                 JS_ASSERT(&fp->scopeChain() == JSStackFrame::sInvalidScopeChain);
                 frameobj->setPrivate(fp);