From a2432d0b527269ac9a522011e560d87cdef4ae13 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Wed, 2 Jun 2010 16:01:36 -0700 Subject: [PATCH] Back out changeset a72a9d72c028 (bug 559653, remove SetPropHit). Checking to see if this caused a 5% Dromaeo regression today. --HG-- extra : rebase_source : 5b1726c8bc4f9504cb1d452d0d21d59d61091ee0 --- js/src/jsdbgapi.cpp | 4 +- js/src/jsdbgapi.h | 2 +- js/src/jsobj.cpp | 30 +- js/src/jsops.cpp | 4 +- js/src/jsscope.cpp | 25 +- js/src/jsscope.h | 46 +- js/src/jstracer.cpp | 471 +++++------------- js/src/jstracer.h | 40 +- .../tests/basic/testMethodWriteBarrier.js | 4 - .../tests/basic/testMethodWriteBarrier2.js | 17 - .../tests/basic/testMethodWriteBarrier3.js | 27 - 11 files changed, 200 insertions(+), 470 deletions(-) delete mode 100644 js/src/trace-test/tests/basic/testMethodWriteBarrier.js delete mode 100644 js/src/trace-test/tests/basic/testMethodWriteBarrier2.js delete mode 100644 js/src/trace-test/tests/basic/testMethodWriteBarrier3.js diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 4689854503ca..e56880d7539a 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -544,7 +544,7 @@ js_SweepWatchPoints(JSContext *cx) * NB: FindWatchPoint must be called with rt->debuggerLock acquired. */ static JSWatchPoint * -FindWatchPoint(JSRuntime *rt, const JSScope *scope, jsid id) +FindWatchPoint(JSRuntime *rt, JSScope *scope, jsid id) { JSWatchPoint *wp; @@ -558,7 +558,7 @@ FindWatchPoint(JSRuntime *rt, const JSScope *scope, jsid id) } JSScopeProperty * -js_FindWatchPoint(JSRuntime *rt, const JSScope *scope, jsid id) +js_FindWatchPoint(JSRuntime *rt, JSScope *scope, jsid id) { JSWatchPoint *wp; JSScopeProperty *sprop; diff --git a/js/src/jsdbgapi.h b/js/src/jsdbgapi.h index ec32a29d6330..ee3faf20a033 100644 --- a/js/src/jsdbgapi.h +++ b/js/src/jsdbgapi.h @@ -112,7 +112,7 @@ extern void js_SweepWatchPoints(JSContext *cx); extern JSScopeProperty * -js_FindWatchPoint(JSRuntime *rt, const JSScope *scope, jsid id); +js_FindWatchPoint(JSRuntime *rt, JSScope *scope, jsid id); /* * NB: callers outside of jsdbgapi.c must pass non-null scope. diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 34a0dbe9f0ac..ad27b830c7c5 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -4308,7 +4308,9 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, if (defineHow & JSDNP_CACHE_RESULT) { JS_ASSERT_NOT_ON_TRACE(cx); - JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, 0, obj, sprop, added); + PropertyCacheEntry *entry = + JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, 0, obj, sprop, added); + TRACE_2(SetPropHit, entry, sprop); } if (propp) *propp = (JSProperty *) sprop; @@ -4962,6 +4964,11 @@ ReportReadOnly(JSContext* cx, jsid id, uintN flags) } +/* + * Note: all non-error exits in this function must notify the tracer using + * SetPropHit when called from the interpreter, which is detected by testing + * (defineHow & JSDNP_CACHE_RESULT). + */ JSBool js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, jsval *vp) @@ -5040,6 +5047,8 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, if (sprop->isAccessorDescriptor()) { if (sprop->hasDefaultSetter()) { JS_UNLOCK_SCOPE(cx, scope); + if (defineHow & JSDNP_CACHE_RESULT) + TRACE_2(SetPropHit, JS_NO_PROP_CACHE_FILL, sprop); return js_ReportGetterOnlyAssignment(cx); } } else { @@ -5049,11 +5058,20 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, JS_UNLOCK_SCOPE(cx, scope); PCMETER((defineHow & JSDNP_CACHE_RESULT) && JS_PROPERTY_CACHE(cx).rofills++); + if (defineHow & JSDNP_CACHE_RESULT) { + JS_ASSERT_NOT_ON_TRACE(cx); + TRACE_2(SetPropHit, JS_NO_PROP_CACHE_FILL, sprop); + } /* Warn in strict mode, otherwise do nothing. */ if (JS_HAS_STRICT_OPTION(cx)) return ReportReadOnly(cx, id, JSREPORT_STRICT | JSREPORT_WARNING); return JS_TRUE; + +#ifdef JS_TRACER + error: // TRACE_2 jumps here in case of error. + return JS_FALSE; +#endif } } if (scope->sealed() && !sprop->hasSlot()) { @@ -5076,7 +5094,9 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, if (!sprop->hasSlot()) { if (defineHow & JSDNP_CACHE_RESULT) { JS_ASSERT_NOT_ON_TRACE(cx); - JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, protoIndex, pobj, sprop); + PropertyCacheEntry *entry = + JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, protoIndex, pobj, sprop); + TRACE_2(SetPropHit, entry, sprop); } if (sprop->hasDefaultSetter() && !sprop->hasGetterValue()) @@ -5134,7 +5154,8 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, * Check for Object class here to avoid defining a method on a class * with magic resolve, addProperty, getProperty, etc. hooks. */ - if ((defineHow & JSDNP_SET_METHOD) && obj->getClass() == &js_ObjectClass) { + if ((defineHow & JSDNP_SET_METHOD) && + obj->getClass() == &js_ObjectClass) { JS_ASSERT(VALUE_IS_FUNCTION(cx, *vp)); JS_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER))); @@ -5171,7 +5192,8 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, uintN defineHow, if (defineHow & JSDNP_CACHE_RESULT) { JS_ASSERT_NOT_ON_TRACE(cx); - JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, 0, obj, sprop, added); + PropertyCacheEntry *entry = JS_PROPERTY_CACHE(cx).fill(cx, obj, 0, 0, obj, sprop, added); + TRACE_2(SetPropHit, entry, sprop); } if (!js_NativeSet(cx, obj, sprop, added, vp)) diff --git a/js/src/jsops.cpp b/js/src/jsops.cpp index 434c032ea6dc..18660e434a7b 100644 --- a/js/src/jsops.cpp +++ b/js/src/jsops.cpp @@ -619,6 +619,7 @@ END_CASE(JSOP_PICK) #define NATIVE_SET(cx,obj,sprop,entry,vp) \ JS_BEGIN_MACRO \ + TRACE_2(SetPropHit, entry, sprop); \ if (sprop->hasDefaultSetter() && \ (sprop)->slot != SPROP_INVALID_SLOT && \ !(obj)->scope()->brandedOrHasMethodBarrier()) { \ @@ -1780,6 +1781,7 @@ BEGIN_CASE(JSOP_SETMETHOD) * slot's value that might contain a method of a * branded scope. */ + TRACE_2(SetPropHit, entry, sprop); obj->lockedSetSlot(slot, rval); /* @@ -3248,7 +3250,6 @@ BEGIN_CASE(JSOP_INITMETHOD) lval = FETCH_OPND(-2); obj = JSVAL_TO_OBJECT(lval); JS_ASSERT(obj->isNative()); - JS_ASSERT(obj->getClass() == &js_ObjectClass); JS_ASSERT(!obj->getClass()->reserveSlots); JSScope *scope = obj->scope(); @@ -3302,6 +3303,7 @@ BEGIN_CASE(JSOP_INITMETHOD) * property, not updating an existing slot's value that might * contain a method of a branded scope. */ + TRACE_2(SetPropHit, entry, sprop); obj->lockedSetSlot(slot, rval); } else { PCMETER(JS_PROPERTY_CACHE(cx).inipcmisses++); diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index 4cd42b9aab08..46317b5ff253 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -787,7 +787,7 @@ JSScope::addProperty(JSContext *cx, jsid id, * SPROP_CALL_[GS]ETTER macros. */ static inline bool -NormalizeGetterAndSetter(JSContext *cx, const JSScope *scope, +NormalizeGetterAndSetter(JSContext *cx, JSScope *scope, jsid id, uintN attrs, uintN flags, JSPropertyOp &getter, JSPropertyOp &setter) @@ -883,29 +883,6 @@ JSScope::addPropertyHelper(JSContext *cx, jsid id, return NULL; } -JSScopeProperty * -JSScope::prepareForAddProperty(JSContext *cx, jsid id, JSPropertyOp getter, JSPropertyOp setter, - uint32 slot, uintN attrs, uintN flags, intN shortid) const -{ - NormalizeGetterAndSetter(cx, this, id, attrs, flags, getter, setter); - - // Find or create a property tree node labeled by our arguments. Do not use - // getChildProperty because it also effectively adds the property to this - // JSScope. - if (inDictionaryMode()) { - JSScopeProperty *sprop = JS_PROPERTY_TREE(cx).newScopeProperty(cx, true); - if (sprop) { - new (sprop) JSScopeProperty(id, getter, setter, slot, attrs, flags, shortid); - sprop->parent = NULL; - sprop->kids = NULL; - sprop->shape = JSObjectMap::SHAPELESS; - } - return sprop; - } - JSScopeProperty child(id, getter, setter, slot, attrs, flags, shortid); - return JS_PROPERTY_TREE(cx).getChild(cx, lastProp, shape, child); -} - JSScopeProperty * JSScope::putProperty(JSContext *cx, jsid id, JSPropertyOp getter, JSPropertyOp setter, diff --git a/js/src/jsscope.h b/js/src/jsscope.h index 124cc78876f2..d9a467f2a0d8 100644 --- a/js/src/jsscope.h +++ b/js/src/jsscope.h @@ -279,8 +279,8 @@ struct JSScope : public JSObjectMap bool changeTable(JSContext *cx, int change); void reportReadOnlyScope(JSContext *cx); - void setOwnShape() { flags |= OWN_SHAPE; } - void clearOwnShape() { flags &= ~OWN_SHAPE; } + void setOwnShape() { flags |= OWN_SHAPE; } + void clearOwnShape() { flags &= ~OWN_SHAPE; } void generateOwnShape(JSContext *cx); JSScopeProperty **searchTable(jsid id, bool adding); @@ -327,17 +327,6 @@ struct JSScope : public JSObjectMap uint32 slot, uintN attrs, uintN flags, intN shortid); - /* - * Like addProperty, but just return the new (or cached) JSScopeProperty - * without adding it. This is for use with js_AddProperty. - * - * Note that the return value is not necessarily rooted. - */ - JSScopeProperty *prepareForAddProperty(JSContext *cx, jsid id, - JSPropertyOp getter, JSPropertyOp setter, - uint32 slot, uintN attrs, - uintN flags, intN shortid) const; - /* Add a data property whose id is not yet in this scope. */ JSScopeProperty *addDataProperty(JSContext *cx, jsid id, uint32 slot, uintN attrs) { JS_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER))); @@ -411,16 +400,16 @@ struct JSScope : public JSObjectMap GENERIC = 0x0080 }; - bool inDictionaryMode() const { return flags & DICTIONARY_MODE; } - void setDictionaryMode() { flags |= DICTIONARY_MODE; } - void clearDictionaryMode() { flags &= ~DICTIONARY_MODE; } + bool inDictionaryMode() { return flags & DICTIONARY_MODE; } + void setDictionaryMode() { flags |= DICTIONARY_MODE; } + void clearDictionaryMode() { flags &= ~DICTIONARY_MODE; } /* * Don't define clearSealed, as it can't be done safely because JS_LOCK_OBJ * will avoid taking the lock if the object owns its scope and the scope is * sealed. */ - bool sealed() const { return flags & SEALED; } + bool sealed() { return flags & SEALED; } void seal(JSContext *cx) { JS_ASSERT(!isSharedEmpty()); @@ -434,7 +423,7 @@ struct JSScope : public JSObjectMap * properties without magic getters and setters), and its scope->shape * evolves whenever a function value changes. */ - bool branded() const { JS_ASSERT(!generic()); return flags & BRANDED; } + bool branded() { JS_ASSERT(!generic()); return flags & BRANDED; } bool brand(JSContext *cx, uint32 slot, jsval v) { JS_ASSERT(!branded()); @@ -445,15 +434,15 @@ struct JSScope : public JSObjectMap return true; } - bool generic() const { return flags & GENERIC; } - void setGeneric() { flags |= GENERIC; } + bool generic() { return flags & GENERIC; } + void setGeneric() { flags |= GENERIC; } - bool hadIndexedProperties() const { return flags & INDEXED_PROPERTIES; } - void setIndexedProperties() { flags |= INDEXED_PROPERTIES; } + bool hadIndexedProperties() { return flags & INDEXED_PROPERTIES; } + void setIndexedProperties() { flags |= INDEXED_PROPERTIES; } - bool hasOwnShape() const { return flags & OWN_SHAPE; } + bool hasOwnShape() { return flags & OWN_SHAPE; } - bool hasRegenFlag(uint8 regenFlag) const { return (flags & SHAPE_REGEN) == regenFlag; } + bool hasRegenFlag(uint8 regenFlag) { return (flags & SHAPE_REGEN) == regenFlag; } /* * A scope has a method barrier when some compiler-created "null closure" @@ -495,8 +484,8 @@ struct JSScope : public JSObjectMap * METHOD_BARRIER too, and regenerate this scope's shape if the method's * value is in fact changing. */ - bool hasMethodBarrier() const { return flags & METHOD_BARRIER; } - void setMethodBarrier() { flags |= METHOD_BARRIER; } + bool hasMethodBarrier() { return flags & METHOD_BARRIER; } + void setMethodBarrier() { flags |= METHOD_BARRIER; } /* * Test whether this scope may be branded due to method calls, which means @@ -504,9 +493,10 @@ struct JSScope : public JSObjectMap * test whether this scope has method properties, which require a method * write barrier. */ - bool brandedOrHasMethodBarrier() const { return flags & (BRANDED | METHOD_BARRIER); } + bool + brandedOrHasMethodBarrier() { return flags & (BRANDED | METHOD_BARRIER); } - bool isSharedEmpty() const { return !object; } + bool isSharedEmpty() const { return !object; } static bool initRuntimeState(JSContext *cx); static void finishRuntimeState(JSContext *cx); diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 11aabe888be4..dc5e076f3153 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -2452,7 +2452,6 @@ TraceRecorder::insImmVal(jsval val) inline LIns* TraceRecorder::insImmObj(JSObject* obj) { - JS_ASSERT(obj); tree->gcthings.addUnique(OBJECT_TO_JSVAL(obj)); return lir->insImmP((void*)obj); } @@ -2460,7 +2459,6 @@ TraceRecorder::insImmObj(JSObject* obj) inline LIns* TraceRecorder::insImmFun(JSFunction* fun) { - JS_ASSERT(fun); tree->gcthings.addUnique(OBJECT_TO_JSVAL(FUN_OBJECT(fun))); return lir->insImmP((void*)fun); } @@ -2468,7 +2466,6 @@ TraceRecorder::insImmFun(JSFunction* fun) inline LIns* TraceRecorder::insImmStr(JSString* str) { - JS_ASSERT(str); tree->gcthings.addUnique(STRING_TO_JSVAL(str)); return lir->insImmP((void*)str); } @@ -2476,7 +2473,6 @@ TraceRecorder::insImmStr(JSString* str) inline LIns* TraceRecorder::insImmSprop(JSScopeProperty* sprop) { - JS_ASSERT(sprop); tree->sprops.addUnique(sprop); return lir->insImmP((void*)sprop); } @@ -11248,95 +11244,20 @@ TraceRecorder::record_JSOP_GETPROP() return getProp(stackval(-1)); } -/* - * If possible, lookup obj[id] without calling any resolve hooks or touching - * any non-native objects or objects that have been shared across contexts, - * store the results in *pobjp and *spropp (NULL if no such property exists), - * and return true. This does not take any locks. That is safe because we - * avoid shared objects. - * - * If a safe lookup is not possible, return false; *pobjp and *spropp are - * undefined. - */ -static bool -SafeLookup(JSContext *cx, JSObject* obj, jsid id, JSObject** pobjp, JSScopeProperty** spropp) +JS_REQUIRES_STACK AbortableRecordingStatus +TraceRecorder::record_JSOP_SETPROP() { - do { - // Avoid non-native lookupProperty hooks. - if (obj->map->ops->lookupProperty != js_LookupProperty) - return false; + jsval& l = stackval(-2); + if (JSVAL_IS_PRIMITIVE(l)) + RETURN_STOP_A("primitive this for SETPROP"); - // Avoid shared objects. - JSScope *scope = obj->scope(); - if (!CX_OWNS_SCOPE_TITLE(cx, scope)) - return false; - - if (JSScopeProperty *sprop = scope->lookup(id)) { - *pobjp = obj; - *spropp = sprop; - return true; - } - - // Avoid resolve hooks. - if (obj->getClass()->resolve != JS_ResolveStub) - return false; - } while ((obj = obj->getProto()) != NULL); - *pobjp = NULL; - *spropp = NULL; - return true; + JSObject* obj = JSVAL_TO_OBJECT(l); + if (obj->map->ops->setProperty != js_SetProperty) + RETURN_STOP_A("non-native JSObjectOps::setProperty"); + return ARECORD_CONTINUE; } -/* - * Lookup the property for the SETPROP/SETNAME/SETMETHOD instruction at pc. - * Emit guards to ensure that the result at run time is the same. - */ -JS_REQUIRES_STACK RecordingStatus -TraceRecorder::lookupForSetPropertyOp(JSObject* obj, LIns* obj_ins, jsid id, - bool* safep, JSObject** pobjp, JSScopeProperty** spropp) -{ - // We could consult the property cache here, but the contract for - // PropertyCache::testForSet is intricate enough that it's a lot less code - // to do a SafeLookup. - *safep = SafeLookup(cx, obj, id, pobjp, spropp); - if (!*safep) - return RECORD_CONTINUE; - - VMSideExit *exit = snapshot(BRANCH_EXIT); - if (*spropp) { - if (obj != globalObj) - CHECK_STATUS(guardShape(obj_ins, obj, obj->shape(), "guard_kshape", exit)); - if (obj != *pobjp && *pobjp != globalObj) { - CHECK_STATUS(guardShape(INS_CONSTOBJ(*pobjp), *pobjp, (*pobjp)->shape(), - "guard_vshape", exit)); - } - } else { - for (;;) { - if (obj != globalObj) - CHECK_STATUS(guardShape(obj_ins, obj, obj->shape(), "guard_proto_chain", exit)); - obj = obj->getProto(); - if (!obj) - break; - obj_ins = INS_CONSTOBJ(obj); - } - } - return RECORD_CONTINUE; -} - -static JSBool FASTCALL -MethodWriteBarrier(JSContext* cx, JSObject* obj, uint32 slot, jsval v) -{ - AutoValueRooter tvr(cx, v); - return obj->scope()->methodWriteBarrier(cx, slot, v); -} -JS_DEFINE_CALLINFO_4(static, BOOL_FAIL, MethodWriteBarrier, CONTEXT, OBJECT, UINT32, JSVAL, - 0, ACC_STORE_ANY) - -/* - * Emit a specialized, inlined copy of js_NativeSet. - * - * Note that since this runs before the interpreter opcode, obj->scope() may - * not actually have the property sprop yet. - */ +/* Emit a specialized, inlined copy of js_NativeSet. */ JS_REQUIRES_STACK RecordingStatus TraceRecorder::nativeSet(JSObject* obj, LIns* obj_ins, JSScopeProperty* sprop, jsval v, LIns* v_ins) @@ -11344,13 +11265,9 @@ TraceRecorder::nativeSet(JSObject* obj, LIns* obj_ins, JSScopeProperty* sprop, JSScope* scope = obj->scope(); uint32 slot = sprop->slot; - // If obj is the global, it must already have sprop. Otherwise we would - // have gone through addDataProperty, which would have stopped recording. - JS_ASSERT_IF(obj == globalObj, scope->hasProperty(sprop)); - /* - * We do not trace assignment to properties that have both a non-default - * setter and a slot, for several reasons. + * We do not trace assignment to properties that have both a nonstub setter + * and a slot, for several reasons. * * First, that would require sampling rt->propertyRemovals before and after * (see js_NativeSet), and even more code to handle the case where the two @@ -11365,18 +11282,8 @@ TraceRecorder::nativeSet(JSObject* obj, LIns* obj_ins, JSScopeProperty* sprop, * setter's return value differed from the record-time type of v, in which * case unboxing would fail and, having called a native setter, we could * not just retry the instruction in the interpreter. - * - * If obj is branded, we would have a similar problem recovering from a - * failed call to MethodWriteBarrier. */ - if (!sprop->hasDefaultSetter() && slot != SPROP_INVALID_SLOT) - RETURN_STOP("can't trace set of property with setter and slot"); - - // These two cases are strict-mode errors and can't be traced. - if (sprop->hasGetterValue() && sprop->hasDefaultSetter()) - RETURN_STOP("can't set a property that has only a getter"); - if (sprop->isDataDescriptor() && !sprop->writable()) - RETURN_STOP("can't assign to readonly property"); + JS_ASSERT(sprop->hasDefaultSetter() || slot == SPROP_INVALID_SLOT); // Box the value to be stored, if necessary. LIns* boxed_ins = NULL; @@ -11384,36 +11291,14 @@ TraceRecorder::nativeSet(JSObject* obj, LIns* obj_ins, JSScopeProperty* sprop, boxed_ins = box_jsval(v, v_ins); // Call the setter, if any. - if (!sprop->hasDefaultSetter()) { - if (sprop->hasSetterValue()) - RETURN_STOP("can't trace JavaScript function setter yet"); + if (!sprop->hasDefaultSetter()) emitNativePropertyOp(scope, sprop, obj_ins, true, boxed_ins); - } + // Store the value, if this property has a slot. if (slot != SPROP_INVALID_SLOT) { - if (scope->brandedOrHasMethodBarrier()) { - if (obj == globalObj) { - // Because the trace is type-specialized to the global object's - // slots, no run-time check is needed. Avoid recording a global - // shape change, though. - if (VALUE_IS_FUNCTION(cx, obj->getSlot(slot))) - RETURN_STOP("can't trace set of function-valued property in branded global"); - } else { - // Setting a function-valued property might need to rebrand the - // object. Call the method write barrier. Note that even if the - // property is not function-valued now, it might be on trace. - enterDeepBailCall(); - LIns* args[] = { boxed_ins, INS_CONST(slot), obj_ins, cx_ins }; - LIns* ok_ins = lir->insCall(&MethodWriteBarrier_ci, args); - guard(false, lir->insEqI_0(ok_ins), OOM_EXIT); - leaveDeepBailCall(); - } - } - - // Store the value. + JS_ASSERT(SPROP_HAS_VALID_SLOT(sprop, scope)); JS_ASSERT(sprop->hasSlot()); if (obj == globalObj) { - JS_ASSERT(SPROP_HAS_VALID_SLOT(sprop, scope)); if (!lazilyImportGlobalSlot(slot)) RETURN_STOP("lazy import of global slot failed"); set(&obj->getSlotRef(slot), v_ins); @@ -11426,67 +11311,90 @@ TraceRecorder::nativeSet(JSObject* obj, LIns* obj_ins, JSScopeProperty* sprop, return RECORD_CONTINUE; } -JS_REQUIRES_STACK RecordingStatus -TraceRecorder::addDataProperty(JSObject* obj, LIns* obj_ins, - jsid id, JSPropertyOp getter, JSPropertyOp setter, - uintN flags, intN shortid, jsval v, LIns* v_ins) +static JSBool FASTCALL +MethodWriteBarrier(JSContext* cx, JSObject* obj, JSScopeProperty* sprop, JSObject* funobj) { - // If obj is the global, the global shape is about to change. Note also - // that since we do not record this case, SETNAME and SETPROP are identical - // as far as the tracer is concerned. (js_CheckUndeclaredVarAssignment - // distinguishes the two, in the interpreter.) - if (obj == globalObj) - RETURN_STOP("set new property of global object"); // global shape change + AutoValueRooter tvr(cx, funobj); - // js_AddProperty does not call the addProperty hook. - if (obj->getClass()->addProperty != JS_PropertyStub) - RETURN_STOP("set new property of object with addProperty hook"); + return obj->scope()->methodWriteBarrier(cx, sprop, tvr.value()); +} +JS_DEFINE_CALLINFO_4(static, BOOL_FAIL, MethodWriteBarrier, CONTEXT, OBJECT, SCOPEPROP, OBJECT, + 0, ACC_STORE_ANY) - // See comment in TR::nativeSet about why we do not support setting a - // property that has both a setter and a slot. - if (setter != JS_PropertyStub) - RETURN_STOP("set new property with setter and slot"); +JS_REQUIRES_STACK RecordingStatus +TraceRecorder::setProp(jsval &l, PropertyCacheEntry* entry, JSScopeProperty* sprop, + jsval &v, LIns*& v_ins, bool isDefinitelyAtom) +{ + if (entry == JS_NO_PROP_CACHE_FILL) + RETURN_STOP("can't trace uncacheable property set"); + JS_ASSERT_IF(entry->vcapTag() >= 1, !sprop->hasSlot()); + if (!sprop->hasDefaultSetter() && sprop->slot != SPROP_INVALID_SLOT) + RETURN_STOP("can't trace set of property with setter and slot"); + if (sprop->hasSetterValue()) + RETURN_STOP("can't trace JavaScript function setter"); - // Methods can be defined only on plain Objects. - jsbytecode op = *cx->regs->pc; - if ((op == JSOP_INITMETHOD || op == JSOP_SETMETHOD) && obj->getClass() == &js_ObjectClass) { - JS_ASSERT(VALUE_IS_FUNCTION(cx, v)); + // These two cases are errors and can't be traced. + if (sprop->hasGetterValue()) + RETURN_STOP("can't assign to property with script getter but no setter"); + if (!sprop->writable()) + RETURN_STOP("can't assign to readonly property"); - JSObject *funobj = JSVAL_TO_OBJECT(v); - if (FUN_OBJECT(GET_FUNCTION_PRIVATE(cx, funobj)) == funobj) { - flags |= JSScopeProperty::METHOD; - getter = CastAsPropertyOp(funobj); - } + JS_ASSERT(!JSVAL_IS_PRIMITIVE(l)); + JSObject* obj = JSVAL_TO_OBJECT(l); + LIns* obj_ins = get(&l); + + JS_ASSERT_IF(entry->directHit(), obj->scope()->hasProperty(sprop)); + + // Fast path for CallClass. This is about 20% faster than the general case. + v_ins = get(&v); + if (obj->getClass() == &js_CallClass) + return setCallProp(obj, obj_ins, sprop, v_ins, v); + + // Find obj2. If entry->adding(), the TAG bits are all 0. + JSObject* obj2 = obj; + for (jsuword i = entry->scopeIndex(); i; i--) + obj2 = obj2->getParent(); + for (jsuword j = entry->protoIndex(); j; j--) + obj2 = obj2->getProto(); + JSScope *scope = obj2->scope(); + JS_ASSERT_IF(entry->adding(), obj2 == obj); + + // Guard before anything else. + PCVal pcval; + CHECK_STATUS(guardPropertyCacheHit(obj_ins, obj, obj2, entry, pcval)); + JS_ASSERT(scope->object == obj2); + JS_ASSERT(scope->hasProperty(sprop)); + JS_ASSERT_IF(obj2 != obj, !sprop->hasSlot()); + + /* + * Setting a function-valued property might need to rebrand the object, so + * we emit a call to the method write barrier. There's no need to guard on + * this, because functions have distinct trace-type from other values and + * branded-ness is implied by the shape, which we've already guarded on. + */ + if (scope->brandedOrHasMethodBarrier() && VALUE_IS_FUNCTION(cx, v) && entry->directHit()) { + if (obj == globalObj) + RETURN_STOP("can't trace function-valued property set in branded global scope"); + + enterDeepBailCall(); + LIns* args[] = { v_ins, INS_CONSTSPROP(sprop), obj_ins, cx_ins }; + LIns* ok_ins = lir->insCall(&MethodWriteBarrier_ci, args); + guard(false, lir->insEqI_0(ok_ins), OOM_EXIT); + leaveDeepBailCall(); } - // Determine what slot the interpreter will assign this property. - JSScope *scope; - uint32 slot; - JS_LOCK_OBJ(cx, obj); - scope = js_GetMutableScope(cx, obj); - if (!scope) - RETURN_ERROR("js_GetMutableScope failed"); - if (!js_AllocSlot(cx, obj, &slot)) - RETURN_ERROR("js_AllocSlot failed"); - js_FreeSlot(cx, obj, slot); - JS_UNLOCK_OBJ(cx, obj); + // Add a property to the object if necessary. + if (entry->adding()) { + JS_ASSERT(sprop->hasSlot()); + if (obj == globalObj) + RETURN_STOP("adding a property to the global object"); - // Find or create an sprop for the new property but avoid adding it to - // obj's scope. The sprop returned by prepareForAddProperty may not be - // rooted, but we immediately pass it to INS_CONSTSPROP, which roots it. - JSScopeProperty* sprop = obj->scope()->prepareForAddProperty(cx, id, getter, setter, slot, - JSPROP_ENUMERATE, flags, shortid); - if (!sprop) - RETURN_ERROR("JSScope::prepareForAddProperty failed"); + LIns* args[] = { INS_CONSTSPROP(sprop), obj_ins, cx_ins }; + const CallInfo *ci = isDefinitelyAtom ? &js_AddAtomProperty_ci : &js_AddProperty_ci; + LIns* ok_ins = lir->insCall(ci, args); + guard(false, lir->insEqI_0(ok_ins), OOM_EXIT); + } - // On trace, call js_AddProperty to do the dirty work. - LIns* args[] = { INS_CONSTSPROP(sprop), obj_ins, cx_ins }; - bool isDefinitelyAtom = (op == JSOP_SETPROP); - const CallInfo *ci = isDefinitelyAtom ? &js_AddAtomProperty_ci : &js_AddProperty_ci; - LIns* ok_ins = lir->insCall(ci, args); - guard(false, lir->insEqI_0(ok_ins), OOM_EXIT); - - // Box the value and store it in the new slot. return nativeSet(obj, obj_ins, sprop, v, v_ins); } @@ -11637,169 +11545,30 @@ TraceRecorder::setCallProp(JSObject *callobj, LIns *callobj_ins, JSScopeProperty return RECORD_CONTINUE; } -/* - * Emit a specialized, inlined copy of js_SetPropertyHelper for the current - * instruction. - */ -JS_REQUIRES_STACK RecordingStatus -TraceRecorder::setProperty(JSObject* obj, LIns* obj_ins, jsval v, LIns* v_ins) +JS_REQUIRES_STACK AbortableRecordingStatus +TraceRecorder::record_SetPropHit(PropertyCacheEntry* entry, JSScopeProperty* sprop) { - JSAtom *atom; - GET_ATOM_FROM_BYTECODE(cx->fp->script, cx->regs->pc, 0, atom); - jsid id = ATOM_TO_JSID(atom); - - if (obj->map->ops->setProperty != js_SetProperty) - RETURN_STOP("non-native object"); // TODO - fall back on slow path - if (obj->scope()->sealed()) - RETURN_STOP("setting property of sealed object"); // this will throw - - bool safe; - JSObject* pobj; - JSScopeProperty* sprop; - CHECK_STATUS(lookupForSetPropertyOp(obj, obj_ins, id, &safe, &pobj, &sprop)); - if (!safe) - RETURN_STOP("setprop: lookup fail"); // TODO - fall back on slow path - - // Handle Call objects specially. The Call objects we create on trace are - // bogus. Calling the setter on such an object wouldn't work. - if (obj->getClass() == &js_CallClass) - return setCallProp(obj, obj_ins, sprop, v_ins, v); - - // Handle setting a property that is not found on obj or anywhere on its - // the prototype chain. - if (!sprop) { - JSClass *cls = obj->getClass(); - return addDataProperty(obj, obj_ins, id, cls->getProperty, cls->setProperty, 0, 0, - v, v_ins); - } - - // Check whether we can assign to/over the existing property. - if (sprop->isAccessorDescriptor()) { - if (sprop->hasDefaultSetter()) - RETURN_STOP("setting accessor property with no setter"); - } else if (!sprop->writable()) { - RETURN_STOP("setting readonly data property"); - } - - // Handle setting an existing own property. - if (pobj == obj) - return nativeSet(obj, obj_ins, sprop, v, v_ins); - - // Handle setting an inherited non-SHARED property. - if (sprop->hasSlot()) { - JSPropertyOp getter, setter; - uintN flags; - intN shortid; - - if (sprop->hasShortID()) { - getter = sprop->getter(); - setter = sprop->setter(); - flags = JSScopeProperty::HAS_SHORTID; - shortid = sprop->shortid; - } else { - JSClass *cls = obj->getClass(); - getter = cls->getProperty; - setter = cls->setProperty; - flags = 0; - shortid = 0; - } - return addDataProperty(obj, obj_ins, id, getter, setter, flags, shortid, v, v_ins); - } - - // Handle setting an inherited SHARED property. - if (pobj->scope()->sealed()) - RETURN_STOP("setting SHARED property inherited from sealed object"); - if (sprop->hasDefaultSetter() && !sprop->hasGetterValue()) - return RECORD_CONTINUE; // nothing happens - - // Handle setting an inherited SHARED property with a non-default setter. - return nativeSet(obj, obj_ins, sprop, v, v_ins); -} - -/* - * Record a JSOP_SET{PROP,NAME,METHOD} instruction or a JSOP_INITPROP - * instruction initializing __proto__. - */ -JS_REQUIRES_STACK RecordingStatus -TraceRecorder::recordSetPropertyOp() -{ - jsval& l = stackval(-2); - if (JSVAL_IS_PRIMITIVE(l)) - RETURN_STOP("primitive operand object"); - JSObject* obj = JSVAL_TO_OBJECT(l); - LIns* obj_ins = get(&l); - jsval& r = stackval(-1); - LIns* r_ins = get(&r); - - CHECK_STATUS(setProperty(obj, obj_ins, r, r_ins)); - set(&l, r_ins); - return RECORD_CONTINUE; -} - -JS_REQUIRES_STACK AbortableRecordingStatus -TraceRecorder::record_JSOP_SETPROP() -{ - return InjectStatus(recordSetPropertyOp()); -} - -JS_REQUIRES_STACK AbortableRecordingStatus -TraceRecorder::record_JSOP_SETMETHOD() -{ - return InjectStatus(recordSetPropertyOp()); -} - -JS_REQUIRES_STACK AbortableRecordingStatus -TraceRecorder::record_JSOP_SETNAME() -{ - return InjectStatus(recordSetPropertyOp()); -} - -JS_REQUIRES_STACK RecordingStatus -TraceRecorder::recordInitPropertyOp() -{ jsval& l = stackval(-2); - JSObject* obj = JSVAL_TO_OBJECT(l); - LIns* obj_ins = get(&l); - JS_ASSERT(obj->getClass() == &js_ObjectClass); + LIns* v_ins; - jsval& v = stackval(-1); - LIns* v_ins = get(&v); + jsbytecode* pc = cx->regs->pc; - JSAtom *atom; - GET_ATOM_FROM_BYTECODE(cx->fp->script, cx->regs->pc, 0, atom); - jsid id = ATOM_TO_JSID(atom); + bool isDefinitelyAtom = (*pc == JSOP_SETPROP); + CHECK_STATUS_A(setProp(l, entry, sprop, r, v_ins, isDefinitelyAtom)); - // If obj already has this property (because the id appears more than once - // in the initializer), just set it. - JSScope* scope = obj->scope(); - if (!CX_OWNS_SCOPE_TITLE(cx, scope)) - RETURN_STOP("object being initialized is shared among contexts"); - if (JSScopeProperty* sprop = scope->lookup(id)) { - JS_ASSERT(sprop->isDataDescriptor()); - JS_ASSERT(SPROP_HAS_VALID_SLOT(sprop, scope)); - JS_ASSERT(sprop->hasDefaultSetter()); - return nativeSet(obj, obj_ins, sprop, v, v_ins); + switch (*pc) { + case JSOP_SETPROP: + case JSOP_SETNAME: + case JSOP_SETMETHOD: + if (pc[JSOP_SETPROP_LENGTH] != JSOP_POP) + set(&l, v_ins); + break; + + default:; } - // Duplicate the interpreter's special treatment of __proto__. - if (atom == cx->runtime->atomState.protoAtom) - return recordSetPropertyOp(); - - // Define a new property. - return addDataProperty(obj, obj_ins, id, JS_PropertyStub, JS_PropertyStub, 0, 0, v, v_ins); -} - -JS_REQUIRES_STACK AbortableRecordingStatus -TraceRecorder::record_JSOP_INITPROP() -{ - return InjectStatus(recordInitPropertyOp()); -} - -JS_REQUIRES_STACK AbortableRecordingStatus -TraceRecorder::record_JSOP_INITMETHOD() -{ - return InjectStatus(recordInitPropertyOp()); + return ARECORD_CONTINUE; } JS_REQUIRES_STACK VMSideExit* @@ -13721,6 +13490,13 @@ TraceRecorder::record_JSOP_ENDINIT() return ARECORD_CONTINUE; } +JS_REQUIRES_STACK AbortableRecordingStatus +TraceRecorder::record_JSOP_INITPROP() +{ + // All the action is in record_SetPropHit. + return ARECORD_CONTINUE; +} + JS_REQUIRES_STACK AbortableRecordingStatus TraceRecorder::record_JSOP_INITELEM() { @@ -14247,6 +14023,13 @@ TraceRecorder::record_JSOP_BINDNAME() return ARECORD_CONTINUE; } +JS_REQUIRES_STACK AbortableRecordingStatus +TraceRecorder::record_JSOP_SETNAME() +{ + // record_SetPropHit does all the work. + return ARECORD_CONTINUE; +} + JS_REQUIRES_STACK AbortableRecordingStatus TraceRecorder::record_JSOP_THROW() { @@ -15547,6 +15330,18 @@ TraceRecorder::record_JSOP_CONCATN() return ARECORD_CONTINUE; } +JS_REQUIRES_STACK AbortableRecordingStatus +TraceRecorder::record_JSOP_SETMETHOD() +{ + return record_JSOP_SETPROP(); +} + +JS_REQUIRES_STACK AbortableRecordingStatus +TraceRecorder::record_JSOP_INITMETHOD() +{ + return record_JSOP_INITPROP(); +} + JSBool FASTCALL js_Unbrand(JSContext *cx, JSObject *obj) { diff --git a/js/src/jstracer.h b/js/src/jstracer.h index e2de827253db..d1004f8bfbb2 100644 --- a/js/src/jstracer.h +++ b/js/src/jstracer.h @@ -1307,35 +1307,25 @@ class TraceRecorder nanojit::LIns* obj_ins, JSScopeProperty* sprop); + JS_REQUIRES_STACK RecordingStatus nativeSet(JSObject* obj, nanojit::LIns* obj_ins, + JSScopeProperty* sprop, + jsval v, nanojit::LIns* v_ins); + JS_REQUIRES_STACK RecordingStatus setProp(jsval &l, PropertyCacheEntry* entry, + JSScopeProperty* sprop, + jsval &v, nanojit::LIns*& v_ins, + bool isDefinitelyAtom); + JS_REQUIRES_STACK RecordingStatus setCallProp(JSObject *callobj, nanojit::LIns *callobj_ins, + JSScopeProperty *sprop, nanojit::LIns *v_ins, + jsval v); JS_REQUIRES_STACK RecordingStatus initOrSetPropertyByName(nanojit::LIns* obj_ins, - jsval* idvalp, jsval* rvalp, - bool init); + jsval* idvalp, jsval* rvalp, + bool init); JS_REQUIRES_STACK RecordingStatus initOrSetPropertyByIndex(nanojit::LIns* obj_ins, - nanojit::LIns* index_ins, - jsval* rvalp, bool init); + nanojit::LIns* index_ins, + jsval* rvalp, bool init); JS_REQUIRES_STACK AbortableRecordingStatus setElem(int lval_spindex, int idx_spindex, int v_spindex); - JS_REQUIRES_STACK RecordingStatus lookupForSetPropertyOp(JSObject* obj, nanojit::LIns* obj_ins, - jsid id, bool* safep, - JSObject** pobjp, - JSScopeProperty** spropp); - JS_REQUIRES_STACK RecordingStatus nativeSet(JSObject* obj, nanojit::LIns* obj_ins, - JSScopeProperty* sprop, - jsval v, nanojit::LIns* v_ins); - JS_REQUIRES_STACK RecordingStatus addDataProperty(JSObject* obj, nanojit::LIns* obj_ins, - jsid id, - JSPropertyOp getter, JSPropertyOp setter, - uintN flags, intN shortid, - jsval v, nanojit::LIns* v_ins); - JS_REQUIRES_STACK RecordingStatus setCallProp(JSObject *callobj, nanojit::LIns *callobj_ins, - JSScopeProperty *sprop, nanojit::LIns *v_ins, - jsval v); - JS_REQUIRES_STACK RecordingStatus setProperty(JSObject* obj, nanojit::LIns* obj_ins, - jsval v, nanojit::LIns* v_ins); - JS_REQUIRES_STACK RecordingStatus recordSetPropertyOp(); - JS_REQUIRES_STACK RecordingStatus recordInitPropertyOp(); - JS_REQUIRES_STACK nanojit::LIns* box_jsval(jsval v, nanojit::LIns* v_ins); JS_REQUIRES_STACK nanojit::LIns* unbox_jsval(jsval v, nanojit::LIns* v_ins, VMSideExit* exit); JS_REQUIRES_STACK void guardClassHelper(bool cond, nanojit::LIns* obj_ins, JSClass* clasp, @@ -1476,6 +1466,8 @@ public: JS_REQUIRES_STACK AbortableRecordingStatus monitorRecording(JSOp op); JS_REQUIRES_STACK AbortableRecordingStatus record_EnterFrame(uintN& inlineCallCount); JS_REQUIRES_STACK AbortableRecordingStatus record_LeaveFrame(); + JS_REQUIRES_STACK AbortableRecordingStatus record_SetPropHit(PropertyCacheEntry* entry, + JSScopeProperty* sprop); JS_REQUIRES_STACK AbortableRecordingStatus record_DefLocalFunSetSlot(uint32 slot, JSObject* obj); JS_REQUIRES_STACK AbortableRecordingStatus record_NativeCallComplete(); void forgetGuardedShapesForObject(JSObject* obj); diff --git a/js/src/trace-test/tests/basic/testMethodWriteBarrier.js b/js/src/trace-test/tests/basic/testMethodWriteBarrier.js deleted file mode 100644 index 86f2b17226ba..000000000000 --- a/js/src/trace-test/tests/basic/testMethodWriteBarrier.js +++ /dev/null @@ -1,4 +0,0 @@ -var x = {p: 0.1, m: function(){}}; -x.m(); // the interpreter brands x -for (var i = 0; i < 9; i++) - x.p = 0.1; diff --git a/js/src/trace-test/tests/basic/testMethodWriteBarrier2.js b/js/src/trace-test/tests/basic/testMethodWriteBarrier2.js deleted file mode 100644 index 19b387198a13..000000000000 --- a/js/src/trace-test/tests/basic/testMethodWriteBarrier2.js +++ /dev/null @@ -1,17 +0,0 @@ -function C() { - this.m = function () {}; // JSOP_SETMETHOD -} - -var a = [new C, new C, new C, new C, new C, new C, new C, new C, new C]; -var b = [new C, new C, new C, new C, new C, new C, a[8], new C, new C]; - -var thrown = 'none'; -try { - for (var i = 0; i < 9; i++) { - a[i].m(); - b[i].m = 0.7; // MethodWriteBarrier required here - } -} catch (exc) { - thrown = exc.name; -} -assertEq(thrown, 'TypeError'); diff --git a/js/src/trace-test/tests/basic/testMethodWriteBarrier3.js b/js/src/trace-test/tests/basic/testMethodWriteBarrier3.js deleted file mode 100644 index dc76dc9b741d..000000000000 --- a/js/src/trace-test/tests/basic/testMethodWriteBarrier3.js +++ /dev/null @@ -1,27 +0,0 @@ -function g() {} - -function h() { - for (var i = 0; i < 9; i++) - x.f = i; -} - -function j() { - x.f(); -} - -var x = {f: 0.7, g: g}; -x.g(); // interpreter brands x -h(); -print(shapeOf(x)); -x.f = function (){}; // does not change x's shape -j(); -print(shapeOf(x)); -h(); // should change x's shape - -var thrown = 'none'; -try { - j(); // should throw since x.f === 8 -} catch (exc) { - thrown = exc.name; -} -assertEq(thrown, 'TypeError');