From 387907ce9f0d1a72b2a0d8ccd8ac61fdb37c5eb6 Mon Sep 17 00:00:00 2001 From: Blake Kaplan Date: Thu, 11 Feb 2010 17:04:42 -0800 Subject: [PATCH] Bug 515496 - Eliminate extra security check when computing this. r=jorendorff. --- js/src/jsapi.cpp | 6 ++--- js/src/jsdbgapi.cpp | 2 +- js/src/jsinterp.cpp | 59 ++++++--------------------------------------- js/src/jsinterp.h | 6 ++--- js/src/jstracer.cpp | 4 +-- 5 files changed, 17 insertions(+), 60 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 777b9d59b1e..f906ccf99b8 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -1693,7 +1693,7 @@ JS_GetGlobalForObject(JSContext *cx, JSObject *obj) JS_PUBLIC_API(jsval) JS_ComputeThis(JSContext *cx, jsval *vp) { - if (!js_ComputeThis(cx, JS_FALSE, vp + 2)) + if (!js_ComputeThis(cx, vp + 2)) return JSVAL_NULL; return vp[1]; } @@ -4285,7 +4285,7 @@ js_generic_fast_native_method_dispatcher(JSContext *cx, uintN argc, jsval *vp) * Follow Function.prototype.apply and .call by using the global object as * the 'this' param if no args. */ - if (!js_ComputeThis(cx, JS_FALSE, vp + 2)) + if (!js_ComputeThis(cx, vp + 2)) return JS_FALSE; /* * Protect against argc underflowing. By calling js_ComputeThis, we made @@ -4349,7 +4349,7 @@ js_generic_native_method_dispatcher(JSContext *cx, JSObject *obj, * Follow Function.prototype.apply and .call by using the global object as * the 'this' param if no args. */ - if (!js_ComputeThis(cx, JS_TRUE, argv)) + if (!js_ComputeThis(cx, argv)) return JS_FALSE; js_GetTopStackFrame(cx)->thisv = argv[-1]; JS_ASSERT(cx->fp->argv == argv); diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 4d8b57b1888..50b6e82512d 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -1243,7 +1243,7 @@ JS_GetFrameThis(JSContext *cx, JSStackFrame *fp) } if (fp->argv) - fp->thisv = OBJECT_TO_JSVAL(js_ComputeThis(cx, JS_TRUE, fp->argv)); + fp->thisv = OBJECT_TO_JSVAL(js_ComputeThis(cx, fp->argv)); if (afp) { cx->fp = afp; diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 32fe8f00e75..fc525ca66b1 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -399,7 +399,7 @@ CallThisObjectHook(JSContext *cx, JSObject *obj, jsval *argv) * The alert should display "true". */ JS_STATIC_INTERPRET JSObject * -js_ComputeGlobalThis(JSContext *cx, JSBool lazy, jsval *argv) +js_ComputeGlobalThis(JSContext *cx, jsval *argv) { JSObject *thisp; @@ -407,57 +407,14 @@ js_ComputeGlobalThis(JSContext *cx, JSBool lazy, jsval *argv) !JSVAL_TO_OBJECT(argv[-2])->getParent()) { thisp = cx->globalObject; } else { - jsid id; - jsval v; - uintN attrs; - JSBool ok; - JSObject *parent; - - /* - * Walk up the parent chain, first checking that the running script - * has access to the callee's parent object. Note that if lazy, the - * running script whose principals we want to check is the script - * associated with fp->down, not with fp. - * - * FIXME: 417851 -- this access check should not be required, as it - * imposes a performance penalty on all js_ComputeGlobalThis calls, - * and it represents a maintenance hazard. - * - * When the above FIXME is made fixed, the whole GC reachable frame - * mechanism can be removed as well. - */ - JSStackFrame *fp = js_GetTopStackFrame(cx); - JSGCReachableFrame reachable; - if (lazy) { - JS_ASSERT(fp->argv == argv); - cx->fp = fp->down; - fp->down = NULL; - cx->pushGCReachableFrame(reachable, fp); - } - thisp = JSVAL_TO_OBJECT(argv[-2]); - id = ATOM_TO_JSID(cx->runtime->atomState.parentAtom); - - ok = thisp->checkAccess(cx, id, JSACC_PARENT, &v, &attrs); - if (lazy) { - fp->down = cx->fp; - cx->fp = fp; - cx->popGCReachableFrame(); - } - if (!ok) - return NULL; - - if (v != JSVAL_NULL) { - thisp = JSVAL_IS_VOID(v) ? thisp->getParent() : JSVAL_TO_OBJECT(v); - while ((parent = thisp->getParent()) != NULL) - thisp = parent; - } + thisp = JS_GetGlobalForObject(cx, JSVAL_TO_OBJECT(argv[-2])); } return CallThisObjectHook(cx, thisp, argv); } static JSObject * -ComputeThis(JSContext *cx, JSBool lazy, jsval *argv) +ComputeThis(JSContext *cx, jsval *argv) { JSObject *thisp; @@ -471,18 +428,18 @@ ComputeThis(JSContext *cx, JSBool lazy, jsval *argv) thisp = JSVAL_TO_OBJECT(argv[-1]); if (OBJ_GET_CLASS(cx, thisp) == &js_CallClass || OBJ_GET_CLASS(cx, thisp) == &js_BlockClass) - return js_ComputeGlobalThis(cx, lazy, argv); + return js_ComputeGlobalThis(cx, argv); return CallThisObjectHook(cx, thisp, argv); } JSObject * -js_ComputeThis(JSContext *cx, JSBool lazy, jsval *argv) +js_ComputeThis(JSContext *cx, jsval *argv) { JS_ASSERT(argv[-1] != JSVAL_HOLE); // check for SynthesizeFrame poisoning if (JSVAL_IS_NULL(argv[-1])) - return js_ComputeGlobalThis(cx, lazy, argv); - return ComputeThis(cx, lazy, argv); + return js_ComputeGlobalThis(cx, argv); + return ComputeThis(cx, argv); } #if JS_HAS_NO_SUCH_METHOD @@ -723,7 +680,7 @@ js_Invoke(JSContext *cx, uintN argc, jsval *vp, uintN flags) * the appropriate this-computing bytecode, e.g., JSOP_THIS. */ if (native && (!fun || !(fun->flags & JSFUN_FAST_NATIVE))) { - if (!js_ComputeThis(cx, JS_FALSE, vp + 2)) { + if (!js_ComputeThis(cx, vp + 2)) { ok = JS_FALSE; goto out2; } diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 12016d72bb7..b4548a8414e 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -265,7 +265,7 @@ js_GetPrimitiveThis(JSContext *cx, jsval *vp, JSClass *clasp, jsval *thisvp); * must not be a JSVAL_VOID. */ extern JSObject * -js_ComputeThis(JSContext *cx, JSBool lazy, jsval *argv); +js_ComputeThis(JSContext *cx, jsval *argv); extern const uint16 js_PrimitiveTestFlags[]; @@ -280,7 +280,7 @@ js_ComputeThisForFrame(JSContext *cx, JSStackFrame *fp) { if (fp->flags & JSFRAME_COMPUTED_THIS) return JSVAL_TO_OBJECT(fp->thisv); /* JSVAL_COMPUTED_THIS invariant */ - JSObject* obj = js_ComputeThis(cx, JS_TRUE, fp->argv); + JSObject* obj = js_ComputeThis(cx, fp->argv); if (!obj) return NULL; fp->thisv = OBJECT_TO_JSVAL(obj); @@ -421,7 +421,7 @@ js_FreeRawStack(JSContext *cx, void *mark); * The alert should display "true". */ extern JSObject * -js_ComputeGlobalThis(JSContext *cx, JSBool lazy, jsval *argv); +js_ComputeGlobalThis(JSContext *cx, jsval *argv); extern JS_REQUIRES_STACK JSBool js_EnterWith(JSContext *cx, jsint stackIndex); diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 27b4e87089b..84fbcf922cd 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -9437,7 +9437,7 @@ TraceRecorder::getThis(LIns*& this_ins) JSObject* thisObj = js_ComputeThisForFrame(cx, cx->fp); if (!thisObj) - RETURN_ERROR("js_ComputeThisForName failed"); + RETURN_ERROR("js_ComputeThisForFrame failed"); /* In global code, bake in the global object as 'this' object. */ if (!cx->fp->callee()) { @@ -10798,7 +10798,7 @@ TraceRecorder::callNative(uintN argc, JSOp mode) */ if (!(fun->flags & JSFUN_FAST_NATIVE)) { if (JSVAL_IS_NULL(vp[1])) { - JSObject* thisObj = js_ComputeThis(cx, JS_FALSE, vp + 2); + JSObject* thisObj = js_ComputeThis(cx, vp + 2); if (!thisObj) RETURN_ERROR("error in js_ComputeGlobalThis"); this_ins = INS_CONSTOBJ(thisObj);