Bug 504797: give arguments objects created on trace a private value that can be used to look up argument values in the native stack, r=gal

js/src/jsbuiltins.h

@@ -224,6 +224,7 @@ class ClosureVarInfo;
 #define _JS_CTYPE_FRAGMENT          _JS_CTYPE(nanojit::Fragment *,    _JS_PTR, --, --, INFALLIBLE)
 #define _JS_CTYPE_CLASS             _JS_CTYPE(JSClass *,              _JS_PTR, --, --, INFALLIBLE)
 #define _JS_CTYPE_DOUBLEPTR         _JS_CTYPE(double *,               _JS_PTR, --, --, INFALLIBLE)
+#define _JS_CTYPE_APNPTR            _JS_CTYPE(js_ArgsPrivateNative *, _JS_PTR, --, --, INFALLIBLE)
 #define _JS_CTYPE_CVIPTR            _JS_CTYPE(const ClosureVarInfo *, _JS_PTR, --, --, INFALLIBLE)
 
 #define _JS_EXPAND(tokens)  tokens
@@ -452,6 +453,7 @@ js_dmod(jsdouble a, jsdouble b);
 #define JS_DEFINE_CALLINFO_3(linkage, rt, op, at0, at1, at2, cse, fold)
 #define JS_DEFINE_CALLINFO_4(linkage, rt, op, at0, at1, at2, at3, cse, fold)
 #define JS_DEFINE_CALLINFO_5(linkage, rt, op, at0, at1, at2, at3, at4, cse, fold)
+#define JS_DEFINE_CALLINFO_6(linkage, rt, op, at0, at1, at2, at3, at4, at5, cse, fold)
 #define JS_DECLARE_CALLINFO(name)
 #define JS_DEFINE_TRCINFO_1(name, tn0)
 #define JS_DEFINE_TRCINFO_2(name, tn0, tn1)

js/src/jsfun.cpp

@@ -148,6 +148,14 @@ GetArgsLength(JSObject *obj)
     return argc;
 }
 
+static inline void 
+SetArgsPrivateNative(JSObject *argsobj, js_ArgsPrivateNative *apn)
+{
+    JS_ASSERT(STOBJ_GET_CLASS(argsobj) == &js_ArgumentsClass);
+    uintptr_t p = (uintptr_t) apn;
+    argsobj->setPrivate((void*) (p | 2));
+}
+
 JSBool
 js_GetArgsValue(JSContext *cx, JSStackFrame *fp, jsval *vp)
 {
@@ -305,22 +313,27 @@ js_PutArgsObject(JSContext *cx, JSStackFrame *fp)
 /*
  * Traced versions of js_GetArgsObject and js_PutArgsObject.
  */
+
+#ifdef JS_TRACER
 JSObject * JS_FASTCALL
 js_Arguments(JSContext *cx, JSObject *parent, uint32 argc, JSObject *callee,
-             JSObject* cached)
+             double *argv, js_ArgsPrivateNative *apn)
 {
-    return cached
-           ? cached
-           : NewArguments(cx, parent, argc, callee);
+    JSObject *argsobj = NewArguments(cx, parent, argc, callee);
+    apn->argv = argv;
+    SetArgsPrivateNative(argsobj, apn);
+    return argsobj;
 }
+#endif
 
-JS_DEFINE_CALLINFO_5(extern, OBJECT, js_Arguments, CONTEXT, OBJECT, UINT32, OBJECT, OBJECT, 0, 0)
+JS_DEFINE_CALLINFO_6(extern, OBJECT, js_Arguments, CONTEXT, OBJECT, UINT32, OBJECT, 
+                     DOUBLEPTR, APNPTR, 0, 0)
 
 /* FIXME change the return type to void. */
 JSBool JS_FASTCALL
 js_PutArguments(JSContext *cx, JSObject *argsobj, jsval *args)
 {
-    JS_ASSERT(!argsobj->getPrivate());
+    JS_ASSERT(js_GetArgsPrivateNative(argsobj));
     PutArguments(cx, argsobj, args);
     return true;
 }
@@ -525,6 +538,16 @@ ArgGetter(JSContext *cx, JSObject *obj, jsval idval, jsval *vp)
          */
         uintN arg = uintN(JSVAL_TO_INT(idval));
         if (arg < GetArgsLength(obj)) {
+#ifdef JS_TRACER
+            js_ArgsPrivateNative *argp = js_GetArgsPrivateNative(obj);
+            if (argp) {
+                if (js_NativeToValue(cx, *vp, (JSTraceType) 1, &argp->argv[arg]))
+                    return true;
+                js_LeaveTrace(cx);
+                return false;
+            }
+#endif
+
             JSStackFrame *fp = (JSStackFrame *) obj->getPrivate();
             if (fp) {
                 *vp = fp->argv[arg];
@@ -569,6 +592,11 @@ ArgSetter(JSContext *cx, JSObject *obj, jsval idval, jsval *vp)
     if (JSVAL_IS_INT(idval)) {
         uintN arg = uintN(JSVAL_TO_INT(idval));
         if (arg < GetArgsLength(obj)) {
+            if (js_GetArgsPrivateNative(obj)) {
+                js_LeaveTrace(cx);
+                return false;
+            }
+
             JSStackFrame *fp = (JSStackFrame *) obj->getPrivate();
             if (fp) {
                 fp->argv[arg] = *vp;
@@ -669,6 +697,8 @@ args_or_call_trace(JSTracer *trc, JSObject *obj)
 {
     JS_ASSERT(STOBJ_GET_CLASS(obj) == &js_ArgumentsClass ||
               STOBJ_GET_CLASS(obj) == &js_CallClass);
+    if (STOBJ_GET_CLASS(obj) == &js_ArgumentsClass && js_GetArgsPrivateNative(obj))
+        return;
 
     JSStackFrame *fp = (JSStackFrame *) obj->getPrivate();
     if (fp && (fp->flags & JSFRAME_GENERATOR)) {

js/src/jsfun.h

@@ -220,6 +220,16 @@ extern JS_FRIEND_DATA(JSClass) js_FunctionClass;
     (JS_ASSERT(HAS_FUNCTION_CLASS(funobj)),                                   \
      (JSFunction *) (funobj)->getPrivate())
 
+struct js_ArgsPrivateNative;
+
+inline js_ArgsPrivateNative *
+js_GetArgsPrivateNative(JSObject *argsobj)
+{
+    JS_ASSERT(STOBJ_GET_CLASS(argsobj) == &js_ArgumentsClass);
+    uintptr_t p = (uintptr_t) argsobj->getPrivate();
+    return (js_ArgsPrivateNative *) (p & 2 ? p & ~2 : NULL);
+}
+
 extern JSObject *
 js_InitFunctionClass(JSContext *cx, JSObject *obj);
 

js/src/jstracer.cpp

@@ -2164,8 +2164,9 @@ JSTraceMonitor::mark(JSTracer* trc)
  * are too large to fit into a jsval are automatically boxed into
  * heap-allocated doubles.
  */
-static void
-NativeToValue(JSContext* cx, jsval& v, JSTraceType type, double* slot)
+template <typename E>
+static inline bool
+NativeToValueBase(JSContext* cx, jsval& v, JSTraceType type, double* slot)
 {
     jsint i;
     jsdouble d;
@@ -2206,12 +2207,9 @@ NativeToValue(JSContext* cx, jsval& v, JSTraceType type, double* slot)
 #endif
                 js_NewDoubleInRootedValue(cx, d, &v);
             JS_ASSERT(ok);
-            return;
+            return true;
         }
-        v = AllocateDoubleFromReservedPool(cx);
-        JS_ASSERT(JSVAL_IS_DOUBLE(v) && *JSVAL_TO_DOUBLE(v) == 0.0);
-        *JSVAL_TO_DOUBLE(v) = d;
-        return;
+        return E::handleDoubleOOM(cx, d, v);
       }
 
       case TT_JSVAL:
@@ -2251,6 +2249,38 @@ NativeToValue(JSContext* cx, jsval& v, JSTraceType type, double* slot)
         break;
       }
     }
+    return true;
+}
+
+struct ReserveDoubleOOMHandler {
+    static bool handleDoubleOOM(JSContext *cx, double d, jsval& v) {
+        v = AllocateDoubleFromReservedPool(cx);
+        JS_ASSERT(JSVAL_IS_DOUBLE(v) && *JSVAL_TO_DOUBLE(v) == 0.0);
+        *JSVAL_TO_DOUBLE(v) = d;
+        return true;
+    }
+};
+
+static void
+NativeToValue(JSContext* cx, jsval& v, JSTraceType type, double* slot)
+{
+#ifdef DEBUG
+    bool ok = 
+#endif
+        NativeToValueBase<ReserveDoubleOOMHandler>(cx, v, type, slot);
+    JS_ASSERT(ok);
+}
+
+struct FailDoubleOOMHandler {
+    static bool handleDoubleOOM(JSContext *cx, double d, jsval& v) {
+        return false;
+    }
+};
+
+bool
+js_NativeToValue(JSContext* cx, jsval& v, JSTraceType type, double* slot)
+{
+    return NativeToValueBase<FailDoubleOOMHandler>(cx, v, type, slot);
 }
 
 class BuildNativeFrameVisitor : public SlotVisitorBase
@@ -2653,8 +2683,8 @@ FlushNativeStackFrame(JSContext* cx, unsigned callDepth, JSTraceType* mp, double
         for (; n != 0; fp = fp->down) {
             --n;
             if (fp->argv) {
-                // fp->argsobj->getPrivate() is NULL iff we created argsobj on trace.
-                if (fp->argsobj && !JSVAL_TO_OBJECT(fp->argsobj)->getPrivate()) {
+                if (fp->argsobj && 
+                    js_GetArgsPrivateNative(JSVAL_TO_OBJECT(fp->argsobj))) {
                     JSVAL_TO_OBJECT(fp->argsobj)->setPrivate(fp);
                 }
 
@@ -8769,23 +8799,62 @@ TraceRecorder::record_JSOP_IFNE()
     return ifop();
 }
 
+LIns*
+TraceRecorder::newArguments()
+{
+    LIns* global_ins = INS_CONSTOBJ(globalObj);
+    LIns* argc_ins = INS_CONST(cx->fp->argc);
+    LIns* callee_ins = get(&cx->fp->argv[-2]);
+    LIns* argv_ins = cx->fp->argc
+        ? lir->ins2(LIR_piadd, lirbuf->sp, 
+                    INS_CONST(-treeInfo->nativeStackBase + nativeStackOffset(&cx->fp->argv[0])))
+        : INS_CONSTPTR((void *) 2);
+    js_ArgsPrivateNative *apn = js_ArgsPrivateNative::create(*traceMonitor->allocator, 
+                                                             cx->fp->argc);
+    for (uintN i = 0; i < cx->fp->argc; ++i) {
+        apn->typemap()[i] = determineSlotType(&cx->fp->argv[i]);
+    }
+
+    LIns* args[] = { INS_CONSTPTR(apn), argv_ins, callee_ins, argc_ins, global_ins, cx_ins };
+    LIns* call_ins = lir->insCall(&js_Arguments_ci, args);
+    guard(false, lir->ins_eq0(call_ins), OOM_EXIT);
+    return call_ins;
+}
+
 JS_REQUIRES_STACK JSRecordingStatus
 TraceRecorder::record_JSOP_ARGUMENTS()
 {
     if (cx->fp->flags & JSFRAME_OVERRIDE_ARGS)
         ABORT_TRACE("Can't trace |arguments| if |arguments| is assigned to");
 
-    LIns* global_ins = INS_CONSTOBJ(globalObj);
-    LIns* argc_ins = INS_CONST(cx->fp->argc);
-    LIns* callee_ins = get(&cx->fp->argv[-2]);
     LIns* a_ins = get(&cx->fp->argsobj);
+    LIns* args_ins;
+    if (a_ins->opcode() == LIR_int) {
+        // |arguments| is set to 0 by EnterFrame on this trace, so call to create it.
+        args_ins = newArguments();
+    } else {
+        // Generate LIR to create arguments only if it has not already been created.
+        
+        LIns* mem_ins = lir->insAlloc(sizeof(jsval));
 
-    /* FIXME inline a_ins check in js_Arguments. */
-    LIns* args[] = { a_ins, callee_ins, argc_ins, global_ins, cx_ins };
-    a_ins = lir->insCall(&js_Arguments_ci, args);
-    guard(false, lir->ins_eq0(a_ins), OOM_EXIT);
-    stack(0, a_ins);
-    set(&cx->fp->argsobj, a_ins);
+        LIns* br1 = lir->insBranch(LIR_jt, lir->ins_eq0(a_ins), NULL);
+        lir->insStorei(a_ins, mem_ins, 0);
+        LIns* br2 = lir->insBranch(LIR_j, NULL, NULL);
+
+        LIns* label1 = lir->ins0(LIR_label);
+        br1->setTarget(label1);
+
+        LIns* call_ins = newArguments();
+        lir->insStorei(call_ins, mem_ins, 0);
+
+        LIns* label2 = lir->ins0(LIR_label);
+        br2->setTarget(label2);
+
+        args_ins = lir->insLoad(LIR_ld, mem_ins, 0);
+    }
+
+    stack(0, args_ins);
+    set(&cx->fp->argsobj, args_ins);
     return JSRS_CONTINUE;
 }
 

js/src/jstracer.h

@@ -622,6 +622,23 @@ struct InterpState
     double*        deepBailSp;
 };
 
+// Arguments objects created on trace have a private value that points to an
+// instance of this struct. The struct includes a typemap that is allocated
+// as part of the object.
+struct js_ArgsPrivateNative {
+    double      *argv;
+
+    static js_ArgsPrivateNative *create(VMAllocator &alloc, unsigned argc)
+    {
+        return (js_ArgsPrivateNative*) new (alloc) char[sizeof(js_ArgsPrivateNative) + argc];
+    }
+
+    JSTraceType *typemap()
+    {
+        return (JSTraceType*) (this+1);
+    }
+};
+
 static JS_INLINE void
 js_SetBuiltinError(JSContext *cx)
 {
@@ -781,6 +798,8 @@ class TraceRecorder : public avmplus::GCObject {
     JS_REQUIRES_STACK nanojit::LIns* makeNumberInt32(nanojit::LIns* f);
     JS_REQUIRES_STACK nanojit::LIns* stringify(jsval& v);
 
+    nanojit::LIns* newArguments();
+
     JS_REQUIRES_STACK JSRecordingStatus call_imacro(jsbytecode* imacro);
 
     JS_REQUIRES_STACK JSRecordingStatus ifop();
@@ -1086,6 +1105,9 @@ js_GetBuiltinFunction(JSContext *cx, uintN index);
 extern void
 js_SetMaxCodeCacheBytes(JSContext* cx, uint32 bytes);
 
+extern bool
+js_NativeToValue(JSContext* cx, jsval& v, JSTraceType type, double* slot);
+
 #ifdef MOZ_TRACEVIS
 
 extern JS_FRIEND_API(bool)

js/src/trace-test/tests/basic/argumentsPassedToBuiltin.js

@@ -0,0 +1,14 @@
+// bug 504797
+
+function f() {
+  g(arguments, 1);
+}
+
+function g(a, b) {
+  var s = Array.prototype.slice.call(a, b);
+  assertEq(s[0] == undefined, false);
+}
+
+for (var i = 0; i < 10; ++i) {
+  f(1, 2, 3, 4);
+}