From 3414c5c5ffc42bf56c65767bf5c7e9fdaf2646c3 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Fri, 21 Nov 2014 21:07:13 -0600 Subject: [PATCH] Bug 1103368, part 2 - Ban stub getter/setter arguments to js::baseops::Define{Property,Generic,Element}, DefineNativeProperty, and DefinePropertyOrElement. r=bhackett. --HG-- extra : rebase_source : 1b8524aebf85aa27fa9542b8001cae489888e887 --- js/src/frontend/BytecodeEmitter.cpp | 8 +++- js/src/jsdate.cpp | 2 +- js/src/jsfun.cpp | 2 +- js/src/jsnum.cpp | 6 +-- js/src/jsobj.cpp | 39 ++++++++++++++----- js/src/json.cpp | 5 +-- js/src/vm/ArgumentsObject.cpp | 18 ++++----- js/src/vm/Debugger.cpp | 6 +-- js/src/vm/JSONParser.cpp | 4 +- js/src/vm/NativeObject-inl.h | 3 ++ js/src/vm/NativeObject.cpp | 60 +++++++++++++---------------- js/src/vm/RegExpObject.cpp | 12 ++++-- 12 files changed, 88 insertions(+), 77 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 4bf0429c09b..214f0c682e2 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -2275,10 +2275,14 @@ IteratorResultShape(ExclusiveContext *cx, BytecodeEmitter *bce, unsigned *shape) Rooted done_id(cx, AtomToId(cx->names().done)); if (!DefineNativeProperty(cx, obj, value_id, UndefinedHandleValue, nullptr, nullptr, JSPROP_ENUMERATE)) + { return false; + } if (!DefineNativeProperty(cx, obj, done_id, UndefinedHandleValue, nullptr, nullptr, JSPROP_ENUMERATE)) + { return false; + } ObjectBox *objbox = bce->parser->newObjectBox(obj); if (!objbox) @@ -6574,8 +6578,8 @@ EmitObject(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn) MOZ_ASSERT(!obj->inDictionaryMode()); Rooted id(cx, AtomToId(key->pn_atom)); RootedValue undefinedValue(cx, UndefinedValue()); - if (!DefineNativeProperty(cx, obj, id, undefinedValue, nullptr, - nullptr, JSPROP_ENUMERATE)) + if (!DefineNativeProperty(cx, obj, id, undefinedValue, nullptr, nullptr, + JSPROP_ENUMERATE)) { return false; } diff --git a/js/src/jsdate.cpp b/js/src/jsdate.cpp index d24d03096ec..59e38c4b933 100644 --- a/js/src/jsdate.cpp +++ b/js/src/jsdate.cpp @@ -3013,7 +3013,7 @@ FinishDateClassInit(JSContext *cx, HandleObject ctor, HandleObject proto) RootedId toGMTStringId(cx, NameToId(cx->names().toGMTString)); return baseops::GetProperty(cx, proto.as(), toUTCStringId, &toUTCStringFun) && baseops::DefineGeneric(cx, proto.as(), toGMTStringId, toUTCStringFun, - JS_PropertyStub, JS_StrictPropertyStub, 0); + nullptr, nullptr, 0); } const Class DateObject::class_ = { diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index f376aa6308a..9cca5a5df4e 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -499,7 +499,7 @@ js::fun_resolve(JSContext *cx, HandleObject obj, HandleId id, bool *resolvedp) v.setString(fun->atom() == nullptr ? cx->runtime()->emptyString : fun->atom()); } - if (!DefineNativeProperty(cx, fun, id, v, JS_PropertyStub, JS_StrictPropertyStub, + if (!DefineNativeProperty(cx, fun, id, v, nullptr, nullptr, JSPROP_PERMANENT | JSPROP_READONLY)) { return false; } diff --git a/js/src/jsnum.cpp b/js/src/jsnum.cpp index 4a14d34339c..148e509a525 100644 --- a/js/src/jsnum.cpp +++ b/js/src/jsnum.cpp @@ -1212,11 +1212,9 @@ js_InitNumberClass(JSContext *cx, HandleObject obj) RootedValue valueInfinity(cx, cx->runtime()->positiveInfinityValue); /* ES5 15.1.1.1, 15.1.1.2 */ - if (!DefineNativeProperty(cx, global, cx->names().NaN, valueNaN, - JS_PropertyStub, JS_StrictPropertyStub, + if (!DefineNativeProperty(cx, global, cx->names().NaN, valueNaN, nullptr, nullptr, JSPROP_PERMANENT | JSPROP_READONLY) || - !DefineNativeProperty(cx, global, cx->names().Infinity, valueInfinity, - JS_PropertyStub, JS_StrictPropertyStub, + !DefineNativeProperty(cx, global, cx->names().Infinity, valueInfinity, nullptr, nullptr, JSPROP_PERMANENT | JSPROP_READONLY)) { return nullptr; diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 2846f6c122b..3b6f8e33507 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -613,9 +613,7 @@ DefinePropertyOnObject(JSContext *cx, HandleNativeObject obj, HandleId id, const if (desc.isGenericDescriptor() || desc.isDataDescriptor()) { MOZ_ASSERT(!obj->getOps()->defineProperty); RootedValue v(cx, desc.hasValue() ? desc.value() : UndefinedValue()); - return baseops::DefineGeneric(cx, obj, id, v, - JS_PropertyStub, JS_StrictPropertyStub, - desc.attributes()); + return baseops::DefineGeneric(cx, obj, id, v, nullptr, nullptr, desc.attributes()); } MOZ_ASSERT(desc.isAccessorDescriptor()); @@ -822,8 +820,8 @@ DefinePropertyOnObject(JSContext *cx, HandleNativeObject obj, HandleId id, const changed |= JSPROP_ENUMERATE; attrs = (shapeAttributes & ~changed) | (desc.attributes() & changed); - getter = IsImplicitDenseOrTypedArrayElement(shape) ? JS_PropertyStub : shape->getter(); - setter = IsImplicitDenseOrTypedArrayElement(shape) ? JS_StrictPropertyStub : shape->setter(); + getter = IsImplicitDenseOrTypedArrayElement(shape) ? nullptr : shape->getter(); + setter = IsImplicitDenseOrTypedArrayElement(shape) ? nullptr : shape->setter(); } else if (desc.isDataDescriptor()) { unsigned unchanged = 0; if (!desc.hasConfigurable()) @@ -837,8 +835,8 @@ DefinePropertyOnObject(JSContext *cx, HandleNativeObject obj, HandleId id, const if (desc.hasValue()) v = desc.value(); attrs = (desc.attributes() & ~unchanged) | (shapeAttributes & unchanged); - getter = JS_PropertyStub; - setter = JS_StrictPropertyStub; + getter = nullptr; + setter = nullptr; } else { MOZ_ASSERT(desc.isAccessorDescriptor()); @@ -858,14 +856,14 @@ DefinePropertyOnObject(JSContext *cx, HandleNativeObject obj, HandleId id, const getter = desc.getter(); } else { getter = (shapeHasDefaultGetter && !shapeHasGetterValue) - ? JS_PropertyStub + ? nullptr : shape->getter(); } if (desc.hasSet()) { setter = desc.setter(); } else { setter = (shapeHasDefaultSetter && !shapeHasSetterValue) - ? JS_StrictPropertyStub + ? nullptr : shape->setter(); } } @@ -2119,8 +2117,11 @@ js::XDRObjectLiteral(XDRState *xdr, MutableHandleNativeObject obj) return false; if (mode == XDR_DECODE) { - if (!DefineNativeProperty(cx, obj, id, tmpValue, NULL, NULL, JSPROP_ENUMERATE)) + if (!DefineNativeProperty(cx, obj, id, tmpValue, nullptr, nullptr, + JSPROP_ENUMERATE)) + { return false; + } } } @@ -2893,6 +2894,15 @@ JSObject::defineGeneric(ExclusiveContext *cx, HandleObject obj, return false; return op(cx->asJSContext(), obj, id, value, getter, setter, attrs); } + + if (getter == nullptr) + getter = obj->getClass()->getProperty; + if (setter == nullptr) + setter = obj->getClass()->setProperty; + if (getter == JS_PropertyStub) + getter = nullptr; + if (setter == JS_StrictPropertyStub) + setter = nullptr; return baseops::DefineGeneric(cx, obj.as(), id, value, getter, setter, attrs); } @@ -2916,6 +2926,15 @@ JSObject::defineElement(ExclusiveContext *cx, HandleObject obj, return false; return op(cx->asJSContext(), obj, index, value, getter, setter, attrs); } + + if (!getter) + getter = obj->getClass()->getProperty; + if (!setter) + setter = obj->getClass()->setProperty; + if (getter == JS_PropertyStub) + getter = nullptr; + if (setter == JS_StrictPropertyStub) + setter = nullptr; return baseops::DefineElement(cx, obj.as(), index, value, getter, setter, attrs); } diff --git a/js/src/json.cpp b/js/src/json.cpp index f34882cab1a..76512761c08 100644 --- a/js/src/json.cpp +++ b/js/src/json.cpp @@ -668,11 +668,8 @@ js_Stringify(JSContext *cx, MutableHandleValue vp, JSObject *replacer_, Value sp /* Step 10. */ RootedId emptyId(cx, NameToId(cx->names().empty)); - if (!DefineNativeProperty(cx, wrapper, emptyId, vp, JS_PropertyStub, JS_StrictPropertyStub, - JSPROP_ENUMERATE)) - { + if (!DefineNativeProperty(cx, wrapper, emptyId, vp, nullptr, nullptr, JSPROP_ENUMERATE)) return false; - } /* Step 11. */ StringifyContext scx(cx, sb, gap, replacer, propertyList); diff --git a/js/src/vm/ArgumentsObject.cpp b/js/src/vm/ArgumentsObject.cpp index e95c64d26aa..8a459cfb0a8 100644 --- a/js/src/vm/ArgumentsObject.cpp +++ b/js/src/vm/ArgumentsObject.cpp @@ -346,12 +346,11 @@ ArgSetter(JSContext *cx, HandleObject obj, HandleId id, bool strict, MutableHand } /* - * For simplicity we use delete/define to replace the property with one - * backed by the default Object getter and setter. Note that we rely on - * args_delProperty to clear the corresponding reserved slot so the GC can - * collect its value. Note also that we must define the property instead - * of setting it in case the user has changed the prototype to an object - * that has a setter for this id. + * For simplicity we use delete/define to replace the property with a + * simple data property. Note that we rely on args_delProperty to clear the + * corresponding reserved slot so the GC can collect its value. Note also + * that we must define the property instead of setting it in case the user + * has changed the prototype to an object that has a setter for this id. */ bool succeeded; return baseops::DeleteGeneric(cx, argsobj, id, &succeeded) && @@ -462,10 +461,9 @@ StrictArgSetter(JSContext *cx, HandleObject obj, HandleId id, bool strict, Mutab } /* - * For simplicity we use delete/define to replace the property with one - * backed by the default Object getter and setter. Note that we rely on - * args_delProperty to clear the corresponding reserved slot so the GC can - * collect its value. + * For simplicity we use delete/define to replace the property with a + * simple data property. Note that we rely on args_delProperty to clear the + * corresponding reserved slot so the GC can collect its value. */ bool succeeded; return baseops::DeleteGeneric(cx, argsobj, id, &succeeded) && diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index a3e851012a1..961d0aebb86 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -1053,8 +1053,7 @@ Debugger::newCompletionValue(JSContext *cx, JSTrapStatus status, Value value_, RootedPlainObject obj(cx, NewBuiltinClassInstance(cx)); if (!obj || !wrapDebuggeeValue(cx, &value) || - !DefineNativeProperty(cx, obj, key, value, JS_PropertyStub, JS_StrictPropertyStub, - JSPROP_ENUMERATE)) + !DefineNativeProperty(cx, obj, key, value, nullptr, nullptr, JSPROP_ENUMERATE)) { return false; } @@ -5426,8 +5425,7 @@ DebuggerFrame_getArguments(JSContext *cx, unsigned argc, Value *vp) MOZ_ASSERT(frame.numActualArgs() <= 0x7fffffff); unsigned fargc = frame.numActualArgs(); RootedValue fargcVal(cx, Int32Value(fargc)); - if (!DefineNativeProperty(cx, argsobj, cx->names().length, - fargcVal, nullptr, nullptr, + if (!DefineNativeProperty(cx, argsobj, cx->names().length, fargcVal, nullptr, nullptr, JSPROP_PERMANENT | JSPROP_READONLY)) { return false; diff --git a/js/src/vm/JSONParser.cpp b/js/src/vm/JSONParser.cpp index 3a9b487f62f..470f377fb9c 100644 --- a/js/src/vm/JSONParser.cpp +++ b/js/src/vm/JSONParser.cpp @@ -606,10 +606,8 @@ JSONParserBase::createFinishedObject(PropertyVector &properties) for (size_t i = 0; i < properties.length(); i++) { propid = properties[i].id; value = properties[i].value; - if (!DefineNativeProperty(cx, obj, propid, value, JS_PropertyStub, JS_StrictPropertyStub, - JSPROP_ENUMERATE)) { + if (!DefineNativeProperty(cx, obj, propid, value, nullptr, nullptr, JSPROP_ENUMERATE)) return nullptr; - } } /* diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index f8df90330c7..52d50f1dad7 100644 --- a/js/src/vm/NativeObject-inl.h +++ b/js/src/vm/NativeObject-inl.h @@ -624,6 +624,9 @@ DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, PropertyName *name, HandleValue value, PropertyOp getter, StrictPropertyOp setter, unsigned attrs) { + MOZ_ASSERT(getter != JS_PropertyStub); + MOZ_ASSERT(setter != JS_StrictPropertyStub); + RootedId id(cx, NameToId(name)); return DefineNativeProperty(cx, obj, id, value, getter, setter, attrs); } diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index ee9d7c124bb..7c7eb735ba9 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -1062,11 +1062,10 @@ NativeObject::addDataProperty(ExclusiveContext *cx, HandlePropertyName name, template static inline bool CallAddPropertyHook(typename ExecutionModeTraits::ExclusiveContextType cxArg, - const Class *clasp, HandleNativeObject obj, HandleShape shape, - HandleValue nominal) + HandleNativeObject obj, HandleShape shape, HandleValue nominal) { - if (clasp->addProperty) { - MOZ_ASSERT(clasp->addProperty != JS_PropertyStub); + if (JSPropertyOp addProperty = obj->getClass()->addProperty) { + MOZ_ASSERT(addProperty != JS_PropertyStub); if (mode == ParallelExecution) return false; @@ -1079,7 +1078,7 @@ CallAddPropertyHook(typename ExecutionModeTraits::ExclusiveContextType cxA RootedValue value(cx, nominal); Rooted id(cx, shape->propid()); - if (!CallJSPropertyOp(cx->asJSContext(), clasp->addProperty, obj, id, &value)) { + if (!CallJSPropertyOp(cx->asJSContext(), addProperty, obj, id, &value)) { obj->removeProperty(cx, shape->propid()); return false; } @@ -1094,8 +1093,7 @@ CallAddPropertyHook(typename ExecutionModeTraits::ExclusiveContextType cxA template static inline bool CallAddPropertyHookDense(typename ExecutionModeTraits::ExclusiveContextType cxArg, - const Class *clasp, HandleNativeObject obj, uint32_t index, - HandleValue nominal) + HandleNativeObject obj, uint32_t index, HandleValue nominal) { /* Inline addProperty for array objects. */ if (obj->is()) { @@ -1114,8 +1112,8 @@ CallAddPropertyHookDense(typename ExecutionModeTraits::ExclusiveContextTyp return true; } - if (clasp->addProperty) { - MOZ_ASSERT(clasp->addProperty != JS_PropertyStub); + if (JSPropertyOp addProperty = obj->getClass()->addProperty) { + MOZ_ASSERT(addProperty != JS_PropertyStub); if (mode == ParallelExecution) return false; @@ -1131,7 +1129,7 @@ CallAddPropertyHookDense(typename ExecutionModeTraits::ExclusiveContextTyp RootedValue value(cx, nominal); Rooted id(cx, INT_TO_JSID(index)); - if (!CallJSPropertyOp(cx->asJSContext(), clasp->addProperty, obj, id, &value)) { + if (!CallJSPropertyOp(cx->asJSContext(), addProperty, obj, id, &value)) { obj->setDenseElementHole(cx, index); return false; } @@ -1197,10 +1195,13 @@ DefinePropertyOrElement(typename ExecutionModeTraits::ExclusiveContextType unsigned attrs, HandleValue value, bool callSetterAfterwards, bool setterIsStrict) { + MOZ_ASSERT(getter != JS_PropertyStub); + MOZ_ASSERT(setter != JS_StrictPropertyStub); + /* Use dense storage for new indexed properties where possible. */ if (JSID_IS_INT(id) && - getter == JS_PropertyStub && - setter == JS_StrictPropertyStub && + !getter && + !setter && attrs == JSPROP_ENUMERATE && (!obj->isIndexed() || !obj->containsPure(id)) && !IsAnyTypedArray(obj)) @@ -1230,7 +1231,7 @@ DefinePropertyOrElement(typename ExecutionModeTraits::ExclusiveContextType } else { obj->setDenseElementWithType(cx->asExclusiveContext(), index, value); } - return CallAddPropertyHookDense(cx, obj->getClass(), obj, index, value); + return CallAddPropertyHookDense(cx, obj, index, value); } } @@ -1261,11 +1262,6 @@ DefinePropertyOrElement(typename ExecutionModeTraits::ExclusiveContextType } AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter); - - if (getter == JS_PropertyStub) - getter = nullptr; - if (setter == JS_StrictPropertyStub) - setter = nullptr; RootedShape shape(cx, NativeObject::putProperty(cx, obj, id, getter, setter, SHAPE_INVALID_SLOT, attrs, 0)); if (!shape) @@ -1293,11 +1289,11 @@ DefinePropertyOrElement(typename ExecutionModeTraits::ExclusiveContextType return false; if (result == NativeObject::ED_OK) { MOZ_ASSERT(!setter); - return CallAddPropertyHookDense(cx, obj->getClass(), obj, index, value); + return CallAddPropertyHookDense(cx, obj, index, value); } } - if (!CallAddPropertyHook(cx, obj->getClass(), obj, shape, value)) + if (!CallAddPropertyHook(cx, obj, shape, value)) return false; if (callSetterAfterwards && setter) { @@ -1426,6 +1422,8 @@ bool js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId id, HandleValue value, PropertyOp getter, StrictPropertyOp setter, unsigned attrs) { + MOZ_ASSERT(getter != JS_PropertyStub); + MOZ_ASSERT(setter != JS_StrictPropertyStub); MOZ_ASSERT(!(attrs & JSPROP_PROPOP_ACCESSORS)); AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter); @@ -1460,10 +1458,6 @@ js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId if (!CheckAccessorRedefinition(cx, obj, shape, getter, setter, id, attrs)) return false; attrs = ApplyOrDefaultAttributes(attrs, shape); - if (getter == JS_PropertyStub) - getter = nullptr; - if (setter == JS_StrictPropertyStub) - setter = nullptr; shape = NativeObject::changeProperty(cx, obj, shape, attrs, JSPROP_GETTER | JSPROP_SETTER, (attrs & JSPROP_GETTER) @@ -1550,13 +1544,6 @@ js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId if (!PurgeScopeChain(cx, obj, id)) return false; - /* Use the object's class getter and setter by default. */ - const Class *clasp = obj->getClass(); - if (!getter && !(attrs & JSPROP_GETTER)) - getter = clasp->getProperty; - if (!setter && !(attrs & JSPROP_SETTER)) - setter = clasp->setProperty; - if (shouldDefine) { // Handle the default cases here. Anyone that wanted to set non-default attributes has // cleared the IGNORE flags by now. Since we can never get here with JSPROP_IGNORE_VALUE @@ -1570,7 +1557,7 @@ js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId JS_ALWAYS_TRUE(UpdateShapeTypeAndValue(cx, obj, shape, updateValue)); - return CallAddPropertyHook(cx, clasp, obj, shape, updateValue); + return CallAddPropertyHook(cx, obj, shape, updateValue); } static bool @@ -2045,8 +2032,13 @@ SetPropertyByDefining(typename ExecutionModeTraits::ContextType cxArg, clasp->getProperty, clasp->setProperty, JSPROP_ENUMERATE); } Rooted nativeReceiver(cxArg, &receiver->as()); - return DefinePropertyOrElement(cxArg, nativeReceiver, id, - clasp->getProperty, clasp->setProperty, + JSPropertyOp getter = clasp->getProperty; + if (getter == JS_PropertyStub) + getter = nullptr; + JSStrictPropertyOp setter = clasp->setProperty; + if (setter == JS_StrictPropertyStub) + setter = nullptr; + return DefinePropertyOrElement(cxArg, nativeReceiver, id, getter, setter, JSPROP_ENUMERATE, v, true, strict); } diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index 473856ee135..7c5704cfcd0 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -717,15 +717,19 @@ RegExpCompartment::createMatchResultTemplateObject(JSContext *cx) /* Set dummy index property */ RootedValue index(cx, Int32Value(0)); - if (!baseops::DefineProperty(cx, templateObject, cx->names().index, index, - JS_PropertyStub, JS_StrictPropertyStub, JSPROP_ENUMERATE)) + if (!baseops::DefineProperty(cx, templateObject, cx->names().index, index, nullptr, nullptr, + JSPROP_ENUMERATE)) + { return matchResultTemplateObject_; // = nullptr + } /* Set dummy input property */ RootedValue inputVal(cx, StringValue(cx->runtime()->emptyString)); - if (!baseops::DefineProperty(cx, templateObject, cx->names().input, inputVal, - JS_PropertyStub, JS_StrictPropertyStub, JSPROP_ENUMERATE)) + if (!baseops::DefineProperty(cx, templateObject, cx->names().input, inputVal, nullptr, nullptr, + JSPROP_ENUMERATE)) + { return matchResultTemplateObject_; // = nullptr + } // Make sure that the properties are in the right slots. DebugOnly shape = templateObject->lastProperty();