diff --git a/js/src/jit-test/tests/basic/redefinedGetter.js b/js/src/jit-test/tests/basic/redefinedGetter.js deleted file mode 100644 index ecda67db27e7..000000000000 --- a/js/src/jit-test/tests/basic/redefinedGetter.js +++ /dev/null @@ -1,24 +0,0 @@ - -function Thing() {} - -function foo(x, expected) { - for (var i = 0; i < 10; i++) - assertEq(x.f, expected); -} - -Object.defineProperty(Thing.prototype, "f", {get: function() { return 2; }, configurable:true}); -foo(new Thing(), 2); -Object.defineProperty(Thing.prototype, "f", {get: function() { return 3; }}); -foo(new Thing(), 3); - -function OtherThing() {} - -function bar(x, expected) { - for (var i = 0; i < 10; i++) - assertEq(x.f, expected); -} - -Object.defineProperty(OtherThing.prototype, "f", {get: function() { return 2; }, configurable:true}); -bar(new OtherThing(), 2); -delete OtherThing.prototype.f; -bar(new OtherThing(), undefined); diff --git a/js/src/jit/BaselineInspector.cpp b/js/src/jit/BaselineInspector.cpp index 402cb9d68a23..c35ba78e5137 100644 --- a/js/src/jit/BaselineInspector.cpp +++ b/js/src/jit/BaselineInspector.cpp @@ -474,7 +474,7 @@ BaselineInspector::templateCallObject() } JSObject * -BaselineInspector::commonGetPropFunction(jsbytecode *pc, JSFunction **commonGetter) +BaselineInspector::commonGetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonGetter) { if (!hasBaselineScript()) return nullptr; @@ -486,6 +486,7 @@ BaselineInspector::commonGetPropFunction(jsbytecode *pc, JSFunction **commonGett stub->isGetProp_CallNativePrototype()) { ICGetPropCallGetter *nstub = static_cast(stub); + *lastProperty = nstub->holderShape(); *commonGetter = nstub->getter(); return nstub->holder(); } @@ -494,7 +495,7 @@ BaselineInspector::commonGetPropFunction(jsbytecode *pc, JSFunction **commonGett } JSObject * -BaselineInspector::commonSetPropFunction(jsbytecode *pc, JSFunction **commonSetter) +BaselineInspector::commonSetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonSetter) { if (!hasBaselineScript()) return nullptr; @@ -503,6 +504,7 @@ BaselineInspector::commonSetPropFunction(jsbytecode *pc, JSFunction **commonSett for (ICStub *stub = entry.firstStub(); stub; stub = stub->next()) { if (stub->isSetProp_CallScripted() || stub->isSetProp_CallNative()) { ICSetPropCallSetter *nstub = static_cast(stub); + *lastProperty = nstub->holderShape(); *commonSetter = nstub->setter(); return nstub->holder(); } diff --git a/js/src/jit/BaselineInspector.h b/js/src/jit/BaselineInspector.h index 8c29fb4e45c3..083c429d950a 100644 --- a/js/src/jit/BaselineInspector.h +++ b/js/src/jit/BaselineInspector.h @@ -117,8 +117,8 @@ class BaselineInspector DeclEnvObject *templateDeclEnvObject(); CallObject *templateCallObject(); - JSObject *commonGetPropFunction(jsbytecode *pc, JSFunction **commonGetter); - JSObject *commonSetPropFunction(jsbytecode *pc, JSFunction **commonSetter); + JSObject *commonGetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonGetter); + JSObject *commonSetPropFunction(jsbytecode *pc, Shape **lastProperty, JSFunction **commonSetter); }; } // namespace jit diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index d92361c0ef79..4f723ec0e74d 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -8366,33 +8366,25 @@ IonBuilder::freezePropertiesForCommonPrototype(types::TemporaryTypeSet *types, P } } -inline bool +inline MDefinition * IonBuilder::testCommonGetterSetter(types::TemporaryTypeSet *types, PropertyName *name, - bool isGetter, JSObject *foundProto, JSFunction *function) + bool isGetter, JSObject *foundProto, Shape *lastProperty) { - types::TypeObjectKey *protoType = types::TypeObjectKey::get(foundProto); - - // The state change done below needs to be done on an object with singleton - // type, as otherwise we won't be able to tell whether it already has the - // correct getter/setter. - if (!protoType->isSingleObject() || protoType->unknownProperties()) - return false; - // Check if all objects being accessed will lookup the name through foundProto. if (!objectsHaveCommonPrototype(types, name, isGetter, foundProto)) - return false; + return nullptr; // We can optimize the getter/setter, so freeze all involved properties to // ensure there isn't a lower shadowing getter or setter installed in the // future. freezePropertiesForCommonPrototype(types, name, foundProto); - // TI doesn't have accurate type information for properties with getters or - // setters, but we can use the state change machinery to watch out for the - // property being redefined on the prototype. - protoType->watchStateChangeForRedefinedProperty(constraints(), name, isGetter, function); - - return true; + // Add a shape guard on the prototype we found the property on. The rest of + // the prototype chain is guarded by TI freezes. Note that a shape guard is + // good enough here, even in the proxy case, because we have ensured there + // are no lookup hooks for this property. + MInstruction *wrapper = constant(ObjectValue(*foundProto)); + return addShapeGuard(wrapper, lastProperty, Bailout_ShapeGuard); } bool @@ -8812,13 +8804,16 @@ IonBuilder::getPropTryCommonGetter(bool *emitted, MDefinition *obj, PropertyName { JS_ASSERT(*emitted == false); + Shape *lastProperty = nullptr; JSFunction *commonGetter = nullptr; - JSObject *foundProto = inspector->commonGetPropFunction(pc, &commonGetter); + JSObject *foundProto = inspector->commonGetPropFunction(pc, &lastProperty, &commonGetter); if (!foundProto) return true; types::TemporaryTypeSet *objTypes = obj->resultTypeSet(); - if (!testCommonGetterSetter(objTypes, name, /* isGetter = */ true, foundProto, commonGetter)) + MDefinition *guard = testCommonGetterSetter(objTypes, name, /* isGetter = */ true, + foundProto, lastProperty); + if (!guard) return true; bool isDOM = objTypes->isDOMClass(); @@ -8829,9 +8824,9 @@ IonBuilder::getPropTryCommonGetter(bool *emitted, MDefinition *obj, PropertyName if (jitinfo->isInSlot) { // We can't use MLoadFixedSlot here because it might not have the // right aliasing behavior; we want to alias DOM setters. - get = MGetDOMMember::New(alloc(), jitinfo, obj); + get = MGetDOMMember::New(alloc(), jitinfo, obj, guard); } else { - get = MGetDOMProperty::New(alloc(), jitinfo, obj); + get = MGetDOMProperty::New(alloc(), jitinfo, obj, guard); } current->add(get); current->push(get); @@ -9230,13 +9225,16 @@ IonBuilder::setPropTryCommonSetter(bool *emitted, MDefinition *obj, { JS_ASSERT(*emitted == false); + Shape *lastProperty = nullptr; JSFunction *commonSetter = nullptr; - JSObject *foundProto = inspector->commonSetPropFunction(pc, &commonSetter); + JSObject *foundProto = inspector->commonSetPropFunction(pc, &lastProperty, &commonSetter); if (!foundProto) return true; types::TemporaryTypeSet *objTypes = obj->resultTypeSet(); - if (!testCommonGetterSetter(objTypes, name, /* isGetter = */ false, foundProto, commonSetter)) + MDefinition *guard = testCommonGetterSetter(objTypes, name, /* isGetter = */ false, + foundProto, lastProperty); + if (!guard) return true; bool isDOM = objTypes->isDOMClass(); diff --git a/js/src/jit/IonBuilder.h b/js/src/jit/IonBuilder.h index c14cc85e29ce..885d03be3103 100644 --- a/js/src/jit/IonBuilder.h +++ b/js/src/jit/IonBuilder.h @@ -763,8 +763,8 @@ class IonBuilder : public MIRGenerator bool isGetter, JSObject *foundProto); void freezePropertiesForCommonPrototype(types::TemporaryTypeSet *types, PropertyName *name, JSObject *foundProto); - bool testCommonGetterSetter(types::TemporaryTypeSet *types, PropertyName *name, - bool isGetter, JSObject *foundProto, JSFunction *function); + MDefinition *testCommonGetterSetter(types::TemporaryTypeSet *types, PropertyName *name, + bool isGetter, JSObject *foundProto, Shape *lastProperty); bool testShouldDOMCall(types::TypeSet *inTypes, JSFunction *func, JSJitInfo::OpType opType); diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index f7ad28072ae5..b32ab6093ddd 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -8489,18 +8489,23 @@ class MSetDOMProperty }; class MGetDOMProperty - : public MUnaryInstruction, + : public MAryInstruction<2>, public ObjectPolicy<0> { const JSJitInfo *info_; protected: - MGetDOMProperty(const JSJitInfo *jitinfo, MDefinition *obj) - : MUnaryInstruction(obj), info_(jitinfo) + MGetDOMProperty(const JSJitInfo *jitinfo, MDefinition *obj, MDefinition *guard) + : info_(jitinfo) { JS_ASSERT(jitinfo); JS_ASSERT(jitinfo->type() == JSJitInfo::Getter); + setOperand(0, obj); + + // Pin the guard as an operand if we want to hoist later + setOperand(1, guard); + // We are movable iff the jitinfo says we can be. if (isDomMovable()) { JS_ASSERT(jitinfo->aliasSet() != JSJitInfo::AliasEverything); @@ -8522,9 +8527,10 @@ class MGetDOMProperty public: INSTRUCTION_HEADER(GetDOMProperty) - static MGetDOMProperty *New(TempAllocator &alloc, const JSJitInfo *info, MDefinition *obj) + static MGetDOMProperty *New(TempAllocator &alloc, const JSJitInfo *info, MDefinition *obj, + MDefinition *guard) { - return new(alloc) MGetDOMProperty(info, obj); + return new(alloc) MGetDOMProperty(info, obj, guard); } const JSJitGetterOp fun() { @@ -8583,17 +8589,18 @@ class MGetDOMProperty class MGetDOMMember : public MGetDOMProperty { // We inherit everything from MGetDOMProperty except our possiblyCalls value - MGetDOMMember(const JSJitInfo *jitinfo, MDefinition *obj) - : MGetDOMProperty(jitinfo, obj) + MGetDOMMember(const JSJitInfo *jitinfo, MDefinition *obj, MDefinition *guard) + : MGetDOMProperty(jitinfo, obj, guard) { } public: INSTRUCTION_HEADER(GetDOMMember) - static MGetDOMMember *New(TempAllocator &alloc, const JSJitInfo *info, MDefinition *obj) + static MGetDOMMember *New(TempAllocator &alloc, const JSJitInfo *info, MDefinition *obj, + MDefinition *guard) { - return new(alloc) MGetDOMMember(info, obj); + return new(alloc) MGetDOMMember(info, obj, guard); } bool possiblyCalls() const { diff --git a/js/src/jsinfer.cpp b/js/src/jsinfer.cpp index 323aab6c2fae..808fdbcda2f9 100644 --- a/js/src/jsinfer.cpp +++ b/js/src/jsinfer.cpp @@ -1627,47 +1627,6 @@ class ConstraintDataFreezeObjectForTypedArrayBuffer } }; -// Constraint which triggers recompilation when a getter or setter property on -// an object is redefined. -class ConstraintDataFreezeObjectForGetterSetter -{ - PropertyName *name; - bool isGetter; - JSFunction *function; - - public: - ConstraintDataFreezeObjectForGetterSetter(PropertyName *name, bool isGetter, JSFunction *function) - : name(name), isGetter(isGetter), function(function) - {} - - const char *kind() { return "freezeObjectForGetterSetter"; } - - bool invalidateOnNewType(Type type) { return false; } - bool invalidateOnNewPropertyState(TypeSet *property) { return false; } - bool invalidateOnNewObjectState(TypeObject *object) { - Shape *shape = object->singleton()->nativeLookupPure(name); - if (!shape) - return true; - - if (isGetter) - return !shape->hasGetterValue() || shape->getterObject() != function; - return !shape->hasSetterValue() || shape->setterObject() != function; - } - - bool constraintHolds(JSContext *cx, - const HeapTypeSetKey &property, TemporaryTypeSet *expected) - { - return !invalidateOnNewObjectState(property.object()->maybeType()); - } - - bool shouldSweep() { - // Note: |name| may be used for looking up properties on the object, - // but if it becomes dead then the property will have been deleted and - // the associated jitcode invalidated. - return false; - } -}; - } /* anonymous namespace */ void @@ -1704,19 +1663,6 @@ TypeObjectKey::watchStateChangeForTypedArrayBuffer(CompilerConstraintList *const ConstraintDataFreezeObjectForTypedArrayBuffer(viewData))); } -void -TypeObjectKey::watchStateChangeForRedefinedProperty(CompilerConstraintList *constraints, - PropertyName *name, bool isGetter, - JSFunction *function) -{ - HeapTypeSetKey objectProperty = property(JSID_EMPTY); - LifoAlloc *alloc = constraints->alloc(); - - typedef CompilerConstraintInstance T; - constraints->add(alloc->new_(alloc, objectProperty, - ConstraintDataFreezeObjectForGetterSetter(name, isGetter, function))); -} - static void ObjectStateChange(ExclusiveContext *cxArg, TypeObject *object, bool markingUnknown) { diff --git a/js/src/jsinfer.h b/js/src/jsinfer.h index 6002df2861ad..fcdbe4ae2a78 100644 --- a/js/src/jsinfer.h +++ b/js/src/jsinfer.h @@ -1424,8 +1424,6 @@ struct TypeObjectKey void watchStateChangeForInlinedCall(CompilerConstraintList *constraints); void watchStateChangeForNewScriptTemplate(CompilerConstraintList *constraints); void watchStateChangeForTypedArrayBuffer(CompilerConstraintList *constraints); - void watchStateChangeForRedefinedProperty(CompilerConstraintList *constraints, - PropertyName *name, bool isGetter, JSFunction *function); HeapTypeSetKey property(jsid id); void ensureTrackedProperty(JSContext *cx, jsid id); diff --git a/js/src/vm/Shape.cpp b/js/src/vm/Shape.cpp index 99ae07b9c7e9..9a03cfb894f9 100644 --- a/js/src/vm/Shape.cpp +++ b/js/src/vm/Shape.cpp @@ -866,10 +866,6 @@ JSObject::putProperty(typename ExecutionModeTraits::ExclusiveContextType c JS_ASSERT_IF(shape->hasSlot() && !(attrs & JSPROP_SHARED), shape->slot() == slot); - bool wasGetterOrSetter = shape->hasGetterValue() || shape->hasSetterValue(); - if (mode == ParallelExecution && wasGetterOrSetter) - return nullptr; - if (obj->inDictionaryMode()) { /* * Updating some property in a dictionary-mode object. Create a new @@ -939,10 +935,6 @@ JSObject::putProperty(typename ExecutionModeTraits::ExclusiveContextType c ++cx->asJSContext()->runtime()->propertyRemovals; } - // Inform type inference about any getter or setter property being changed. - if (wasGetterOrSetter) - types::MarkObjectStateChange(cx->asExclusiveContext(), obj); - obj->checkShapeConsistency(); return shape; @@ -1133,10 +1125,6 @@ JSObject::removeProperty(ExclusiveContext *cx, jsid id_) self->removeLastProperty(cx); } - // Inform type inference about any getter or setter property being deleted. - if (shape->hasGetterValue() || shape->hasSetterValue()) - types::MarkObjectStateChange(cx, self); - self->checkShapeConsistency(); return true; }