Only set onTrace flag while running native code, not when recording (474771, r=brendan).

js/src/jscntxt.h

@@ -148,10 +148,8 @@ typedef struct JSTraceMonitor {
 
 #ifdef JS_TRACER
 # define JS_ON_TRACE(cx)            (JS_TRACE_MONITOR(cx).onTrace)
-# define JS_EXECUTING_TRACE(cx)     (JS_ON_TRACE(cx) && !JS_TRACE_MONITOR(cx).recorder)
 #else
 # define JS_ON_TRACE(cx)            JS_FALSE
-# define JS_EXECUTING_TRACE(cx)     JS_FALSE
 #endif
 
 #ifdef JS_THREADSAFE

js/src/jsinterp.cpp

@@ -2574,21 +2574,16 @@ js_Interpret(JSContext *cx)
 
 #ifdef JS_TRACER
     /* We had better not be entering the interpreter from JIT-compiled code. */
-    TraceRecorder *tr = NULL;
-    if (JS_ON_TRACE(cx)) {
-        tr = TRACE_RECORDER(cx);
+    TraceRecorder *tr = TRACE_RECORDER(cx);
+
+    /* If a recorder is pending and we try to re-enter the interpreter, flag 
+       the recorder to be destroyed when we return. */
+    if (tr) {
         SET_TRACE_RECORDER(cx, NULL);
-        JS_TRACE_MONITOR(cx).onTrace = JS_FALSE;
-        /*
-         * ON_TRACE means either recording or coming from traced code.
-         * If there's no recorder (the latter case), don't care.
-         */
-        if (tr) {
-            if (tr->wasDeepAborted())
-                tr->removeFragmentoReferences();
-            else
-                tr->pushAbortStack();
-        }
+        if (tr->wasDeepAborted())
+            tr->removeFragmentoReferences();
+        else
+            tr->pushAbortStack();
     }
 #endif
 
@@ -7089,7 +7084,6 @@ js_Interpret(JSContext *cx)
 
 #ifdef JS_TRACER
     if (tr) {
-        JS_TRACE_MONITOR(cx).onTrace = JS_TRUE;
         SET_TRACE_RECORDER(cx, tr);
         if (!tr->wasDeepAborted()) {
             tr->popAbortStack();

js/src/jsobj.cpp

@@ -3614,7 +3614,7 @@ js_FindPropertyHelper(JSContext *cx, jsid id, JSObject **objp,
     JSProperty *prop;
     JSScopeProperty *sprop;
 
-    JS_ASSERT_IF(entryp, !JS_EXECUTING_TRACE(cx));
+    JS_ASSERT_IF(entryp, !JS_ON_TRACE(cx));
     obj = js_GetTopStackFrame(cx)->scopeChain;
     shape = OBJ_SHAPE(obj);
     for (scopeIndex = 0; ; scopeIndex++) {
@@ -3891,7 +3891,7 @@ js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
         return JS_FALSE;
 
     if (entryp) {
-        JS_ASSERT_NOT_EXECUTING_TRACE(cx);
+        JS_ASSERT_NOT_ON_TRACE(cx);
         js_FillPropertyCache(cx, obj, shape, 0, protoIndex, obj2, sprop, entryp);
     }
     JS_UNLOCK_OBJ(cx, obj2);
@@ -4097,7 +4097,7 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
         return JS_FALSE;
 
     if (entryp) {
-        JS_ASSERT_NOT_EXECUTING_TRACE(cx);
+        JS_ASSERT_NOT_ON_TRACE(cx);
         if (!(attrs & JSPROP_SHARED))
             js_FillPropertyCache(cx, obj, shape, 0, 0, obj, sprop, entryp);
         else

js/src/jsstaticcheck.h

@@ -53,16 +53,16 @@ inline __attribute__ ((unused)) void MUST_FLOW_THROUGH(const char *label) {
 inline JS_FORCES_STACK void VOUCH_DOES_NOT_REQUIRE_STACK() {}
 
 inline JS_FORCES_STACK void
-JS_ASSERT_NOT_EXECUTING_TRACE(JSContext *cx)
+JS_ASSERT_NOT_ON_TRACE(JSContext *cx)
 {
-    JS_ASSERT(!JS_EXECUTING_TRACE(cx));
+    JS_ASSERT(!JS_ON_TRACE(cx));
 }
 
 #else
 #define MUST_FLOW_THROUGH(label)            ((void) 0)
 #define MUST_FLOW_LABEL(label)
 #define VOUCH_DOES_NOT_REQUIRE_STACK()      ((void) 0)
-#define JS_ASSERT_NOT_EXECUTING_TRACE(cx)   JS_ASSERT(!JS_EXECUTING_TRACE(cx))
+#define JS_ASSERT_NOT_ON_TRACE(cx)          JS_ASSERT(!JS_ON_TRACE(cx))
 #endif
 
 #endif /* jsstaticcheck_h___ */

js/src/jstracer.cpp

@@ -2854,9 +2854,6 @@ js_DeleteRecorder(JSContext* cx)
     JSTraceMonitor* tm = &JS_TRACE_MONITOR(cx);
 
     /* Aborting and completing a trace end up here. */
-    JS_ASSERT(tm->onTrace);
-    tm->onTrace = false;
-
     delete tm->recorder;
     tm->recorder = NULL;
 }
@@ -2884,15 +2881,6 @@ js_StartRecorder(JSContext* cx, VMSideExit* anchor, Fragment* f, TreeInfo* ti,
 {
     JSTraceMonitor* tm = &JS_TRACE_MONITOR(cx);
 
-    /*
-     * Emulate on-trace semantics and avoid rooting headaches while recording,
-     * by suppressing last-ditch GC attempts while recording a trace. This does
-     * means that trace recording must not nest or the following assertion will
-     * botch.
-     */
-    JS_ASSERT(!tm->onTrace);
-    tm->onTrace = true;
-
     /* start recording if no exception during construction */
     tm->recorder = new (&gc) TraceRecorder(cx, anchor, f, ti,
                                            stackSlots, ngslots, typeMap,
@@ -3775,15 +3763,12 @@ js_ExecuteTree(JSContext* cx, Fragment* f, uintN& inlineCallCount,
 #endif
 #endif
 
-    /*
-     * We may be called from js_MonitorLoopEdge while not recording, or while
-     * recording. Rather than over-generalize by using a counter instead of a
-     * flag, we simply sample and update tm->onTrace if necessary.
-     */
-    bool onTrace = tm->onTrace;
-    if (!onTrace)
-        tm->onTrace = true;
-    VMSideExit* lr;
+    /* Set a flag that indicates to the runtime system that we are running in native code
+       now and we don't want automatic GC to happen. Instead we will get a silent failure,
+       which will cause a trace exit at which point the interpreter re-tries the operation
+       and eventually triggers the GC. */
+    JS_ASSERT(!tm->onTrace);
+    tm->onTrace = true;
     
     debug_only(fflush(NULL);)
     GuardRecord* rec;
@@ -3792,13 +3777,13 @@ js_ExecuteTree(JSContext* cx, Fragment* f, uintN& inlineCallCount,
 #else
     rec = u.func(&state, NULL);
 #endif
-    lr = (VMSideExit*)rec->exit;
+    VMSideExit* lr = (VMSideExit*)rec->exit;
 
     AUDIT(traceTriggered);
 
     JS_ASSERT(lr->exitType != LOOP_EXIT || !lr->calldepth);
 
-    tm->onTrace = onTrace;
+    tm->onTrace = false;
 
     /* Except if we find that this is a nested bailout, the guard the call returned is the
        one we have to use to adjust pc and sp. */
@@ -4363,7 +4348,7 @@ js_FlushJITCache(JSContext* cx)
 JS_FORCES_STACK JSStackFrame *
 js_GetTopStackFrame(JSContext *cx)
 {
-    if (JS_EXECUTING_TRACE(cx)) {
+    if (JS_ON_TRACE(cx)) {
         /*
          * TODO: If executing a tree, synthesize stack frames and bail off
          * trace. See bug 462027.

js/src/liveconnect/nsCLiveconnect.cpp

@@ -163,7 +163,7 @@ AutoPushJSContext::AutoPushJSContext(nsISupports* aSecuritySupports,
         // See if there are any scripts on the stack.
         // If not, we need to add a dummy frame with a principal.
         JSStackFrame* tempFP = JS_GetScriptedCaller(cx, NULL);
-        JS_ASSERT_NOT_EXECUTING_TRACE(cx);
+        JS_ASSERT_NOT_ON_TRACE(cx);
 
         if (!tempFP)
         {

js/src/trace-test.js

@@ -4034,6 +4034,36 @@ function testBug474769() {
 testBug474769.expected = 1;
 test(testBug474769);
 
+function testInterpreterReentry() {
+    this.__defineSetter__('x', function(){})
+    for (var j = 0; j < 5; ++j) { x = 3; }
+    return 1;
+}
+testInterpreterReentry.expected = 1;
+test(testInterpreterReentry);
+
+function testInterpreterReentry2() {
+    var a = false;
+    var b = {};
+    var c = false;
+    var d = {};
+    this.__defineGetter__('e', function(){});
+    for (let f in this) print(f);
+    [1 for each (g in this) for each (h in [])]
+    return 1;
+}
+testInterpreterReentry2.expected = 1;
+test(testInterpreterReentry2);
+
+function testInterpreterReentry3() {
+    for (let i=0;i<5;++i) this["y" + i] = function(){};
+    this.__defineGetter__('e', function (x2) { yield; });
+    [1 for each (a in this) for (b in {})];
+    return 1;
+}
+testInterpreterReentry3.expected = 1;
+test(testInterpreterReentry3);
+
 /*****************************************************************************
  *                                                                           *
  *  _____ _   _  _____ ______ _____ _______                                  *