Fix getter/setter built-in vs. scripted type confusion: union getter/setter callable object pointer with raw JSPropertyOp pointer, fix watchpoint assertion/null-deref related to ES5's {get: undefined, set: undefined} new scripted getter/setter state encoding, clean up related code (560796, r=jwalden).

This commit is contained in:
Brendan Eich 2010-04-30 16:03:37 -07:00
parent 44cd0ab6dd
commit 52ed1d8098
7 changed files with 101 additions and 98 deletions

View File

@ -506,10 +506,8 @@ js_TraceWatchPoints(JSTracer *trc, JSObject *obj)
wp = (JSWatchPoint *)wp->links.next) {
if (wp->object == obj) {
wp->sprop->trace(trc);
if (wp->sprop->hasSetterValue() && wp->setter) {
JS_CALL_OBJECT_TRACER(trc, js_CastAsObject(wp->setter),
"wp->setter");
}
if (wp->sprop->hasSetterValue() && wp->setter)
JS_CALL_OBJECT_TRACER(trc, CastAsObject(wp->setter), "wp->setter");
JS_SET_TRACING_NAME(trc, "wp->closure");
js_CallValueTracerIfGCThing(trc, OBJECT_TO_JSVAL(wp->closure));
}
@ -735,7 +733,7 @@ js_watch_set(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
ok = !wp->setter ||
(sprop->hasSetterValue()
? js_InternalCall(cx, obj,
js_CastAsObjectJSVal(wp->setter),
CastAsObjectJSVal(wp->setter),
1, vp, vp)
: wp->setter(cx, obj, userid, vp));
if (injectFrame) {
@ -774,7 +772,7 @@ IsWatchedProperty(JSContext *cx, JSScopeProperty *sprop)
{
if (sprop->hasSetterValue()) {
JSObject *funobj = sprop->setterObject();
if (!funobj->isFunction())
if (!funobj || !funobj->isFunction())
return false;
JSFunction *fun = GET_FUNCTION_PRIVATE(cx, funobj);
@ -803,10 +801,10 @@ js_WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, JSPropertyOp setter)
}
wrapper = js_NewFunction(cx, NULL, js_watch_set_wrapper, 1, 0,
setter ? js_CastAsObject(setter)->getParent() : NULL, atom);
setter ? CastAsObject(setter)->getParent() : NULL, atom);
if (!wrapper)
return NULL;
return js_CastAsPropertyOp(FUN_OBJECT(wrapper));
return CastAsPropertyOp(FUN_OBJECT(wrapper));
}
JS_PUBLIC_API(JSBool)

View File

@ -1731,7 +1731,7 @@ js_obj_defineGetter(JSContext *cx, uintN argc, jsval *vp)
return JS_FALSE;
*vp = JSVAL_VOID;
return obj->defineProperty(cx, id, JSVAL_VOID,
js_CastAsPropertyOp(JSVAL_TO_OBJECT(fval)), JS_PropertyStub,
CastAsPropertyOp(JSVAL_TO_OBJECT(fval)), JS_PropertyStub,
JSPROP_ENUMERATE | JSPROP_GETTER | JSPROP_SHARED);
}
@ -1764,7 +1764,7 @@ js_obj_defineSetter(JSContext *cx, uintN argc, jsval *vp)
return JS_FALSE;
*vp = JSVAL_VOID;
return obj->defineProperty(cx, id, JSVAL_VOID,
JS_PropertyStub, js_CastAsPropertyOp(JSVAL_TO_OBJECT(fval)),
JS_PropertyStub, CastAsPropertyOp(JSVAL_TO_OBJECT(fval)),
JSPROP_ENUMERATE | JSPROP_SETTER | JSPROP_SHARED);
}
@ -2137,8 +2137,8 @@ Reject(JSContext *cx, JSObject *obj, JSProperty *prop, uintN errorNumber, bool t
}
static JSBool
DefinePropertyObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &desc,
bool throwError, bool *rval)
DefinePropertyOnObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &desc,
bool throwError, bool *rval)
{
/* 8.12.9 step 1. */
JSProperty *current;
@ -2176,9 +2176,7 @@ DefinePropertyObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &des
return JS_FALSE;
return js_DefineProperty(cx, obj, desc.id, JSVAL_VOID,
desc.getterObject() ? desc.getter() : JS_PropertyStub,
desc.setterObject() ? desc.setter() : JS_PropertyStub,
desc.attrs);
desc.getter(), desc.setter(), desc.attrs);
}
/* 8.12.9 steps 5-6 (note 5 is merely a special case of 6). */
@ -2202,13 +2200,15 @@ DefinePropertyObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &des
if (desc.hasGet &&
!js_SameValue(desc.getterValue(),
sprop->hasGetterValue() ? sprop->getterValue() : JSVAL_VOID, cx)) {
sprop->hasGetterValue() ? sprop->getterValue() : JSVAL_VOID,
cx)) {
break;
}
if (desc.hasSet &&
!js_SameValue(desc.setterValue(),
sprop->hasSetterValue() ? sprop->setterValue() : JSVAL_VOID, cx)) {
sprop->hasSetterValue() ? sprop->setterValue() : JSVAL_VOID,
cx)) {
break;
}
} else {
@ -2305,8 +2305,9 @@ DefinePropertyObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &des
return Reject(cx, obj2, current, JSMSG_CANT_REDEFINE_UNCONFIGURABLE_PROP,
throwError, desc.id, rval);
}
} else if (desc.isDataDescriptor() && sprop->isDataDescriptor()) {
} else if (desc.isDataDescriptor()) {
/* 8.12.9 step 10. */
JS_ASSERT(sprop->isDataDescriptor());
if (!sprop->configurable() && !sprop->writable()) {
if ((desc.hasWritable && desc.writable()) ||
(desc.hasValue && !js_SameValue(desc.value, v, cx))) {
@ -2324,7 +2325,8 @@ DefinePropertyObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &des
cx)) ||
(desc.hasGet &&
!js_SameValue(desc.getterValue(),
sprop->hasGetterValue() ? sprop->getterValue() : JSVAL_VOID, cx)))
sprop->hasGetterValue() ? sprop->getterValue() : JSVAL_VOID,
cx)))
{
return Reject(cx, obj2, current, JSMSG_CANT_REDEFINE_UNCONFIGURABLE_PROP,
throwError, desc.id, rval);
@ -2384,14 +2386,20 @@ DefinePropertyObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &des
changed |= JSPROP_SETTER | JSPROP_SHARED;
attrs = (desc.attrs & changed) | (sprop->attributes() & ~changed);
if (desc.hasGet)
getter = desc.getterObject() ? desc.getter() : JS_PropertyStub;
else
getter = sprop->hasDefaultGetter() ? JS_PropertyStub : sprop->getter();
if (desc.hasSet)
setter = desc.setterObject() ? desc.setter() : JS_PropertyStub;
else
setter = sprop->hasDefaultSetter() ? JS_PropertyStub : sprop->setter();
if (desc.hasGet) {
getter = desc.getter();
} else {
getter = (sprop->hasDefaultGetter() && !sprop->hasGetterValue())
? JS_PropertyStub
: sprop->getter();
}
if (desc.hasSet) {
setter = desc.setter();
} else {
setter = (sprop->hasDefaultSetter() && !sprop->hasSetterValue())
? JS_PropertyStub
: sprop->setter();
}
}
*rval = true;
@ -2400,8 +2408,8 @@ DefinePropertyObject(JSContext *cx, JSObject *obj, const PropertyDescriptor &des
}
static JSBool
DefinePropertyArray(JSContext *cx, JSObject *obj, const PropertyDescriptor &desc,
bool throwError, bool *rval)
DefinePropertyOnArray(JSContext *cx, JSObject *obj, const PropertyDescriptor &desc,
bool throwError, bool *rval)
{
/*
* We probably should optimize dense array property definitions where
@ -2434,7 +2442,7 @@ DefinePropertyArray(JSContext *cx, JSObject *obj, const PropertyDescriptor &desc
if (index >= oldLen && lengthPropertyNotWritable())
return ThrowTypeError(cx, JSMSG_CANT_APPEND_PROPERTIES_TO_UNWRITABLE_LENGTH_ARRAY);
*/
if (!DefinePropertyObject(cx, obj, desc, false, rval))
if (!DefinePropertyOnObject(cx, obj, desc, false, rval))
return JS_FALSE;
if (!*rval)
return Reject(cx, JSMSG_CANT_DEFINE_ARRAY_INDEX, throwError, rval);
@ -2448,7 +2456,7 @@ DefinePropertyArray(JSContext *cx, JSObject *obj, const PropertyDescriptor &desc
return JS_TRUE;
}
return DefinePropertyObject(cx, obj, desc, throwError, rval);
return DefinePropertyOnObject(cx, obj, desc, throwError, rval);
}
static JSBool
@ -2456,12 +2464,12 @@ DefineProperty(JSContext *cx, JSObject *obj, const PropertyDescriptor &desc, boo
bool *rval)
{
if (obj->isArray())
return DefinePropertyArray(cx, obj, desc, throwError, rval);
return DefinePropertyOnArray(cx, obj, desc, throwError, rval);
if (obj->map->ops->lookupProperty != js_LookupProperty)
return Reject(cx, JSMSG_OBJECT_NOT_EXTENSIBLE, throwError, rval);
return DefinePropertyObject(cx, obj, desc, throwError, rval);
return DefinePropertyOnObject(cx, obj, desc, throwError, rval);
}
JSBool
@ -4195,9 +4203,9 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value,
/* Use the object's class getter and setter by default. */
clasp = obj->getClass();
if (!(defineHow & JSDNP_SET_METHOD)) {
if (!getter)
if (!getter && !(attrs & JSPROP_GETTER))
getter = clasp->getProperty;
if (!setter)
if (!setter && !(attrs & JSPROP_SETTER))
setter = clasp->setProperty;
}
@ -4218,7 +4226,7 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value,
JSObject *funobj = JSVAL_TO_OBJECT(value);
if (FUN_OBJECT(GET_FUNCTION_PRIVATE(cx, funobj)) == funobj) {
flags |= JSScopeProperty::METHOD;
getter = js_CastAsPropertyOp(funobj);
getter = CastAsPropertyOp(funobj);
}
}
@ -5095,7 +5103,7 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow,
JSObject *funobj = JSVAL_TO_OBJECT(*vp);
if (FUN_OBJECT(GET_FUNCTION_PRIVATE(cx, funobj)) == funobj) {
flags |= JSScopeProperty::METHOD;
getter = js_CastAsPropertyOp(funobj);
getter = CastAsPropertyOp(funobj);
}
}

View File

@ -97,10 +97,10 @@ struct PropertyDescriptor {
}
JSObject* getterObject() const {
return get != JSVAL_VOID ? JSVAL_TO_OBJECT(get) : NULL;
return (get != JSVAL_VOID) ? JSVAL_TO_OBJECT(get) : NULL;
}
JSObject* setterObject() const {
return set != JSVAL_VOID ? JSVAL_TO_OBJECT(set) : NULL;
return (set != JSVAL_VOID) ? JSVAL_TO_OBJECT(set) : NULL;
}
jsval getterValue() const {
@ -111,10 +111,10 @@ struct PropertyDescriptor {
}
JSPropertyOp getter() const {
return js_CastAsPropertyOp(getterObject());
return js::CastAsPropertyOp(getterObject());
}
JSPropertyOp setter() const {
return js_CastAsPropertyOp(setterObject());
return js::CastAsPropertyOp(setterObject());
}
static void traceDescriptorArray(JSTracer* trc, JSObject* obj);

View File

@ -2933,9 +2933,9 @@ BEGIN_CASE(JSOP_DEFFUN)
attrs |= flags | JSPROP_SHARED;
rval = JSVAL_VOID;
if (flags == JSPROP_GETTER)
getter = js_CastAsPropertyOp(obj);
getter = CastAsPropertyOp(obj);
else
setter = js_CastAsPropertyOp(obj);
setter = CastAsPropertyOp(obj);
}
/*
@ -3033,10 +3033,10 @@ BEGIN_CASE(JSOP_DEFFUN_DBGFC)
ok = parent->defineProperty(cx, id, rval,
(flags & JSPROP_GETTER)
? js_CastAsPropertyOp(obj)
? CastAsPropertyOp(obj)
: JS_PropertyStub,
(flags & JSPROP_SETTER)
? js_CastAsPropertyOp(obj)
? CastAsPropertyOp(obj)
: JS_PropertyStub,
attrs);
}
@ -3259,12 +3259,12 @@ BEGIN_CASE(JSOP_SETTER)
goto error;
if (op == JSOP_GETTER) {
getter = js_CastAsPropertyOp(JSVAL_TO_OBJECT(rval));
getter = CastAsPropertyOp(JSVAL_TO_OBJECT(rval));
setter = JS_PropertyStub;
attrs = JSPROP_GETTER;
} else {
getter = JS_PropertyStub;
setter = js_CastAsPropertyOp(JSVAL_TO_OBJECT(rval));
setter = CastAsPropertyOp(JSVAL_TO_OBJECT(rval));
attrs = JSPROP_SETTER;
}
attrs |= JSPROP_ENUMERATE | JSPROP_SHARED;

View File

@ -91,10 +91,12 @@ typedef uint32 jsatomid;
#ifdef __cplusplus
/* Class and struct forward declarations in namespace js. */
extern "C++" {
namespace js {
struct Parser;
struct Compiler;
}
}
#endif
@ -184,17 +186,18 @@ class DeflatedStringCache;
class PropertyCache;
struct PropertyCacheEntry;
static inline JSPropertyOp
CastAsPropertyOp(JSObject *object)
{
return JS_DATA_TO_FUNC_PTR(JSPropertyOp, object);
}
} /* namespace js */
/* Common instantiations. */
typedef js::Vector<jschar, 32> JSCharBuffer;
static inline JSPropertyOp
js_CastAsPropertyOp(JSObject *object)
{
return JS_DATA_TO_FUNC_PTR(JSPropertyOp, object);
}
} /* export "C++" */
#endif /* __cplusplus */

View File

@ -768,16 +768,20 @@ NormalizeGetterAndSetter(JSContext *cx, JSScope *scope,
JSPropertyOp &getter,
JSPropertyOp &setter)
{
if (setter == JS_PropertyStub)
if (setter == JS_PropertyStub) {
JS_ASSERT(!(attrs & JSPROP_SETTER));
setter = NULL;
}
if (flags & JSScopeProperty::METHOD) {
/* Here, getter is the method, a function object reference. */
JS_ASSERT(getter);
JS_ASSERT(!setter || setter == js_watch_set);
JS_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER)));
} else {
if (getter == JS_PropertyStub)
if (getter == JS_PropertyStub) {
JS_ASSERT(!(attrs & JSPROP_GETTER));
getter = NULL;
}
}
/*

View File

@ -575,21 +575,23 @@ JSObject::lockedSetSlot(uintN slot, jsval value)
* Helpers for reinterpreting JSPropertyOp as JSObject* for scripted getters
* and setters.
*/
namespace js {
inline JSObject *
js_CastAsObject(JSPropertyOp op)
CastAsObject(JSPropertyOp op)
{
return JS_FUNC_TO_DATA_PTR(JSObject *, op);
}
inline jsval
js_CastAsObjectJSVal(JSPropertyOp op)
CastAsObjectJSVal(JSPropertyOp op)
{
return OBJECT_TO_JSVAL(JS_FUNC_TO_DATA_PTR(JSObject *, op));
}
namespace js {
class PropertyTree;
}
} /* namespace js */
struct JSScopeProperty {
friend struct JSScope;
@ -602,13 +604,17 @@ struct JSScopeProperty {
private:
union {
JSPropertyOp rawGetter; /* getter and setter hooks or objects */
JSPropertyOp rawGetter; /* getter and setter hooks or objects */
JSObject *getterObj; /* user-defined callable "get" object or
null if sprop->hasGetterValue() */
JSScopeProperty *next; /* next node in freelist */
};
union {
JSPropertyOp rawSetter; /* getter is JSObject* and setter is 0
JSPropertyOp rawSetter; /* getter is JSObject* and setter is 0
if sprop->isMethod() */
JSObject *setterObj; /* user-defined callable "set" object or
null if sprop->hasSetterValue() */
JSScopeProperty **prevp; /* pointer to previous node's next, or
pointer to head of freelist */
};
@ -673,10 +679,8 @@ struct JSScopeProperty {
: id(id), rawGetter(getter), rawSetter(setter), slot(slot), attrs(uint8(attrs)),
flags(uint8(flags)), shortid(int16(shortid))
{
JS_ASSERT_IF(getter && (attrs & JSPROP_GETTER),
JSVAL_TO_OBJECT(getterValue())->isCallable());
JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER),
JSVAL_TO_OBJECT(setterValue())->isCallable());
JS_ASSERT_IF(getter && (attrs & JSPROP_GETTER), getterObj->isCallable());
JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER), setterObj->isCallable());
}
bool marked() const { return (flags & MARK) != 0; }
@ -698,48 +702,34 @@ struct JSScopeProperty {
PUBLIC_FLAGS = ALIAS | HAS_SHORTID | METHOD
};
uintN getFlags() const { return flags & PUBLIC_FLAGS; }
bool isAlias() const { return (flags & ALIAS) != 0; }
uintN getFlags() const { return flags & PUBLIC_FLAGS; }
bool isAlias() const { return (flags & ALIAS) != 0; }
bool hasShortID() const { return (flags & HAS_SHORTID) != 0; }
bool isMethod() const { return (flags & METHOD) != 0; }
bool isMethod() const { return (flags & METHOD) != 0; }
JSObject *methodObject() const {
JS_ASSERT(isMethod());
return js_CastAsObject(rawGetter);
}
jsval methodValue() const {
JS_ASSERT(isMethod());
return js_CastAsObjectJSVal(rawGetter);
}
JSObject *methodObject() const { JS_ASSERT(isMethod()); return getterObj; }
jsval methodValue() const { return OBJECT_TO_JSVAL(methodObject()); }
JSPropertyOp getter() const { return rawGetter; }
bool hasDefaultGetter() const { return !rawGetter; }
JSPropertyOp getterOp() const {
JS_ASSERT(!hasGetterValue());
return rawGetter;
}
JSObject *getterObject() const {
JS_ASSERT(hasGetterValue());
return js_CastAsObject(rawGetter);
}
JSPropertyOp getter() const { return rawGetter; }
bool hasDefaultGetter() const { return !rawGetter; }
JSPropertyOp getterOp() const { JS_ASSERT(!hasGetterValue()); return rawGetter; }
JSObject *getterObject() const { JS_ASSERT(hasGetterValue()); return getterObj; }
// Per ES5, decode null getterObj as the undefined value, which encodes as null.
jsval getterValue() const {
JS_ASSERT(hasGetterValue());
return rawGetter ? js_CastAsObjectJSVal(rawGetter) : JSVAL_VOID;
return getterObj ? OBJECT_TO_JSVAL(getterObj) : JSVAL_VOID;
}
JSPropertyOp setter() const { return rawSetter; }
bool hasDefaultSetter() const { return !rawSetter; }
JSPropertyOp setterOp() const {
JS_ASSERT(!hasSetterValue());
return rawSetter;
}
JSObject *setterObject() const {
JS_ASSERT(hasSetterValue() && rawSetter);
return js_CastAsObject(rawSetter);
}
JSPropertyOp setter() const { return rawSetter; }
bool hasDefaultSetter() const { return !rawSetter; }
JSPropertyOp setterOp() const { JS_ASSERT(!hasSetterValue()); return rawSetter; }
JSObject *setterObject() const { JS_ASSERT(hasSetterValue()); return setterObj; }
// Per ES5, decode null setterObj as the undefined value, which encodes as null.
jsval setterValue() const {
JS_ASSERT(hasSetterValue());
return rawSetter ? js_CastAsObjectJSVal(rawSetter) : JSVAL_VOID;
return setterObj ? OBJECT_TO_JSVAL(setterObj) : JSVAL_VOID;
}
inline JSDHashNumber hash() const;