From 94fa86c42c6085fd34b0cab008849eb86f3c69cc Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Thu, 18 Dec 2008 22:35:46 -0800 Subject: [PATCH] Bug 469625 - TM: Crash [@ js_String_getelem] (r=jorendorff). --- js/src/jsarray.cpp | 2 -- js/src/jsemit.cpp | 18 +++++++++++++++++- js/src/jsobj.cpp | 21 ++++++++++++++++++--- js/src/jsopcode.tbl | 1 + js/src/jstracer.cpp | 3 +-- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index bc8915e03736..070d198a51da 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -831,8 +831,6 @@ array_defineProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, if (!isIndex || attrs != JSPROP_ENUMERATE) { if (!ENSURE_SLOW_ARRAY(cx, obj)) return JS_FALSE; - if (isIndex && STOBJ_IS_DELEGATE(obj)) - cx->runtime->anyArrayProtoHasElement = JS_TRUE; return js_DefineProperty(cx, obj, id, value, getter, setter, attrs, propp); } diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 3d2269bdf6c8..0b3154095a3b 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -1697,7 +1697,6 @@ EmitIndexOp(JSContext *cx, JSOp op, uintN index, JSCodeGenerator *cg) return JS_FALSE; \ JS_END_MACRO - static JSBool EmitAtomOp(JSContext *cx, JSParseNode *pn, JSOp op, JSCodeGenerator *cg) { @@ -2324,6 +2323,9 @@ EmitXMLName(JSContext *cx, JSParseNode *pn, JSOp op, JSCodeGenerator *cg) } #endif +static JSBool +EmitElemOp(JSContext *cx, JSParseNode *pn, JSOp op, JSCodeGenerator *cg); + static JSBool EmitPropOp(JSContext *cx, JSParseNode *pn, JSOp op, JSCodeGenerator *cg, JSBool callContext) @@ -2331,6 +2333,15 @@ EmitPropOp(JSContext *cx, JSParseNode *pn, JSOp op, JSCodeGenerator *cg, JSParseNode *pn2, *pndot, *pnup, *pndown; ptrdiff_t top; + /* + * Special case obj.__proto__ to deoptimize away from fast paths in the + * interpreter and trace recorder, which skip dense array instances by + * skipping over the directly referenced object to Array.prototype before + * looking up the property name. See bug 450274. + */ + if (pn->pn_atom == cx->runtime->atomState.protoAtom) + return EmitElemOp(cx, pn, callContext ? JSOP_CALLELEM : JSOP_GETELEM, cg); + pn2 = pn->pn_expr; if (callContext) { JS_ASSERT(pn->pn_type == TOK_DOT); @@ -5321,6 +5332,11 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) if (pn2->pn_atom == cx->runtime->atomState.lengthAtom) { if (js_Emit1(cx, cg, JSOP_LENGTH) < 0) return JS_FALSE; + } else if (pn2->pn_atom == cx->runtime->atomState.protoAtom) { + if (!EmitIndexOp(cx, JSOP_QNAMEPART, atomIndex, cg)) + return JS_FALSE; + if (js_Emit1(cx, cg, JSOP_GETELEM) < 0) + return JS_FALSE; } else { EMIT_INDEX_OP(JSOP_GETPROP, atomIndex); } diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 06f4ffcf4e32..2709459d0100 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -326,10 +326,20 @@ js_SetProtoOrParent(JSContext *cx, JSObject *obj, uint32 slot, JSObject *pobj) return JS_FALSE; } - // Maintain the "any Array prototype has indexed properties hazard" flag. + /* + * Maintain the "any Array prototype has indexed properties hazard" flag by + * conservatively setting it. We simply don't know what pobj has in the way + * of indexed properties, either directly or along its prototype chain, and + * we won't expend effort here to find out. We do know that if obj is not + * an array or a prototype (delegate), then we're ok. And, of course, pobj + * must be non-null. + * + * This pessimistic approach could be improved, but setting __proto__ is + * quite rare and arguably deserving of deoptimization. + */ if (slot == JSSLOT_PROTO && - OBJ_IS_ARRAY(cx, pobj) && - pobj->fslots[JSSLOT_ARRAY_LENGTH] != 0) { + pobj && + (OBJ_IS_ARRAY(cx, obj) || OBJ_IS_DELEGATE(cx, obj))) { rt->anyArrayProtoHasElement = JS_TRUE; } return JS_TRUE; @@ -3161,6 +3171,9 @@ js_DefineProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, * nominal initial value of a slot-full property, while GC safety wants that * value to be stored before the call-out through the hook. Optimize to do * both while saving cycles for classes that stub their addProperty hook. + * + * As in js_SetProtoOrParent (see above), we maintain the "any Array prototype + * has indexed properties hazard" flag by conservatively setting it. */ #define ADD_PROPERTY_HELPER(cx,clasp,obj,scope,sprop,vp,cleanup) \ JS_BEGIN_MACRO \ @@ -3174,6 +3187,8 @@ js_DefineProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, LOCKED_OBJ_WRITE_BARRIER(cx, obj, (sprop)->slot, *(vp)); \ } \ } \ + if (STOBJ_IS_DELEGATE(obj) && JSID_IS_INT(sprop->id)) \ + cx->runtime->anyArrayProtoHasElement = JS_TRUE; \ JS_END_MACRO JSBool diff --git a/js/src/jsopcode.tbl b/js/src/jsopcode.tbl index 6392209a5533..bb8ea7e25eb9 100644 --- a/js/src/jsopcode.tbl +++ b/js/src/jsopcode.tbl @@ -536,6 +536,7 @@ OPDEF(JSOP_INDEXBASE3, 222,"atombase3", NULL, 1, 0, 0, 0, JOF_BYTE | OPDEF(JSOP_CALLGVAR, 223, "callgvar", NULL, 3, 0, 2, 19, JOF_ATOM|JOF_NAME|JOF_CALLOP) OPDEF(JSOP_CALLLOCAL, 224, "calllocal", NULL, 3, 0, 2, 19, JOF_LOCAL|JOF_NAME|JOF_CALLOP) OPDEF(JSOP_CALLARG, 225, "callarg", NULL, 3, 0, 2, 19, JOF_QARG |JOF_NAME|JOF_CALLOP) + OPDEF(JSOP_UNUSED226, 226, "unused226", NULL, 1, 0, 1, 1, JOF_BYTE) /* diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index ee06191af11b..c9cd352f1ebd 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -7795,7 +7795,6 @@ TraceRecorder::record_JSOP_GOTOX() JS_REQUIRES_STACK bool TraceRecorder::record_JSOP_IFEQX() { - trackCfgMerges(cx->fp->regs->pc); return record_JSOP_IFEQ(); } @@ -8488,7 +8487,7 @@ TraceRecorder::record_JSOP_LENGTH() jsval& l = stackval(-1); if (JSVAL_IS_PRIMITIVE(l)) { if (!JSVAL_IS_STRING(l)) - ABORT_TRACE("non-string primitives unsupported"); + ABORT_TRACE("non-string primitive JSOP_LENGTH unsupported"); LIns* str_ins = get(&l); LIns* len_ins = lir->insLoad(LIR_ldp, str_ins, (int)offsetof(JSString, length));