Bug 505591: trace JSOP_NAME for returned closures, r=dvander

js/src/jsbuiltins.h

@@ -116,6 +116,8 @@ struct JSTraceableNative {
 # define _JS_PTR_RETSIZE    nanojit::ARGSIZE_LO
 #endif
 
+class ClosureVarInfo;
+
 /*
  * Supported types for builtin functions.
  *
@@ -215,6 +217,7 @@ struct JSTraceableNative {
 #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_CVIPTR            _JS_CTYPE(const ClosureVarInfo *, _JS_PTR, --, --, INFALLIBLE)
 
 #define _JS_EXPAND(tokens)  tokens
 

js/src/jsinterp.cpp

@@ -3021,7 +3021,7 @@ js_Interpret(JSContext *cx)
 /********************** Here we include the operations ***********************/
 #include "jsops.cpp"
 /*****************************************************************************/
-              
+
 #if !JS_THREADED_INTERP
           default:
 #endif

js/src/jsops.cpp

@@ -39,7 +39,7 @@
  * ***** END LICENSE BLOCK ***** */
 
 /* This file needs to be included in possibly multiple places. */
-            
+
 #if JS_THREADED_INTERP
   interrupt:
 #else /* !JS_THREADED_INTERP */

js/src/jstracer.cpp

@@ -2393,30 +2393,34 @@ GetUpvarStackOnTrace(JSContext* cx, uint32 upvarLevel, int32 slot, uint32 callDe
     return GetUpvarOnTrace<UpvarStackTraits>(cx, upvarLevel, slot, callDepth, result);
 }
 
+// Parameters needed to access a value from a closure on trace.
+struct ClosureVarInfo
+{
+    jsid     id;
+    uint32   scopeIndex;
+    uint32   slot;
+    uint32   callDepth;
+    uint32   resolveFlags;
+};
+
 /*
  * Generic function to read upvars from Call objects of active heavyweight functions.
  *     callee       Callee Function object in which the upvar is accessed.
- *     scopeIndex   Number of parent steps to make from |callee| to find upvar definition.
- *                  This must be at least 1 because |callee| is a Function and we must reach a Call.
- *     slot         Slot in Call object to read.
- *     callDepth    callDepth of current point relative to trace entry.
  */
 template<typename T>
 inline uint32
-GetFromClosure(JSContext* cx, JSObject* callee, uint32 scopeIndex, uint32 slot, uint32 callDepth,
+GetFromClosure(JSContext* cx, JSObject* callee, const ClosureVarInfo* cv, double* result)
-               double* result)
 {
-    JS_ASSERT(scopeIndex >= 1);
+    JS_ASSERT(cv->scopeIndex >= 1);
     JS_ASSERT(OBJ_GET_CLASS(cx, callee) == &js_FunctionClass);
     JSObject* call = callee;
-
+    for (uint32 i = 0; i < cv->scopeIndex; ++i)
-    for (uint32 i = 0; i < scopeIndex; ++i)
         call = OBJ_GET_PARENT(cx, call);
 
     JS_ASSERT(OBJ_GET_CLASS(cx, call) == &js_CallClass);
 
     InterpState* state = cx->interpState;
-    FrameInfo** fip = state->rp + callDepth;
+    FrameInfo** fip = state->rp + cv->callDepth;
     while (--fip >= state->callstackBase) {
         FrameInfo* fi = *fip;
         if (fi->callee == call) {
@@ -2434,6 +2438,7 @@ GetFromClosure(JSContext* cx, JSObject* callee, uint32 scopeIndex, uint32 slot,
     /*
      * Here we specifically want to check the call object of the trace entry frame.
      */
+    uint32 slot = cv->slot;
     VOUCH_DOES_NOT_REQUIRE_STACK();
     if (cx->fp->callobj == call) {
         slot = T::adj_slot(cx->fp, slot);
@@ -2442,9 +2447,18 @@ GetFromClosure(JSContext* cx, JSObject* callee, uint32 scopeIndex, uint32 slot,
     }
 
     JSStackFrame* fp = (JSStackFrame*) call->getPrivate();
-    if (!fp)
+    jsval v;
-        return TT_INVALID;
+    if (fp) {
-    jsval v = T::slots(fp)[slot];
+        v = T::slots(fp)[slot];
+    } else {
+        JS_ASSERT(cv->resolveFlags != JSRESOLVE_INFER);
+        JSAutoResolveFlags rf(cx, cv->resolveFlags);
+#ifdef DEBUG
+        JSBool rv =
+#endif
+            js_GetPropertyHelper(cx, call, cv->id, JS_FALSE, &v);
+        JS_ASSERT(rv);
+    }
     JSTraceType type = getCoercedType(v);
     ValueToNative(cx, v, type, result);
     return type;
@@ -2459,10 +2473,9 @@ private:
 };
 
 uint32 JS_FASTCALL
-GetClosureArg(JSContext* cx, JSObject* callee, uint32 scopeIndex, uint32 slot, uint32 callDepth,
+GetClosureArg(JSContext* cx, JSObject* callee, const ClosureVarInfo* cv, double* result)
-              double* result)
 {
-    return GetFromClosure<ArgClosureTraits>(cx, callee, scopeIndex, slot, callDepth, result);
+    return GetFromClosure<ArgClosureTraits>(cx, callee, cv, result);
 }
 
 struct VarClosureTraits
@@ -2474,10 +2487,9 @@ private:
 };
 
 uint32 JS_FASTCALL
-GetClosureVar(JSContext* cx, JSObject* callee, uint32 scopeIndex, uint32 slot, uint32 callDepth,
+GetClosureVar(JSContext* cx, JSObject* callee, const ClosureVarInfo* cv, double* result)
-              double* result)
 {
-    return GetFromClosure<VarClosureTraits>(cx, callee, scopeIndex, slot, callDepth, result);
+    return GetFromClosure<VarClosureTraits>(cx, callee, cv, result);
 }
 
 /**
@@ -3641,7 +3653,7 @@ class SlotMap : public SlotVisitorBase
     checkType(unsigned i, JSTraceType t)
     {
         debug_only_printf(LC_TMTracer,
-                          "checkType slot %d: interp=%c typemap=%c isNum=%d promoteInt=%d\n", 
+                          "checkType slot %d: interp=%c typemap=%c isNum=%d promoteInt=%d\n",
                           i,
                           typeChar[getCoercedType(*slots[i].v)],
                           typeChar[t],
@@ -6809,10 +6821,8 @@ TraceRecorder::frameIfInRange(JSObject* obj, unsigned* depthp) const
     return NULL;
 }
 
-JS_DEFINE_CALLINFO_6(extern, UINT32, GetClosureVar, CONTEXT, OBJECT, UINT32,
+JS_DEFINE_CALLINFO_4(extern, UINT32, GetClosureVar, CONTEXT, OBJECT, CVIPTR, DOUBLEPTR, 0, 0)
-                     UINT32, UINT32, DOUBLEPTR, 0, 0)
+JS_DEFINE_CALLINFO_4(extern, UINT32, GetClosureArg, CONTEXT, OBJECT, CVIPTR, DOUBLEPTR, 0, 0)
-JS_DEFINE_CALLINFO_6(extern, UINT32, GetClosureArg, CONTEXT, OBJECT, UINT32,
-                     UINT32, UINT32, DOUBLEPTR, 0, 0)
 
 /*
  * Search the scope chain for a property lookup operation at the current PC and
@@ -6861,84 +6871,110 @@ TraceRecorder::scopeChainProp(JSObject* obj, jsval*& vp, LIns*& ins, NameResult&
     if (wasDeepAborted())
         ABORT_TRACE("deep abort from property lookup");
 
-    if (obj == obj2 && OBJ_GET_CLASS(cx, obj) == &js_CallClass) {
+    if (obj == obj2 && OBJ_GET_CLASS(cx, obj) == &js_CallClass)
-        JSStackFrame* cfp = (JSStackFrame*) obj->getPrivate();
+        return callProp(obj, obj2, prop, ATOM_TO_JSID(atom), vp, ins, nr);
-        if (cfp) {
-            JSScopeProperty* sprop = (JSScopeProperty*) prop;
-
-            uint32 setflags = (js_CodeSpec[*cx->fp->regs->pc].format & (JOF_SET | JOF_INCDEC | JOF_FOR));
-            if (setflags && (sprop->attrs & JSPROP_READONLY))
-                ABORT_TRACE("writing to a read-only property");
-
-            uintN slot = sprop->shortid;
-
-            vp = NULL;
-            uintN upvar_slot = SPROP_INVALID_SLOT;
-            if (sprop->getter == js_GetCallArg) {
-                JS_ASSERT(slot < cfp->fun->nargs);
-                vp = &cfp->argv[slot];
-                upvar_slot = slot;
-            } else if (sprop->getter == js_GetCallVar) {
-                JS_ASSERT(slot < cfp->script->nslots);
-                vp = &cfp->slots[slot];
-                upvar_slot = cx->fp->fun->nargs + slot;
-            }
-            obj2->dropProperty(cx, prop);
-            if (!vp)
-                ABORT_TRACE("dynamic property of Call object");
-
-            if (frameIfInRange(obj)) {
-                // At this point we are guaranteed to be looking at an active call object
-                // whose properties are stored in the corresponding JSStackFrame.
-                ins = get(vp);
-                nr.tracked = true;
-                return JSRS_CONTINUE;
-            }
-
-            // Compute number of scope chain links to result.
-            jsint scopeIndex = 0;
-            JSObject* tmp = JSVAL_TO_OBJECT(cx->fp->argv[-2]);
-            while (tmp != obj) {
-                tmp = OBJ_GET_PARENT(cx, tmp);
-                scopeIndex++;
-            }
-            JS_ASSERT(scopeIndex >= 1);
-
-            LIns* callee_ins = get(&cx->fp->argv[-2]);
-            LIns* outp = lir->insAlloc(sizeof(double));
-            LIns* args[] = {
-                outp,
-                INS_CONST(callDepth),
-                INS_CONST(slot),
-                INS_CONST(scopeIndex),
-                callee_ins,
-                cx_ins
-            };
-            const CallInfo* ci;
-            if (sprop->getter == js_GetCallArg)
-                ci = &GetClosureArg_ci;
-            else
-                ci = &GetClosureVar_ci;
-
-            LIns* call_ins = lir->insCall(ci, args);
-            JSTraceType type = getCoercedType(*vp);
-            guard(true,
-                  addName(lir->ins2(LIR_eq, call_ins, lir->insImm(type)),
-                          "guard(type-stable name access)"),
-                  BRANCH_EXIT);
-            ins = stackLoad(outp, type);
-            nr.tracked = false;
-            nr.obj = obj;
-            nr.scopeIndex = scopeIndex;
-            nr.sprop = sprop;
-            return JSRS_CONTINUE;
-        }
-    }
 
     obj2->dropProperty(cx, prop);
     ABORT_TRACE("fp->scopeChain is not global or active call object");
 }
 
+/*
+ * Generate LIR to access a property of a Call object.
+ */
+JS_REQUIRES_STACK JSRecordingStatus
+TraceRecorder::callProp(JSObject* obj, JSObject* obj2, JSProperty* prop, jsid id, jsval*& vp, 
+                        LIns*& ins, NameResult& nr)
+{
+    JSScopeProperty *sprop = (JSScopeProperty*) prop;
+
+    uint32 setflags = (js_CodeSpec[*cx->fp->regs->pc].format & (JOF_SET | JOF_INCDEC | JOF_FOR));
+    if (setflags && (sprop->attrs & JSPROP_READONLY))
+        ABORT_TRACE("writing to a read-only property");
+
+    uintN slot = sprop->shortid;
+
+    vp = NULL;
+    uintN upvar_slot = SPROP_INVALID_SLOT;
+    JSStackFrame* cfp = (JSStackFrame*) obj->getPrivate();
+    if (cfp) {
+        if (sprop->getter == js_GetCallArg) {
+            JS_ASSERT(slot < cfp->fun->nargs);
+            vp = &cfp->argv[slot];
+            upvar_slot = slot;
+            nr.v = *vp;
+        } else if (sprop->getter == js_GetCallVar) {
+            JS_ASSERT(slot < cfp->script->nslots);
+            vp = &cfp->slots[slot];
+            upvar_slot = cx->fp->fun->nargs + slot;
+            nr.v = *vp;
+        } else {
+            ABORT_TRACE("dynamic property of Call object");
+        }
+        obj2->dropProperty(cx, prop);
+
+        if (frameIfInRange(obj)) {
+            // At this point we are guaranteed to be looking at an active call oject
+            // whose properties are stored in the corresponding JSStackFrame.
+            ins = get(vp);
+            nr.tracked = true;
+            return JSRS_CONTINUE;
+        }
+    } else {
+#ifdef DEBUG
+        JSBool rv =
+#endif
+            js_GetPropertyHelper(cx, obj, sprop->id, JS_FALSE, &nr.v);
+        JS_ASSERT(rv);
+        obj2->dropProperty(cx, prop);
+    }
+
+    // Compute number of scope chain links to result.
+    jsint scopeIndex = 0;
+    JSObject* tmp = JSVAL_TO_OBJECT(cx->fp->argv[-2]);
+    while (tmp != obj) {
+        tmp = OBJ_GET_PARENT(cx, tmp);
+        scopeIndex++;
+    }
+    JS_ASSERT(scopeIndex >= 1);
+
+    LIns* cv_ins = lir_buf_writer->insSkip(sizeof(ClosureVarInfo));
+    ClosureVarInfo* cv = (ClosureVarInfo*) cv_ins->payload();
+    cv->id = id;
+    cv->scopeIndex = scopeIndex;
+    cv->slot = slot;
+    cv->callDepth = callDepth;
+    cv->resolveFlags = cx->resolveFlags == JSRESOLVE_INFER
+                     ? js_InferFlags(cx, 0)
+                     : cx->resolveFlags;
+
+    LIns* callee_ins = get(&cx->fp->argv[-2]);
+    LIns* outp = lir->insAlloc(sizeof(double));
+    LIns* args[] = {
+        outp,
+        INS_CONSTPTR(cv),
+        callee_ins,
+        cx_ins
+    };
+    const CallInfo* ci;
+    if (sprop->getter == js_GetCallArg)
+        ci = &GetClosureArg_ci;
+    else
+        ci = &GetClosureVar_ci;
+
+    LIns* call_ins = lir->insCall(ci, args);
+    JSTraceType type = getCoercedType(nr.v);
+    guard(true,
+          addName(lir->ins2(LIR_eq, call_ins, lir->insImm(type)),
+                  "guard(type-stable name access)"),
+          BRANCH_EXIT);
+    ins = stackLoad(outp, type);
+    nr.tracked = false;
+    nr.obj = obj;
+    nr.scopeIndex = scopeIndex;
+    nr.sprop = sprop;
+    return JSRS_CONTINUE;
+}
+
 JS_REQUIRES_STACK LIns*
 TraceRecorder::arg(unsigned n)
 {
@@ -9616,9 +9652,10 @@ TraceRecorder::incName(jsint incr, bool pre)
     LIns* v_ins;
     LIns* v_after;
     NameResult nr;
-    
+
     CHECK_STATUS(name(vp, v_ins, nr));
-    CHECK_STATUS(incHelper(*vp, v_ins, v_after, incr));
+    jsval v = nr.tracked ? *vp : nr.v;
+    CHECK_STATUS(incHelper(v, v_ins, v_after, incr));
     LIns* v_result = pre ? v_after : v_ins;
     if (nr.tracked) {
         set(vp, v_after);
@@ -9631,7 +9668,7 @@ TraceRecorder::incName(jsint incr, bool pre)
     LIns* callobj_ins = get(&cx->fp->argv[-2]);
     for (jsint i = 0; i < nr.scopeIndex; ++i)
         callobj_ins = stobj_get_parent(callobj_ins);
-    CHECK_STATUS(setCallProp(nr.obj, callobj_ins, nr.sprop, v_after, *vp));
+    CHECK_STATUS(setCallProp(nr.obj, callobj_ins, nr.sprop, v_after, v));
     stack(0, v_result);
     return JSRS_CONTINUE;
 }

js/src/jstracer.h

@@ -696,6 +696,7 @@ class TraceRecorder : public avmplus::GCObject {
         // is already in the tracker. The rest of the fields are set only if
         // |tracked| is false.
         bool             tracked;
+        jsval            v;              // current property value
         JSObject         *obj;           // Call object where name was found
         jsint            scopeIndex;     // scope chain links from callee to obj
         JSScopeProperty  *sprop;         // sprop name was resolved to
@@ -704,6 +705,7 @@ class TraceRecorder : public avmplus::GCObject {
     JS_REQUIRES_STACK nanojit::LIns* scopeChain() const;
     JS_REQUIRES_STACK JSStackFrame* frameIfInRange(JSObject* obj, unsigned* depthp = NULL) const;
     JS_REQUIRES_STACK JSRecordingStatus scopeChainProp(JSObject* obj, jsval*& vp, nanojit::LIns*& ins, NameResult& nr);
+    JS_REQUIRES_STACK JSRecordingStatus callProp(JSObject* obj, JSObject* obj2, JSProperty* sprop, jsid id, jsval*& vp, nanojit::LIns*& ins, NameResult& nr);
 
     JS_REQUIRES_STACK nanojit::LIns* arg(unsigned n);
     JS_REQUIRES_STACK void arg(unsigned n, nanojit::LIns* i);
@@ -730,7 +732,7 @@ class TraceRecorder : public avmplus::GCObject {
     JS_REQUIRES_STACK JSRecordingStatus inc(jsval& v, jsint incr, bool pre = true);
     JS_REQUIRES_STACK JSRecordingStatus inc(jsval v, nanojit::LIns*& v_ins, jsint incr,
                                             bool pre = true);
-    JS_REQUIRES_STACK JSRecordingStatus incHelper(jsval v, nanojit::LIns* v_ins, 
+    JS_REQUIRES_STACK JSRecordingStatus incHelper(jsval v, nanojit::LIns* v_ins,
                                                   nanojit::LIns*& v_after, jsint incr);
     JS_REQUIRES_STACK JSRecordingStatus incProp(jsint incr, bool pre = true);
     JS_REQUIRES_STACK JSRecordingStatus incElem(jsint incr, bool pre = true);

js/src/trace-test/tests/basic/name-inactive-del.js

@@ -0,0 +1,28 @@
+function mp(g) {
+  var ans = '';
+  for (var i = 0; i < 5; ++i) {
+    ans += g();
+  }
+
+  return ans;
+}
+
+function f() {
+  var k = 5;
+
+  function g() {
+    return k;
+  }
+
+  ans = '';
+
+  k = 6;
+  ans += mp(g);
+
+  delete k;
+  ans += mp(g);
+
+  return ans;
+}
+
+assertEq(f(), '6666666666');

js/src/trace-test/tests/basic/name-inactive-eval-del.js

@@ -0,0 +1,10 @@
+function mp(g) {
+  ans = ''
+  for (var i = 0; i < 5; ++i) {
+    ans += g();
+  }
+  return ans;
+}
+
+var f = eval("(function() { var k = 5; function g() { return k; } k = 6; mp(g); delete k; return mp(g); })");
+assertEq(f(), "66666");

js/src/trace-test/tests/basic/name-inactive-inferflags.js

@@ -0,0 +1,18 @@
+function addAccumulations(f) {
+  var a = f();
+  var b = f();
+  return a() + b();
+}
+
+function loopingAccumulator() {
+  var x = 0;
+  return function () {
+    for (var i = 0; i < 10; ++i) {
+      ++x;
+    }
+    return x;
+  }
+}
+
+var x = addAccumulations(loopingAccumulator);
+assertEq(x, 20);

js/src/trace-test/tests/basic/name-inactive.js

@@ -0,0 +1,20 @@
+function f(k) {
+  function g(j) {
+    return j + k;
+  }
+  return g;
+}
+
+g = f(10);
+var ans = '';
+for (var i = 0; i < 5; ++i) {
+  ans += g(i) + ',';
+}
+
+assertEq(ans, '10,11,12,13,14,');
+
+checkStats({
+    recorderStarted: 1,
+    recorderAborted: 0,
+    traceTriggered: 1
+});