Fix exception handling inside trace and method JIT integration (bug 597871, r=jorendorff, a=sayrer).
☠☠ backed out by b5c1497459bb ☠ ☠
authorDavid Anderson <danderson@mozilla.com>
Mon, 27 Sep 2010 09:02:08 -0700
changeset 54723 63066ec9dd8d8fa7d7c5e0ee55ebc15dd29b908d
parent 54722 660c0c8a0d34a334fccfdfcdad11ee1dc1dda8ce
child 54724 491044fa498af5b4f8167b36b9b0c8a057807ca6
child 54727 b5c1497459bb795251f0ff048b5625d077c10d15
push id16011
push userrsayre@mozilla.com
push dateWed, 29 Sep 2010 06:01:57 +0000
treeherdermozilla-central@d7e659b4f80c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, sayrer
bugs597871
milestone2.0b7pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Fix exception handling inside trace and method JIT integration (bug 597871, r=jorendorff, a=sayrer).
js/src/jsinterp.cpp
js/src/methodjit/InvokeHelpers.cpp
js/src/trace-test/tests/jaeger/bug597871.js
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -6508,17 +6508,17 @@ END_CASE(JSOP_ARRAYPUSH)
         } /* switch (op) */
     } /* for (;;) */
 #endif /* !JS_THREADED_INTERP */
 
   error:
     JS_ASSERT(cx->regs == &regs);
 #ifdef JS_TRACER
     if (regs.fp->hasImacropc() && cx->throwing) {
-        // Handle other exceptions as if they came from the imacro-calling pc.
+        // Handle exceptions as if they came from the imacro-calling pc.
         regs.pc = regs.fp->imacropc();
         regs.fp->clearImacropc();
         atoms = script->atomMap.vector;
     }
 #endif
 
     JS_ASSERT(size_t((regs.fp->hasImacropc() ? regs.fp->imacropc() : regs.pc) - script->code) <
               script->length);
--- a/js/src/methodjit/InvokeHelpers.cpp
+++ b/js/src/methodjit/InvokeHelpers.cpp
@@ -81,19 +81,16 @@ using namespace JSC;
 
 #define THROWV(v)       \
     do {                \
         void *ptr = JS_FUNC_TO_DATA_PTR(void *, JaegerThrowpoline); \
         *f.returnAddressLocation() = ptr; \
         return v;       \
     } while (0)
 
-static bool
-InlineReturn(VMFrame &f, JSBool ok);
-
 static jsbytecode *
 FindExceptionHandler(JSContext *cx)
 {
     JSStackFrame *fp = cx->fp();
     JSScript *script = fp->script();
 
 top:
     if (cx->throwing && script->trynotesOffset) {
@@ -637,56 +634,65 @@ AdvanceReturnPC(JSContext *cx)
               *cx->regs->pc == JSOP_EVAL ||
               *cx->regs->pc == JSOP_APPLY);
     cx->regs->pc += JSOP_CALL_LENGTH;
 }
 
 #ifdef JS_TRACER
 
 static inline bool
-SwallowErrors(VMFrame &f, JSStackFrame *stopFp)
+HandleErrorInExcessFrames(VMFrame &f, JSStackFrame *stopFp)
 {
     JSContext *cx = f.cx;
 
+    /*
+     * Callers of this called either Interpret() or JaegerShot(), which would
+     * have searched for exception handlers already. If we see stopFp, just
+     * return false. Otherwise, pop the frame, since it's guaranteed useless.
+     */
+    JSStackFrame *fp = cx->fp();
+    if (fp == stopFp)
+        return false;
+
+    bool returnOK = InlineReturn(f, false);
+
     /* Remove the bottom frame. */
-    bool ok = false;
     for (;;) {
-        JSStackFrame *fp = cx->fp();
+        fp = cx->fp();
 
-        /* Look for an imacro with hard-coded exception handlers. */
-        if (fp->hasImacropc() && cx->throwing) {
+        /* Clear imacros. */
+        if (fp->hasImacropc()) {
             cx->regs->pc = fp->imacropc();
             fp->clearImacropc();
-            if (ok)
-                break;
         }
         JS_ASSERT(!fp->hasImacropc());
 
         /* If there's an exception and a handler, set the pc and leave. */
-        jsbytecode *pc = FindExceptionHandler(cx);
-        if (pc) {
-            cx->regs->pc = pc;
-            ok = true;
-            break;
+        if (cx->throwing) {
+            jsbytecode *pc = FindExceptionHandler(cx);
+            if (pc) {
+                cx->regs->pc = pc;
+                returnOK = true;
+                break;
+            }
         }
 
         /* Don't unwind if this was the entry frame. */
         if (fp == stopFp)
             break;
 
         /* Unwind and return. */
-        ok &= bool(js_UnwindScope(cx, 0, cx->throwing));
-        InlineReturn(f, ok);
+        returnOK = bool(js_UnwindScope(cx, 0, returnOK || cx->throwing));
+        returnOK = InlineReturn(f, returnOK);
     }
 
-    /* Update the VMFrame before leaving. */
     JS_ASSERT(&f.regs == cx->regs);
+    JS_ASSERT_IF(returnOK, cx->fp() == stopFp);
 
-    JS_ASSERT_IF(!ok, cx->fp() == stopFp);
-    return ok;
+    return returnOK;
 }
 
 static inline bool
 AtSafePoint(JSContext *cx)
 {
     JSStackFrame *fp = cx->fp();
     if (fp->hasImacropc())
         return false;
@@ -723,36 +729,36 @@ FrameIsFinished(JSContext *cx)
     return (op == JSOP_RETURN ||
             op == JSOP_RETRVAL ||
             op == JSOP_STOP)
         ? op
         : JSOP_NOP;
 }
 
 static bool
-RemoveExcessFrames(VMFrame &f, JSStackFrame *entryFrame)
+FinishExcessFrames(VMFrame &f, JSStackFrame *entryFrame)
 {
     JSContext *cx = f.cx;
     while (cx->fp() != entryFrame || entryFrame->hasImacropc()) {
         JSStackFrame *fp = cx->fp();
 
         if (AtSafePoint(cx)) {
             JSScript *script = fp->script();
             if (!JaegerShotAtSafePoint(cx, script->nmap[cx->regs->pc - script->code])) {
-                if (!SwallowErrors(f, entryFrame))
+                if (!HandleErrorInExcessFrames(f, entryFrame))
                     return false;
 
                 /* Could be anywhere - restart outer loop. */
                 continue;
             }
             InlineReturn(f, JS_TRUE);
             AdvanceReturnPC(cx);
         } else {
             if (!PartialInterpret(f)) {
-                if (!SwallowErrors(f, entryFrame))
+                if (!HandleErrorInExcessFrames(f, entryFrame))
                     return false;
             } else if (cx->fp() != entryFrame) {
                 /*
                  * Partial interpret could have dropped us anywhere. Deduce the
                  * edge case: at a RETURN, needing to pop a frame.
                  */
                 JS_ASSERT(!cx->fp()->hasImacropc());
                 if (FrameIsFinished(cx)) {
@@ -833,17 +839,17 @@ RunTracer(VMFrame &f)
 	/* Sync up the VMFrame's view of cx->fp(). */
 	f.fp() = cx->fp();
 
     switch (tpa) {
       case TPA_Nothing:
         return NULL;
 
       case TPA_Error:
-        if (!SwallowErrors(f, entryFrame))
+        if (!HandleErrorInExcessFrames(f, entryFrame))
             THROWV(NULL);
         JS_ASSERT(!cx->fp()->hasImacropc());
         break;
 
       case TPA_RanStuff:
       case TPA_Recorded:
         break;
     }
@@ -866,18 +872,18 @@ RunTracer(VMFrame &f)
      *    move the return value down.
      * 2) The entryFrame is NOT the last inline frame. Pop the frame.
      *
      * In both cases, we hijack the stub to return to InjectJaegerReturn. This
      * moves |oldFp->rval| into the scripted return registers.
      */
 
   restart:
-    /* Step 1. Initial removal of excess frames. */
-    if (!RemoveExcessFrames(f, entryFrame))
+    /* Step 1. Finish frames created after the entry frame. */
+    if (!FinishExcessFrames(f, entryFrame))
         THROWV(NULL);
 
     /* IMacros are guaranteed to have been removed by now. */
     JS_ASSERT(!entryFrame->hasImacropc());
 
     /* Step 2. If entryFrame is at a safe point, just leave. */
     if (AtSafePoint(cx)) {
         uint32 offs = uint32(cx->regs->pc - entryFrame->script()->code);
@@ -898,17 +904,17 @@ RunTracer(VMFrame &f)
         }
         void *retPtr = JS_FUNC_TO_DATA_PTR(void *, InjectJaegerReturn);
         *f.returnAddressLocation() = retPtr;
         return NULL;
     }
 
     /* Step 4. Do a partial interp, then restart the whole process. */
     if (!PartialInterpret(f)) {
-        if (!SwallowErrors(f, entryFrame))
+        if (!HandleErrorInExcessFrames(f, entryFrame))
             THROWV(NULL);
     }
 
     goto restart;
 }
 
 #endif /* JS_TRACER */
 
new file mode 100644
--- /dev/null
+++ b/js/src/trace-test/tests/jaeger/bug597871.js
@@ -0,0 +1,8 @@
+for (a in [0, 0, 0, 0, 0, 0, 0, 0, 0]) {
+    try { (function() {
+            for each(l in [false, 0, 0, 0]) {}
+            h
+        })()
+    } catch(e) {}
+}
+