Try harder to trace array access with non-int / non-string index (478525, r=brendan).

This commit is contained in:
Andreas Gal 2009-03-09 17:34:22 -07:00
parent d3cc05e80b
commit 088eeb6c9d
2 changed files with 78 additions and 139 deletions

View File

@ -351,16 +351,16 @@ static inline bool isInt32(jsval v)
return JSDOUBLE_IS_INT(d, i); return JSDOUBLE_IS_INT(d, i);
} }
static inline bool asInt32(jsval v, jsint& rv) static inline jsint asInt32(jsval v)
{ {
if (!isNumber(v)) JS_ASSERT(isNumber(v));
return false; if (JSVAL_IS_INT(v))
if (JSVAL_IS_INT(v)) { return JSVAL_TO_INT(v);
rv = JSVAL_TO_INT(v); #ifdef DEBUG
return true; jsint i;
} JS_ASSERT(JSDOUBLE_IS_INT(*JSVAL_TO_DOUBLE(v), i));
jsdouble d = asNumber(v); #endif
return JSDOUBLE_IS_INT(d, rv); return jsint(*JSVAL_TO_DOUBLE(v));
} }
/* Return JSVAL_DOUBLE for all numbers (int and double) and the tag otherwise. */ /* Return JSVAL_DOUBLE for all numbers (int and double) and the tag otherwise. */
@ -6040,68 +6040,12 @@ TraceRecorder::guardDenseArrayIndex(JSObject* obj, jsint idx, LIns* obj_ins,
return cond; return cond;
} }
/* bool
* Guard that a computed property access via an element op (JSOP_GETELEM, etc.) TraceRecorder::guardNotGlobalObject(JSObject* obj, LIns* obj_ins)
* does not find an alias to a global variable, or a property without a slot,
* or a slot-ful property with a getter or setter (depending on op_offset in
* JSObjectOps). Finally, beware resolve hooks mutating objects. Oh, and watch
* out for bears too ;-).
*
* One win here is that we do not need to generate a guard that obj_ins does
* not result in the global object on trace, because we guard on shape and rule
* out obj's shape being the global object's shape at recording time. This is
* safe because the global shape cannot change on trace.
*/
JS_REQUIRES_STACK bool
TraceRecorder::guardElemOp(JSObject* obj, LIns* obj_ins, jsid id, size_t op_offset, jsval* vp)
{ {
JS_ASSERT(op_offset == offsetof(JSObjectOps, getProperty) || if (obj == globalObj)
op_offset == offsetof(JSObjectOps, setProperty)); ABORT_TRACE("reference aliases global object");
guard(false, lir->ins2(LIR_eq, obj_ins, globalObj_ins), MISMATCH_EXIT);
LIns* map_ins = lir->insLoad(LIR_ldp, obj_ins, (int)offsetof(JSObject, map));
LIns* ops_ins;
if (!map_is_native(obj->map, map_ins, ops_ins, op_offset))
ABORT_TRACE("non-native map");
uint32 shape = OBJ_SHAPE(obj);
if (JSID_IS_ATOM(id) && shape == treeInfo->globalShape)
ABORT_TRACE("elem op probably aliases global");
JSObject* pobj;
JSProperty* prop;
if (!js_LookupProperty(cx, obj, id, &pobj, &prop))
return false;
if (vp)
*vp = JSVAL_VOID;
if (prop) {
bool traceable_slot = true;
if (pobj == obj) {
JSScopeProperty* sprop = (JSScopeProperty*) prop;
traceable_slot = ((op_offset == offsetof(JSObjectOps, getProperty))
? SPROP_HAS_STUB_GETTER(sprop)
: SPROP_HAS_STUB_SETTER(sprop)) &&
SPROP_HAS_VALID_SLOT(sprop, OBJ_SCOPE(obj));
if (vp && traceable_slot)
*vp = LOCKED_OBJ_GET_SLOT(obj, sprop->slot);
}
OBJ_DROP_PROPERTY(cx, pobj, prop);
if (pobj != obj)
ABORT_TRACE("elem op hit prototype property, can't shape-guard");
if (!traceable_slot)
ABORT_TRACE("elem op hit direct and slotless getter or setter");
}
if (wasDeepAborted())
ABORT_TRACE("deep abort from property lookup");
// If we got this far, we're almost safe -- but we must check for a rogue resolve hook.
if (OBJ_SHAPE(obj) != shape)
ABORT_TRACE("resolve hook mutated elem op base object");
LIns* shape_ins = addName(lir->insLoad(LIR_ld, map_ins, offsetof(JSScope, shape)), "shape");
guard(true, addName(lir->ins2i(LIR_eq, shape_ins, shape), "guard(shape)"), BRANCH_EXIT);
return true; return true;
} }
@ -7205,9 +7149,10 @@ TraceRecorder::record_JSOP_GETELEM()
LIns* obj_ins = get(&lval); LIns* obj_ins = get(&lval);
LIns* idx_ins = get(&idx); LIns* idx_ins = get(&idx);
if (JSVAL_IS_STRING(lval) && JSVAL_IS_INT(idx)) { // Special case for array-like access of strings.
int i = JSVAL_TO_INT(idx); if (JSVAL_IS_STRING(lval) && isInt32(idx)) {
if ((size_t)i >= JSSTRING_LENGTH(JSVAL_TO_STRING(lval))) int i = asInt32(idx);
if (size_t(i) >= JSSTRING_LENGTH(JSVAL_TO_STRING(lval)))
ABORT_TRACE("Invalid string index in JSOP_GETELEM"); ABORT_TRACE("Invalid string index in JSOP_GETELEM");
idx_ins = makeNumberInt32(idx_ins); idx_ins = makeNumberInt32(idx_ins);
LIns* args[] = { idx_ins, obj_ins, cx_ins }; LIns* args[] = { idx_ins, obj_ins, cx_ins };
@ -7222,43 +7167,35 @@ TraceRecorder::record_JSOP_GETELEM()
JSObject* obj = JSVAL_TO_OBJECT(lval); JSObject* obj = JSVAL_TO_OBJECT(lval);
jsval id; jsval id;
jsval v;
LIns* v_ins; LIns* v_ins;
/* Property access using a string name. */ /* Property access using a string name or something we have to stringify. */
if (JSVAL_IS_STRING(idx)) { if (!JSVAL_IS_INT(idx)) {
if (!js_ValueToStringId(cx, idx, &id)) // If index is not a string, turn it into a string.
return false; if (!js_InternNonIntElementId(cx, obj, idx, &id))
ABORT_TRACE("failed to intern non-int element id");
set(&idx, stringify(idx));
// Store the interned string to the stack to save the interpreter from redoing this work. // Store the interned string to the stack to save the interpreter from redoing this work.
idx = ID_TO_VALUE(id); idx = ID_TO_VALUE(id);
jsuint index;
if (js_IdIsIndex(idx, &index) && guardDenseArray(obj, obj_ins, BRANCH_EXIT)) { // The object is not guaranteed to be a dense array at this point, so it might be the
v = (index >= js_DenseArrayCapacity(obj)) ? JSVAL_HOLE : obj->dslots[index]; // global object, which we have to guard against.
if (v == JSVAL_HOLE) if (!guardNotGlobalObject(obj, obj_ins))
ABORT_TRACE("can't see through hole in dense array"); return false;
} else {
if (!guardElemOp(obj, obj_ins, id, offsetof(JSObjectOps, getProperty), &v))
return false;
}
return call_imacro(getelem_imacros.getprop); return call_imacro(getelem_imacros.getprop);
} }
/* At this point we expect a whole number or we bail. */ // Invalid dense array index or not a dense array.
if (!JSVAL_IS_INT(idx)) if (JSVAL_TO_INT(idx) < 0 || !OBJ_IS_DENSE_ARRAY(cx, obj)) {
ABORT_TRACE("non-string, non-int JSOP_GETELEM index"); if (!guardNotGlobalObject(obj, obj_ins))
if (JSVAL_TO_INT(idx) < 0) return false;
ABORT_TRACE("negative JSOP_GETELEM index");
/* Accessing an object using integer index but not a dense array. */
if (!OBJ_IS_DENSE_ARRAY(cx, obj)) {
idx_ins = makeNumberInt32(idx_ins);
if (!js_IndexToId(cx, JSVAL_TO_INT(idx), &id))
return false;
if (!guardElemOp(obj, obj_ins, id, offsetof(JSObjectOps, getProperty), &v))
return false;
return call_imacro(getelem_imacros.getelem); return call_imacro(getelem_imacros.getelem);
} }
// Fast path for dense arrays accessed with a non-negative integer index.
jsval* vp; jsval* vp;
LIns* addr_ins; LIns* addr_ins;
if (!elem(lval, idx, vp, v_ins, addr_ins)) if (!elem(lval, idx, vp, v_ins, addr_ins))
@ -7353,46 +7290,50 @@ TraceRecorder::record_JSOP_SETELEM()
LIns* v_ins = get(&v); LIns* v_ins = get(&v);
jsid id; jsid id;
if (!JSVAL_IS_INT(idx)) {
// If index is not a string, turn it into a string.
if (!js_InternNonIntElementId(cx, obj, idx, &id))
ABORT_TRACE("failed to intern non-int element id");
set(&idx, stringify(idx));
// Store the interned string to the stack to save the interpreter from redoing this work.
idx = ID_TO_VALUE(id);
// The object is not guaranteed to be a dense array at this point, so it might be the
// global object, which we have to guard against.
if (!guardNotGlobalObject(obj, obj_ins))
return false;
return call_imacro(setelem_imacros.setprop);
}
if (JSVAL_TO_INT(idx) < 0 || !OBJ_IS_DENSE_ARRAY(cx, obj)) {
if (!guardNotGlobalObject(obj, obj_ins))
return false;
return call_imacro((*cx->fp->regs->pc == JSOP_INITELEM)
? initelem_imacros.initelem
: setelem_imacros.setelem);
}
// Fast path for dense arrays accessed with a non-negative integer index. In case the trace
// calculated the index using the FPU, force it to be an integer.
idx_ins = makeNumberInt32(idx_ins);
// Box the value so we can use one builtin instead of having to add one builtin for every
// storage type.
LIns* boxed_v_ins = v_ins; LIns* boxed_v_ins = v_ins;
box_jsval(v, boxed_v_ins); box_jsval(v, boxed_v_ins);
if (JSVAL_IS_STRING(idx)) { LIns* args[] = { boxed_v_ins, idx_ins, obj_ins, cx_ins };
if (!js_ValueToStringId(cx, idx, &id)) LIns* res_ins = lir->insCall(&js_Array_dense_setelem_ci, args);
return false; guard(false, lir->ins_eq0(res_ins), MISMATCH_EXIT);
// Store the interned string to the stack to save the interpreter from redoing this work.
idx = ID_TO_VALUE(id);
if (!guardElemOp(obj, obj_ins, id, offsetof(JSObjectOps, setProperty), NULL))
return false;
return call_imacro(setelem_imacros.setprop);
}
if (JSVAL_IS_INT(idx)) {
if (JSVAL_TO_INT(idx) < 0)
ABORT_TRACE("negative JSOP_SETELEM index");
idx_ins = makeNumberInt32(idx_ins);
if (!guardDenseArray(obj, obj_ins, BRANCH_EXIT)) { jsbytecode* pc = cx->fp->regs->pc;
if (!js_IndexToId(cx, JSVAL_TO_INT(idx), &id)) if (*pc == JSOP_SETELEM && pc[JSOP_SETELEM_LENGTH] != JSOP_POP)
return false; set(&lval, v_ins);
idx = ID_TO_VALUE(id);
if (!guardElemOp(obj, obj_ins, id, offsetof(JSObjectOps, setProperty), NULL))
return false;
jsbytecode* pc = cx->fp->regs->pc;
return call_imacro((*pc == JSOP_INITELEM)
? initelem_imacros.initelem
: setelem_imacros.setelem);
}
LIns* args[] = { boxed_v_ins, idx_ins, obj_ins, cx_ins }; return true;
LIns* res_ins = lir->insCall(&js_Array_dense_setelem_ci, args);
guard(false, lir->ins_eq0(res_ins), MISMATCH_EXIT);
jsbytecode* pc = cx->fp->regs->pc;
if (*pc == JSOP_SETELEM && pc[JSOP_SETELEM_LENGTH] != JSOP_POP)
set(&lval, v_ins);
return true;
}
ABORT_TRACE("non-string, non-int JSOP_SETELEM index");
} }
JS_REQUIRES_STACK bool JS_REQUIRES_STACK bool
@ -7712,9 +7653,8 @@ TraceRecorder::prop(JSObject* obj, LIns* obj_ins, uint32& slot, LIns*& v_ins)
* Can't specialize to assert obj != global, must guard to avoid aliasing * Can't specialize to assert obj != global, must guard to avoid aliasing
* stale homes of stacked global variables. * stale homes of stacked global variables.
*/ */
if (obj == globalObj) if (!guardNotGlobalObject(obj, obj_ins))
ABORT_TRACE("prop op aliases global"); return false;
guard(false, lir->ins2(LIR_eq, obj_ins, globalObj_ins), MISMATCH_EXIT);
/* /*
* Property cache ensures that we are dealing with an existing property, * Property cache ensures that we are dealing with an existing property,

View File

@ -536,8 +536,7 @@ class TraceRecorder : public avmplus::GCObject {
JS_REQUIRES_STACK bool guardDenseArrayIndex(JSObject* obj, jsint idx, nanojit::LIns* obj_ins, JS_REQUIRES_STACK bool guardDenseArrayIndex(JSObject* obj, jsint idx, nanojit::LIns* obj_ins,
nanojit::LIns* dslots_ins, nanojit::LIns* idx_ins, nanojit::LIns* dslots_ins, nanojit::LIns* idx_ins,
ExitType exitType); ExitType exitType);
JS_REQUIRES_STACK bool guardElemOp(JSObject* obj, nanojit::LIns* obj_ins, jsid id, bool guardNotGlobalObject(JSObject* obj, nanojit::LIns* obj_ins);
size_t op_offset, jsval* vp);
void clearFrameSlotsFromCache(); void clearFrameSlotsFromCache();
JS_REQUIRES_STACK bool guardCallee(jsval& callee); JS_REQUIRES_STACK bool guardCallee(jsval& callee);
JS_REQUIRES_STACK bool getClassPrototype(JSObject* ctor, nanojit::LIns*& proto_ins); JS_REQUIRES_STACK bool getClassPrototype(JSObject* ctor, nanojit::LIns*& proto_ins);