From 6043733d9418136847e472d2888016016b0e4bfe Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 3 Feb 2009 23:14:36 -0800 Subject: [PATCH] Bug 474501 - JS array and object literals should define properties, not set them, to avoid calling getters or setters along the prototype chain. r=brendan --- js/src/jsinterp.cpp | 22 +++++++++--------- js/src/jsobj.cpp | 14 ++++++++++-- js/src/jsobj.h | 5 ++++- js/src/jsscope.h | 55 +++++++++++++++++++++++++++------------------ 4 files changed, 61 insertions(+), 35 deletions(-) diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index d3d928c50d44..8056b1922615 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -1,4 +1,4 @@ -/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- * vim: set ts=8 sw=4 et tw=79: * * ***** BEGIN LICENSE BLOCK ***** @@ -6244,8 +6244,13 @@ js_Interpret(JSContext *cx) NULL, NULL)) { goto error; } - if (!js_SetPropertyHelper(cx, obj, id, &rval, &entry)) + if (JS_UNLIKELY(atom == cx->runtime->atomState.protoAtom) + ? !js_SetPropertyHelper(cx, obj, id, &rval, &entry) + : !js_DefineNativeProperty(cx, obj, id, rval, NULL, NULL, + JSPROP_ENUMERATE, 0, 0, NULL, + &entry)) { goto error; + } #ifdef JS_TRACER if (entry) TRACE_1(SetPropMiss, entry); @@ -6273,13 +6278,11 @@ js_Interpret(JSContext *cx) * Check for property redeclaration strict warning (we may be in * an object initialiser, not an array initialiser). */ - if (!js_CheckRedeclaration(cx, obj, id, JSPROP_INITIALIZER, NULL, - NULL)) { + if (!js_CheckRedeclaration(cx, obj, id, JSPROP_INITIALIZER, NULL, NULL)) goto error; - } /* - * If rval is a hole, do not call OBJ_SET_PROPERTY. In this case, + * If rval is a hole, do not call OBJ_DEFINE_PROPERTY. In this case, * obj must be an array, so if the current op is the last element * initialiser, set the array length to one greater than id. */ @@ -6288,12 +6291,11 @@ js_Interpret(JSContext *cx) JS_ASSERT(JSID_IS_INT(id)); JS_ASSERT((jsuint) JSID_TO_INT(id) < ARRAY_INIT_LIMIT); if ((JSOp) regs.pc[JSOP_INITELEM_LENGTH] == JSOP_ENDINIT && - !js_SetLengthProperty(cx, obj, - (jsuint) (JSID_TO_INT(id) + 1))) { + !js_SetLengthProperty(cx, obj, (jsuint) (JSID_TO_INT(id) + 1))) { goto error; } } else { - if (!OBJ_SET_PROPERTY(cx, obj, id, &rval)) + if (!OBJ_DEFINE_PROPERTY(cx, obj, id, rval, NULL, NULL, JSPROP_ENUMERATE, NULL)) goto error; } regs.sp -= 2; @@ -6846,7 +6848,7 @@ js_Interpret(JSContext *cx) goto error; } id = INT_TO_JSID(i); - if (!OBJ_SET_PROPERTY(cx, obj, id, &rval)) + if (!OBJ_DEFINE_PROPERTY(cx, obj, id, rval, NULL, NULL, JSPROP_ENUMERATE, NULL)) goto error; regs.sp--; END_CASE(JSOP_ARRAYPUSH) diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index dcb38915e152..8012b2b93074 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -3428,7 +3428,8 @@ js_DefineProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, JSBool js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, JSPropertyOp getter, JSPropertyOp setter, uintN attrs, - uintN flags, intN shortid, JSProperty **propp) + uintN flags, intN shortid, JSProperty **propp, + JSPropCacheEntry** entryp /* = NULL */) { JSClass *clasp; JSScope *scope; @@ -3437,6 +3438,8 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, /* Convert string indices to integers if appropriate. */ CHECK_FOR_STRING_INDEX(id); + uint32 shape = OBJ_SHAPE(obj); + #if JS_HAS_GETTER_SETTER /* * If defining a getter or setter, we must check for its counterpart and @@ -3524,6 +3527,13 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, js_RemoveScopeProperty(cx, scope, id); goto bad); + if (entryp) { + JS_ASSERT_NOT_ON_TRACE(cx); + if (!(attrs & JSPROP_SHARED)) + js_FillPropertyCache(cx, obj, shape, 0, 0, obj, sprop, entryp); + else + PCMETER(JS_PROPERTY_CACHE(cx).nofills++); + } if (propp) *propp = (JSProperty *) sprop; else @@ -4156,7 +4166,7 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp, return JS_TRUE; } - return !!SPROP_SET(cx, sprop, obj, pobj, vp); + return SPROP_SET(cx, sprop, obj, pobj, vp); } /* Restore attrs to the ECMA default for new properties. */ diff --git a/js/src/jsobj.h b/js/src/jsobj.h index fad3bc732e46..915fd46960fa 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -546,10 +546,13 @@ js_DefineProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, JSPropertyOp getter, JSPropertyOp setter, uintN attrs, JSProperty **propp); +#ifdef __cplusplus /* FIXME: bug 442399 removes this LiveConnect requirement. */ extern JSBool js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, JSPropertyOp getter, JSPropertyOp setter, uintN attrs, - uintN flags, intN shortid, JSProperty **propp); + uintN flags, intN shortid, JSProperty **propp, + JSPropCacheEntry** entryp = NULL); +#endif /* * Unlike js_DefineProperty, propp must be non-null. On success, and if id was diff --git a/js/src/jsscope.h b/js/src/jsscope.h index 96b9be9ee7b5..fac02ad35ecc 100644 --- a/js/src/jsscope.h +++ b/js/src/jsscope.h @@ -328,29 +328,40 @@ struct JSScopeProperty { #define SPROP_HAS_STUB_GETTER(sprop) (!(sprop)->getter) #define SPROP_HAS_STUB_SETTER(sprop) (!(sprop)->setter) -/* - * NB: SPROP_GET must not be called if SPROP_HAS_STUB_GETTER(sprop). - */ -#define SPROP_GET(cx,sprop,obj,obj2,vp) \ - (((sprop)->attrs & JSPROP_GETTER) \ - ? js_InternalGetOrSet(cx, obj, (sprop)->id, \ - OBJECT_TO_JSVAL((JSObject *) (sprop)->getter), \ - JSACC_READ, 0, 0, vp) \ - : (sprop)->getter(cx, OBJ_THIS_OBJECT(cx,obj), SPROP_USERID(sprop), vp)) +static JS_INLINE JSBool +SPROP_GET(JSContext* cx, JSScopeProperty* sprop, JSObject* obj, JSObject* obj2, jsval* vp) +{ + JS_ASSERT(!SPROP_HAS_STUB_GETTER(sprop)); -/* - * NB: SPROP_SET must not be called if (SPROP_HAS_STUB_SETTER(sprop) && - * !(sprop->attrs & JSPROP_GETTER)). - */ -#define SPROP_SET(cx,sprop,obj,obj2,vp) \ - (((sprop)->attrs & JSPROP_SETTER) \ - ? js_InternalGetOrSet(cx, obj, (sprop)->id, \ - OBJECT_TO_JSVAL((JSObject *) (sprop)->setter), \ - JSACC_WRITE, 1, vp, vp) \ - : ((sprop)->attrs & JSPROP_GETTER) \ - ? (JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, \ - JSMSG_GETTER_ONLY, NULL), JS_FALSE) \ - : (sprop)->setter(cx, OBJ_THIS_OBJECT(cx,obj), SPROP_USERID(sprop), vp)) + if (sprop->attrs & JSPROP_GETTER) { + return js_InternalGetOrSet(cx, obj, sprop->id, + OBJECT_TO_JSVAL((JSObject *) sprop->getter), JSACC_READ, + 0, 0, vp); + } + + return sprop->getter(cx, OBJ_THIS_OBJECT(cx, obj), SPROP_USERID(sprop), vp); +} + +static JS_INLINE JSBool +SPROP_SET(JSContext* cx, JSScopeProperty* sprop, JSObject* obj, JSObject* obj2, jsval* vp) +{ + JS_ASSERT(!(SPROP_HAS_STUB_SETTER(sprop) && + !(sprop->attrs & JSPROP_GETTER))); + + if (sprop->attrs & JSPROP_SETTER) { + return js_InternalGetOrSet(cx, obj, (sprop)->id, + OBJECT_TO_JSVAL((JSObject *) sprop->setter), + JSACC_WRITE, 1, vp, vp); + } + + if (sprop->attrs & JSPROP_GETTER) { + JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, + JSMSG_GETTER_ONLY, NULL); + return JS_FALSE; + } + + return sprop->setter(cx, OBJ_THIS_OBJECT(cx, obj), SPROP_USERID(sprop), vp); +} /* Macro for common expression to test for shared permanent attributes. */ #define SPROP_IS_SHARED_PERMANENT(sprop) \