From de8c94c37827d1681dcab6b6d57e3c1d0ea9e1ab Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 12 Oct 2010 11:38:06 -0700 Subject: [PATCH] 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 | 18 ++++++++++++-- js/src/jsfun.cpp | 52 +++++++++++++++++++++++++++------------- js/src/jsfun.h | 9 +++++++ js/src/jsinterp.cpp | 26 +++++++++++++++----- js/src/jsinterpinlines.h | 4 +--- js/src/jsobj.h | 18 +++++++++++--- js/src/jsobjinlines.h | 12 ++++++---- js/src/jstracer.cpp | 2 +- 8 files changed, 105 insertions(+), 36 deletions(-) diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index acbf33cfe88..f3bea1faf2f 100644 --- a/js/src/jsemit.cpp +++ b/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 - * access to an undeclared global would be an error. + * This conversion is not made if we are in strict mode. In eval code nested + * 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) diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index b7b15a52ff3..9bec10f2946 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -941,12 +941,16 @@ CalleeGetter(JSContext *cx, JSObject *obj, jsid id, Value *vp) return CheckForEscapingClosure(cx, obj, vp); } -static JSObject * -NewCallObject(JSContext *cx, JSFunction *fun, JSObject &scopeChain, JSObject &callee) -{ - Bindings &bindings = fun->script()->bindings; +namespace js { - 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 diff --git a/js/src/jsfun.h b/js/src/jsfun.h index 8baf59e3aed..a8aaef2c4cd 100644 --- a/js/src/jsfun.h +++ b/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. */ diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index e3f0967cefe..bce16717634 100644 --- a/js/src/jsinterp.cpp +++ b/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) - ? chain->getGlobal() - : chain; + initialVarObj = (cx->options & JSOPTION_VAROBJFIX) ? 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) { diff --git a/js/src/jsinterpinlines.h b/js/src/jsinterpinlines.h index f7771feef55..451e723bbe3 100644 --- a/js/src/jsinterpinlines.h +++ b/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 | - JSFRAME_GLOBAL | - JSFRAME_HAS_CALL_OBJ)); + (prev->flags_ & (JSFRAME_FUNCTION | JSFRAME_GLOBAL | JSFRAME_HAS_CALL_OBJ)); if (isFunctionFrame()) { exec = prev->exec; args.script = script; diff --git a/js/src/jsobj.h b/js/src/jsobj.h index f190ff065d2..29640b3494b 100644 --- a/js/src/jsobj.h +++ b/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); diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index b6693512946..09c2011e719 100644 --- a/js/src/jsobjinlines.h +++ b/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()); - return getSlotRef(JSSLOT_CALL_CALLEE).setObject(callee); + JS_ASSERT_IF(callee, callee->isFunction()); + 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); } diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 484dcd6a22d..2a3f93e2f20 100644 --- a/js/src/jstracer.cpp +++ b/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);