Set nativeVp immediately before the native call (514999, r=mrbkap).

js/src/jscntxt.h

@@ -1080,10 +1080,6 @@ struct JSContext {
      */
     InterpState         *interpState;
     VMSideExit          *bailExit;
-
-    /* Used when calling natives from trace to root the vp vector. */
-    uintN               nativeVpLen;
-    jsval               *nativeVp;
 #endif
 
 #ifdef JS_THREADSAFE

js/src/jsgc.cpp

@@ -3047,8 +3047,12 @@ js_TraceContext(JSTracer *trc, JSContext *acx)
     js_TraceRegExpStatics(trc, acx);
 
 #ifdef JS_TRACER
-    if (acx->nativeVp)
-        TRACE_JSVALS(trc, acx->nativeVpLen, acx->nativeVp, "nativeVp");
+    InterpState* state = acx->interpState;
+    while (state) {
+        if (state->nativeVp)
+            TRACE_JSVALS(trc, state->nativeVpLen, state->nativeVp, "nativeVp");
+        state = state->prev;
+    }
 #endif
 }
 

js/src/jstracer.cpp

@@ -4794,7 +4794,7 @@ SynthesizeFrame(JSContext* cx, const FrameInfo& fi, JSObject* callee)
 }
 
 static void
-SynthesizeSlowNativeFrame(JSContext *cx, VMSideExit *exit)
+SynthesizeSlowNativeFrame(InterpState& state, JSContext *cx, VMSideExit *exit)
 {
     VOUCH_DOES_NOT_REQUIRE_STACK();
 
@@ -4815,9 +4815,9 @@ SynthesizeSlowNativeFrame(JSContext *cx, VMSideExit *exit)
     fp->varobj = cx->fp->varobj;
     fp->script = NULL;
     // fp->thisp is really a jsval, so reinterpret_cast here, not JSVAL_TO_OBJECT.
-    fp->thisp = (JSObject *) cx->nativeVp[1];
-    fp->argc = cx->nativeVpLen - 2;
-    fp->argv = cx->nativeVp + 2;
+    fp->thisp = (JSObject *) state.nativeVp[1];
+    fp->argc = state.nativeVpLen - 2;
+    fp->argv = state.nativeVp + 2;
     fp->fun = GET_FUNCTION_PRIVATE(cx, JSVAL_TO_OBJECT(fp->argv[-2]));
     fp->rval = JSVAL_VOID;
     fp->down = cx->fp;
@@ -5572,6 +5572,7 @@ ExecuteTree(JSContext* cx, Fragment* f, uintN& inlineCallCount,
     state->lastTreeExitGuard = NULL;
     state->lastTreeCallGuard = NULL;
     state->rpAtLastTreeCall = NULL;
+    state->nativeVp = NULL;
     state->builtinStatus = 0;
 
     /* Set up the native global frame. */
@@ -5643,6 +5644,7 @@ ExecuteTree(JSContext* cx, Fragment* f, uintN& inlineCallCount,
     }
 
     JS_ASSERT(*(uint64*)&global[globalFrameSize] == 0xdeadbeefdeadbeefLL);
+    JS_ASSERT(!state->nativeVp);
 
     VMSideExit* lr = (VMSideExit*)rec->exit;
 
@@ -5941,7 +5943,7 @@ LeaveTree(InterpState& state, VMSideExit* lr)
     JS_ASSERT(unsigned(slots) == innermost->numStackSlots);
 
     if (innermost->nativeCalleeWord)
-        SynthesizeSlowNativeFrame(cx, innermost);
+        SynthesizeSlowNativeFrame(state, cx, innermost);
 
     /* Write back interned globals. */
     double* global = (double*)(&state + 1);
@@ -9293,8 +9295,8 @@ TraceRecorder::emitNativePropertyOp(JSScope* scope, JSScopeProperty* sprop, LIns
     // because the getter or setter could end up resizing the object's dslots.
     // Instead, use a word of stack and root it in nativeVp.
     LIns* vp_ins = lir->insAlloc(sizeof(jsval));
-    lir->insStorei(vp_ins, cx_ins, offsetof(JSContext, nativeVp));
-    lir->insStorei(INS_CONST(1), cx_ins, offsetof(JSContext, nativeVpLen));
+    lir->insStorei(vp_ins, lirbuf->state, offsetof(InterpState, nativeVp));
+    lir->insStorei(INS_CONST(1), lirbuf->state, offsetof(InterpState, nativeVpLen));
     if (setflag)
         lir->insStorei(boxed_ins, vp_ins, 0);
 
@@ -9314,7 +9316,7 @@ TraceRecorder::emitNativePropertyOp(JSScope* scope, JSScopeProperty* sprop, LIns
     LIns* ok_ins = lir->insCall(ci, args);
 
     // Cleanup. Immediately clear nativeVp before we might deep bail.
-    lir->insStorei(INS_NULL(), cx_ins, offsetof(JSContext, nativeVp));
+    lir->insStorei(INS_NULL(), lirbuf->state, offsetof(InterpState, nativeVp));
     leaveDeepBailCall();
 
     // Guard that the call succeeded and builtinStatus is still 0.
@@ -9361,7 +9363,7 @@ TraceRecorder::emitNativeCall(JSSpecializedNative* sn, uintN argc, LIns* args[],
 
     // Immediately unroot the vp as soon we return since we might deep bail next.
     if (rooted)
-        lir->insStorei(INS_NULL(), cx_ins, offsetof(JSContext, nativeVp));
+        lir->insStorei(INS_NULL(), lirbuf->state, offsetof(InterpState, nativeVp));
 
     rval_ins = res_ins;
     switch (JSTN_ERRTYPE(sn)) {
@@ -9570,9 +9572,7 @@ TraceRecorder::callNative(uintN argc, JSOp mode)
     uintN vplen = 2 + JS_MAX(argc, FUN_MINARGS(fun)) + fun->u.n.extra;
     if (!(fun->flags & JSFUN_FAST_NATIVE))
         vplen++; // slow native return value slot
-    lir->insStorei(INS_CONST(vplen), cx_ins, offsetof(JSContext, nativeVpLen));
     LIns* invokevp_ins = lir->insAlloc(vplen * sizeof(jsval));
-    lir->insStorei(invokevp_ins, cx_ins, offsetof(JSContext, nativeVp));
 
     // vp[0] is the callee.
     lir->insStorei(INS_CONSTWORD(OBJECT_TO_JSVAL(funobj)), invokevp_ins, 0);
@@ -9705,6 +9705,14 @@ TraceRecorder::callNative(uintN argc, JSOp mode)
     generatedSpecializedNative.prefix = NULL;
     generatedSpecializedNative.argtypes = NULL;
 
+    // We only have to ensure that the values we wrote into the stack buffer
+    // are rooted if we actually make it to the call, so only set nativeVp and
+    // nativeVpLen immediately before emitting the call code. This way we avoid
+    // leaving trace with a bogus nativeVp because we fall off trace while unboxing
+    // values into the stack buffer.
+    lir->insStorei(INS_CONST(vplen), lirbuf->state, offsetof(InterpState, nativeVpLen));
+    lir->insStorei(invokevp_ins, lirbuf->state, offsetof(InterpState, nativeVp));
+
     // argc is the original argc here. It is used to calculate where to place
     // the return value.
     return emitNativeCall(&generatedSpecializedNative, argc, args, true);

js/src/jstracer.h

@@ -611,15 +611,18 @@ struct InterpState
 #endif
     InterpState*   prev;
 
-    /*
-     * Used by _FAIL builtins; see jsbuiltins.h. The builtin sets the
-     * JSBUILTIN_BAILED bit if it bails off trace and the JSBUILTIN_ERROR bit
-     * if an error or exception occurred.
-     */
+    // Used by _FAIL builtins; see jsbuiltins.h. The builtin sets the
+    // JSBUILTIN_BAILED bit if it bails off trace and the JSBUILTIN_ERROR bit
+    // if an error or exception occurred.
     uint32         builtinStatus;
 
     // Used to communicate the location of the return value in case of a deep bail.
     double*        deepBailSp;
+
+
+    // Used when calling natives from trace to root the vp vector. */
+    uintN          nativeVpLen;
+    jsval         *nativeVp;
 };
 
 // Arguments objects created on trace have a private value that points to an

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

@@ -0,0 +1,13 @@
+(function () {
+   (eval("\
+       (function () {\
+           for (var y = 0; y < 16; ++y) {\
+               if (y % 3 == 2) {\
+                   gczeal(1);\
+               } else {\
+                   print(0 / 0);\
+               }\
+           }\
+       });\
+   "))()
+})();