Bug 716647 - Part 2: Bailout in place instead of directly to catch on Ion exception when Debugger is on. (r=jandem)
authorShu-yu Guo <shu@rfrn.org>
Thu, 24 Apr 2014 01:59:37 -0700
changeset 198440 b30afb9de404554b547a9adba6bc483fd045a584
parent 198439 d34458e80bcbfb39b6b04214ca6dcd47c0be3027
child 198441 061ebab47be320047966424d449c90de296ea930
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs716647
milestone31.0a1
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
Bug 716647 - Part 2: Bailout in place instead of directly to catch on Ion exception when Debugger is on. (r=jandem)
js/src/jit/Bailouts.cpp
js/src/jit/Bailouts.h
js/src/jit/BaselineBailouts.cpp
js/src/jit/IonBuilder.cpp
js/src/jit/IonFrames.cpp
js/src/jit/IonTypes.h
--- a/js/src/jit/Bailouts.cpp
+++ b/js/src/jit/Bailouts.cpp
@@ -162,33 +162,59 @@ IonBailoutIterator::IonBailoutIterator(c
     current_ = (uint8_t *) frame.fp();
     type_ = JitFrame_IonJS;
     topFrameSize_ = frame.frameSize();
     snapshotOffset_ = osiIndex->snapshotOffset();
 }
 
 uint32_t
 jit::ExceptionHandlerBailout(JSContext *cx, const InlineFrameIterator &frame,
+                             ResumeFromException *rfe,
                              const ExceptionBailoutInfo &excInfo,
-                             BaselineBailoutInfo **bailoutInfo)
+                             bool *overrecursed)
 {
-    JS_ASSERT(cx->isExceptionPending());
+    // We can be propagating debug mode exceptions without there being an
+    // actual exception pending. For instance, when we return false from an
+    // operation callback like a timeout handler.
+    MOZ_ASSERT_IF(!excInfo.propagatingIonExceptionForDebugMode(), cx->isExceptionPending());
 
     cx->mainThread().ionTop = nullptr;
     JitActivationIterator jitActivations(cx->runtime());
     IonBailoutIterator iter(jitActivations, frame.frame());
     JitActivation *activation = jitActivations.activation()->asJit();
 
-    *bailoutInfo = nullptr;
-    uint32_t retval = BailoutIonToBaseline(cx, activation, iter, true, bailoutInfo, &excInfo);
-    JS_ASSERT(retval == BAILOUT_RETURN_OK ||
-              retval == BAILOUT_RETURN_FATAL_ERROR ||
-              retval == BAILOUT_RETURN_OVERRECURSED);
+    BaselineBailoutInfo *bailoutInfo = nullptr;
+    uint32_t retval = BailoutIonToBaseline(cx, activation, iter, true, &bailoutInfo, &excInfo);
+
+    if (retval == BAILOUT_RETURN_OK) {
+        MOZ_ASSERT(bailoutInfo);
+
+        // Overwrite the kind so HandleException after the bailout returns
+        // false, jumping directly to the exception tail.
+        if (excInfo.propagatingIonExceptionForDebugMode())
+            bailoutInfo->bailoutKind = Bailout_IonExceptionDebugMode;
 
-    JS_ASSERT((retval == BAILOUT_RETURN_OK) == (*bailoutInfo != nullptr));
+        rfe->kind = ResumeFromException::RESUME_BAILOUT;
+        rfe->target = cx->runtime()->jitRuntime()->getBailoutTail()->raw();
+        rfe->bailoutInfo = bailoutInfo;
+    } else {
+        // Bailout failed. If there was a fatal error, clear the
+        // exception to turn this into an uncatchable error. If the
+        // overrecursion check failed, continue popping all inline
+        // frames and have the caller report an overrecursion error.
+        MOZ_ASSERT(!bailoutInfo);
+
+        if (!excInfo.propagatingIonExceptionForDebugMode())
+            cx->clearPendingException();
+
+        if (retval == BAILOUT_RETURN_OVERRECURSED)
+            *overrecursed = true;
+        else
+            MOZ_ASSERT(retval == BAILOUT_RETURN_FATAL_ERROR);
+    }
 
     return retval;
 }
 
 // Initialize the decl env Object, call object, and any arguments obj of the current frame.
 bool
 jit::EnsureHasScopeObjects(JSContext *cx, AbstractFramePtr fp)
 {
--- a/js/src/jit/Bailouts.h
+++ b/js/src/jit/Bailouts.h
@@ -150,28 +150,62 @@ struct BaselineBailoutInfo;
 
 // Called from a bailout thunk. Returns a BAILOUT_* error code.
 uint32_t Bailout(BailoutStack *sp, BaselineBailoutInfo **info);
 
 // Called from the invalidation thunk. Returns a BAILOUT_* error code.
 uint32_t InvalidationBailout(InvalidationBailoutStack *sp, size_t *frameSizeOut,
                              BaselineBailoutInfo **info);
 
-struct ExceptionBailoutInfo
+class ExceptionBailoutInfo
 {
-    size_t frameNo;
-    jsbytecode *resumePC;
-    size_t numExprSlots;
+    size_t frameNo_;
+    jsbytecode *resumePC_;
+    size_t numExprSlots_;
+
+  public:
+    ExceptionBailoutInfo(size_t frameNo, jsbytecode *resumePC, size_t numExprSlots)
+      : frameNo_(frameNo),
+        resumePC_(resumePC),
+        numExprSlots_(numExprSlots)
+    { }
+
+    ExceptionBailoutInfo()
+      : frameNo_(0),
+        resumePC_(nullptr),
+        numExprSlots_(0)
+    { }
+
+    bool catchingException() const {
+        return !!resumePC_;
+    }
+    bool propagatingIonExceptionForDebugMode() const {
+        return !resumePC_;
+    }
+
+    size_t frameNo() const {
+        MOZ_ASSERT(catchingException());
+        return frameNo_;
+    }
+    jsbytecode *resumePC() const {
+        MOZ_ASSERT(catchingException());
+        return resumePC_;
+    }
+    size_t numExprSlots() const {
+        MOZ_ASSERT(catchingException());
+        return numExprSlots_;
+    }
 };
 
 // Called from the exception handler to enter a catch or finally block.
 // Returns a BAILOUT_* error code.
 uint32_t ExceptionHandlerBailout(JSContext *cx, const InlineFrameIterator &frame,
+                                 ResumeFromException *rfe,
                                  const ExceptionBailoutInfo &excInfo,
-                                 BaselineBailoutInfo **bailoutInfo);
+                                 bool *overrecursed);
 
 uint32_t FinishBailoutToBaseline(BaselineBailoutInfo *bailoutInfo);
 
 bool CheckFrequentBailouts(JSContext *cx, JSScript *script);
 
 } // namespace jit
 } // namespace js
 
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -479,22 +479,26 @@ static bool
 InitFromBailout(JSContext *cx, HandleScript caller, jsbytecode *callerPC,
                 HandleFunction fun, HandleScript script, IonScript *ionScript,
                 SnapshotIterator &iter, bool invalidate, BaselineStackBuilder &builder,
                 AutoValueVector &startFrameFormals, MutableHandleFunction nextCallee,
                 jsbytecode **callPC, const ExceptionBailoutInfo *excInfo)
 {
     MOZ_ASSERT(script->hasBaselineScript());
 
-    // If excInfo is non-nullptr, we are bailing out to a catch or finally block
-    // and this is the frame where we will resume. Usually the expression stack
-    // should be empty in this case but there can be iterators on the stack.
+    // Are we catching an exception?
+    bool catchingException = excInfo && excInfo->catchingException();
+
+    // If we are catching an exception, we are bailing out to a catch or
+    // finally block and this is the frame where we will resume. Usually the
+    // expression stack should be empty in this case but there can be
+    // iterators on the stack.
     uint32_t exprStackSlots;
-    if (excInfo)
-        exprStackSlots = excInfo->numExprSlots;
+    if (catchingException)
+        exprStackSlots = excInfo->numExprSlots();
     else
         exprStackSlots = iter.numAllocations() - (script->nfixed() + CountArgSlots(script, fun));
 
     builder.resetFramePushed();
 
     // Build first baseline frame:
     // +===============+
     // | PrevFramePtr  |
@@ -675,18 +679,18 @@ InitFromBailout(JSContext *cx, HandleScr
     for (uint32_t i = 0; i < script->nfixed(); i++) {
         Value slot = iter.read();
         if (!builder.writeValue(slot, "FixedValue"))
             return false;
     }
 
     // Get the pc. If we are handling an exception, resume at the pc of the
     // catch or finally block.
-    jsbytecode *pc = excInfo ? excInfo->resumePC : script->offsetToPC(iter.pcOffset());
-    bool resumeAfter = excInfo ? false : iter.resumeAfter();
+    jsbytecode *pc = catchingException ? excInfo->resumePC() : script->offsetToPC(iter.pcOffset());
+    bool resumeAfter = catchingException ? false : iter.resumeAfter();
 
     JSOp op = JSOp(*pc);
 
     // Fixup inlined JSOP_FUNCALL, JSOP_FUNAPPLY, and accessors on the caller side.
     // On the caller side this must represent like the function wasn't inlined.
     uint32_t pushedSlots = 0;
     AutoValueVector savedCallerArgs(cx);
     bool needToSaveArgs = op == JSOP_FUNAPPLY || IsGetPropPC(pc) || IsSetPropPC(pc);
@@ -769,26 +773,40 @@ InitFromBailout(JSContext *cx, HandleScr
         }
     }
 
     IonSpew(IonSpew_BaselineBailouts, "      pushing %u expression stack slots",
                                       exprStackSlots - pushedSlots);
     for (uint32_t i = pushedSlots; i < exprStackSlots; i++) {
         Value v;
 
-        // If coming from an invalidation bailout, and this is the topmost
-        // value, and a value override has been specified, don't read from the
-        // iterator. Otherwise, we risk using a garbage value.
         if (!iter.moreFrames() && i == exprStackSlots - 1 &&
             cx->runtime()->hasIonReturnOverride())
         {
+            // If coming from an invalidation bailout, and this is the topmost
+            // value, and a value override has been specified, don't read from the
+            // iterator. Otherwise, we risk using a garbage value.
             JS_ASSERT(invalidate);
             iter.skip();
             IonSpew(IonSpew_BaselineBailouts, "      [Return Override]");
             v = cx->runtime()->takeIonReturnOverride();
+        } else if (excInfo && excInfo->propagatingIonExceptionForDebugMode()) {
+            // If we are in the middle of propagating an exception from Ion by
+            // bailing to baseline due to debug mode, we might not have all
+            // the stack if we are at the newest frame.
+            //
+            // For instance, if calling |f()| pushed an Ion frame which threw,
+            // the snapshot expects the return value to be pushed, but it's
+            // possible nothing was pushed before we threw. Iterators might
+            // still be on the stack, so we can't just drop the stack.
+            MOZ_ASSERT(cx->compartment()->debugMode());
+            if (iter.moreFrames())
+                v = iter.read();
+            else
+                v = MagicValue(JS_OPTIMIZED_OUT);
         } else {
             v = iter.read();
         }
         if (!builder.writeValue(v, "StackValue"))
             return false;
     }
 
     size_t endOfBaselineJSFrameStack = builder.framePushed();
@@ -851,17 +869,17 @@ InitFromBailout(JSContext *cx, HandleScr
                 resumeAfter ? "after" : "at", (int) pcOff, js_CodeName[op],
                 PCToLineNumber(script, pc), script->filename(), (int) script->lineno());
     IonSpew(IonSpew_BaselineBailouts, "      Bailout kind: %s",
             BailoutKindString(bailoutKind));
 #endif
 
     // If this was the last inline frame, or we are bailing out to a catch or
     // finally block in this frame, then unpacking is almost done.
-    if (!iter.moreFrames() || excInfo) {
+    if (!iter.moreFrames() || catchingException) {
         // Last frame, so PC for call to next frame is set to nullptr.
         *callPC = nullptr;
 
         // If the bailout was a resumeAfter, and the opcode is monitored,
         // then the bailed out state should be in a position to enter
         // into the ICTypeMonitor chain for the op.
         bool enterMonitorChain = false;
         if (resumeAfter && (js_CodeSpec[op].format & JOF_TYPESET)) {
@@ -1315,18 +1333,31 @@ jit::BailoutIonToBaseline(JSContext *cx,
     //      |    |||||      |
     //      |    |||||      |
     //      +---------------+
 
     IonSpew(IonSpew_BaselineBailouts, "Bailing to baseline %s:%u (IonScript=%p) (FrameType=%d)",
             iter.script()->filename(), iter.script()->lineno(), (void *) iter.ionScript(),
             (int) prevFrameType);
 
-    if (excInfo)
-        IonSpew(IonSpew_BaselineBailouts, "Resuming in catch or finally block");
+    bool catchingException;
+    bool propagatingExceptionForDebugMode;
+    if (excInfo) {
+        catchingException = excInfo->catchingException();
+        propagatingExceptionForDebugMode = excInfo->propagatingIonExceptionForDebugMode();
+
+        if (catchingException)
+            IonSpew(IonSpew_BaselineBailouts, "Resuming in catch or finally block");
+
+        if (propagatingExceptionForDebugMode)
+            IonSpew(IonSpew_BaselineBailouts, "Resuming in-place for debug mode");
+    } else {
+        catchingException = false;
+        propagatingExceptionForDebugMode = false;
+    }
 
     IonSpew(IonSpew_BaselineBailouts, "  Reading from snapshot offset %u size %u",
             iter.snapshotOffset(), iter.ionScript()->snapshotsListSize());
 
     if (!excInfo)
         iter.ionScript()->incNumBailouts();
     iter.script()->updateBaselineOrIonRaw();
 
@@ -1371,23 +1402,27 @@ jit::BailoutIonToBaseline(JSContext *cx,
             TraceLogStartEvent(logger, TraceLogCreateTextId(logger, scr));
             TraceLogStartEvent(logger, TraceLogger::Baseline);
         }
 
         IonSpew(IonSpew_BaselineBailouts, "    FrameNo %d", frameNo);
 
         // If we are bailing out to a catch or finally block in this frame,
         // pass excInfo to InitFromBailout and don't unpack any other frames.
-        bool handleException = (excInfo && excInfo->frameNo == frameNo);
+        bool handleException = (catchingException && excInfo->frameNo() == frameNo);
+
+        // We also need to pass excInfo if we're bailing out in place for
+        // debug mode.
+        bool passExcInfo = handleException || propagatingExceptionForDebugMode;
 
         jsbytecode *callPC = nullptr;
         RootedFunction nextCallee(cx, nullptr);
         if (!InitFromBailout(cx, caller, callerPC, fun, scr, iter.ionScript(),
                              snapIter, invalidate, builder, startFrameFormals,
-                             &nextCallee, &callPC, handleException ? excInfo : nullptr))
+                             &nextCallee, &callPC, passExcInfo ? excInfo : nullptr))
         {
             return BAILOUT_RETURN_FATAL_ERROR;
         }
 
         if (!snapIter.moreFrames()) {
             JS_ASSERT(!callPC);
             break;
         }
@@ -1603,16 +1638,20 @@ jit::FinishBailoutToBaseline(BaselineBai
       case Bailout_ShapeGuard:
         if (!HandleShapeGuardFailure(cx, outerScript, innerScript))
             return false;
         break;
       case Bailout_BaselineInfo:
         if (!HandleBaselineInfoBailout(cx, outerScript, innerScript))
             return false;
         break;
+      case Bailout_IonExceptionDebugMode:
+        // Return false to resume in HandleException with reconstructed
+        // baseline frame.
+        return false;
       default:
         MOZ_ASSUME_UNREACHABLE("Unknown bailout kind!");
     }
 
     if (!CheckFrequentBailouts(cx, outerScript))
         return false;
 
     return true;
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -3681,44 +3681,49 @@ IonBuilder::processReturn(JSOp op)
     return processControlEnd();
 }
 
 IonBuilder::ControlStatus
 IonBuilder::processThrow()
 {
     MDefinition *def = current->pop();
 
-    if (graph().hasTryBlock()) {
-        // MThrow is not marked as effectful. This means when it throws and we
-        // are inside a try block, we could use an earlier resume point and this
-        // resume point may not be up-to-date, for example:
-        //
-        // (function() {
-        //     try {
-        //         var x = 1;
-        //         foo(); // resume point
-        //         x = 2;
-        //         throw foo;
-        //     } catch(e) {
-        //         print(x);
-        //     }
-        // ])();
-        //
-        // If we use the resume point after the call, this will print 1 instead
-        // of 2. To fix this, we create a resume point right before the MThrow.
-        //
-        // Note that this is not a problem for instructions other than MThrow
-        // because they are either marked as effectful (have their own resume
-        // point) or cannot throw a catchable exception.
-        MNop *ins = MNop::New(alloc());
-        current->add(ins);
-
-        if (!resumeAfter(ins))
-            return ControlStatus_Error;
-    }
+    // MThrow is not marked as effectful. This means when it throws and we
+    // are inside a try block, we could use an earlier resume point and this
+    // resume point may not be up-to-date, for example:
+    //
+    // (function() {
+    //     try {
+    //         var x = 1;
+    //         foo(); // resume point
+    //         x = 2;
+    //         throw foo;
+    //     } catch(e) {
+    //         print(x);
+    //     }
+    // ])();
+    //
+    // If we use the resume point after the call, this will print 1 instead
+    // of 2. To fix this, we create a resume point right before the MThrow.
+    //
+    // Note that this is not a problem for instructions other than MThrow
+    // because they are either marked as effectful (have their own resume
+    // point) or cannot throw a catchable exception.
+    //
+    // We always install this resume point (instead of only when the function
+    // has a try block) in order to handle the Debugger onExceptionUnwind
+    // hook. When we need to handle the hook, we bail out to baseline right
+    // after the throw and propagate the exception when debug mode is on. This
+    // is opposed to the normal behavior of resuming directly in the
+    // associated catch block.
+    MNop *nop = MNop::New(alloc());
+    current->add(nop);
+
+    if (!resumeAfter(nop))
+        return ControlStatus_Error;
 
     MThrow *ins = MThrow::New(alloc(), def);
     current->end(ins);
 
     // Make sure no one tries to use this block now.
     setCurrent(nullptr);
     return processControlEnd();
 }
--- a/js/src/jit/IonFrames.cpp
+++ b/js/src/jit/IonFrames.cpp
@@ -368,16 +368,36 @@ CloseLiveIterator(JSContext *cx, const I
 
 static void
 HandleExceptionIon(JSContext *cx, const InlineFrameIterator &frame, ResumeFromException *rfe,
                    bool *overrecursed)
 {
     RootedScript script(cx, frame.script());
     jsbytecode *pc = frame.pc();
 
+    bool bailedOutForDebugMode = false;
+    if (cx->compartment()->debugMode()) {
+        // If we have an exception from within Ion and the debugger is active,
+        // we do the following:
+        //
+        //   1. Bailout to baseline to reconstruct a baseline frame.
+        //   2. Resume immediately into the exception tail afterwards, and
+        //   handle the exception again with the top frame now a baseline
+        //   frame.
+        //
+        // An empty exception info denotes that we're propagating an Ion
+        // exception due to debug mode, which BailoutIonToBaseline needs to
+        // know. This is because we might not be able to fully reconstruct up
+        // to the stack depth at the snapshot, as we could've thrown in the
+        // middle of a call.
+        ExceptionBailoutInfo propagateInfo;
+        uint32_t retval = ExceptionHandlerBailout(cx, frame, rfe, propagateInfo, overrecursed);
+        bailedOutForDebugMode = retval == BAILOUT_RETURN_OK;
+    }
+
     if (!script->hasTrynotes())
         return;
 
     JSTryNote *tn = script->trynotes()->vector;
     JSTryNote *tnEnd = tn + script->trynotes()->length;
 
     uint32_t pcOffset = uint32_t(pc - script->main());
     for (; tn != tnEnd; ++tn) {
@@ -395,52 +415,31 @@ HandleExceptionIon(JSContext *cx, const 
             CloseLiveIterator(cx, frame, localSlot);
             break;
           }
 
           case JSTRY_LOOP:
             break;
 
           case JSTRY_CATCH:
-            if (cx->isExceptionPending()) {
+            if (cx->isExceptionPending() && !bailedOutForDebugMode) {
                 // Ion can compile try-catch, but bailing out to catch
                 // exceptions is slow. Reset the use count so that if we
                 // catch many exceptions we won't Ion-compile the script.
                 script->resetUseCount();
 
                 // Bailout at the start of the catch block.
                 jsbytecode *catchPC = script->main() + tn->start + tn->length;
-
-                ExceptionBailoutInfo excInfo;
-                excInfo.frameNo = frame.frameNo();
-                excInfo.resumePC = catchPC;
-                excInfo.numExprSlots = tn->stackDepth;
-
-                BaselineBailoutInfo *info = nullptr;
-                uint32_t retval = ExceptionHandlerBailout(cx, frame, excInfo, &info);
+                ExceptionBailoutInfo excInfo(frame.frameNo(), catchPC, tn->stackDepth);
+                uint32_t retval = ExceptionHandlerBailout(cx, frame, rfe, excInfo, overrecursed);
+                if (retval == BAILOUT_RETURN_OK)
+                    return;
 
-                if (retval == BAILOUT_RETURN_OK) {
-                    JS_ASSERT(info);
-                    rfe->kind = ResumeFromException::RESUME_BAILOUT;
-                    rfe->target = cx->runtime()->jitRuntime()->getBailoutTail()->raw();
-                    rfe->bailoutInfo = info;
-                    return;
-                }
-
-                // Bailout failed. If there was a fatal error, clear the
-                // exception to turn this into an uncatchable error. If the
-                // overrecursion check failed, continue popping all inline
-                // frames and have the caller report an overrecursion error.
-                JS_ASSERT(!info);
-                cx->clearPendingException();
-
-                if (retval == BAILOUT_RETURN_OVERRECURSED)
-                    *overrecursed = true;
-                else
-                    JS_ASSERT(retval == BAILOUT_RETURN_FATAL_ERROR);
+                // Error on bailout clears pending exception.
+                MOZ_ASSERT(!cx->isExceptionPending());
             }
             break;
 
           default:
             MOZ_ASSUME_UNREACHABLE("Unexpected try note");
         }
     }
 }
--- a/js/src/jit/IonTypes.h
+++ b/js/src/jit/IonTypes.h
@@ -43,33 +43,38 @@ enum BailoutKind
 
     // A bailout triggered by a bounds-check failure.
     Bailout_BoundsCheck,
 
     // A shape guard based on TI information failed.
     Bailout_ShapeGuard,
 
     // A bailout caused by invalid assumptions based on Baseline code.
-    Bailout_BaselineInfo
+    Bailout_BaselineInfo,
+
+    // A bailout to baseline from Ion on exception to handle Debugger hooks.
+    Bailout_IonExceptionDebugMode,
 };
 
 inline const char *
 BailoutKindString(BailoutKind kind)
 {
     switch (kind) {
       case Bailout_Normal:
         return "Bailout_Normal";
       case Bailout_ArgumentCheck:
         return "Bailout_ArgumentCheck";
       case Bailout_BoundsCheck:
         return "Bailout_BoundsCheck";
       case Bailout_ShapeGuard:
         return "Bailout_ShapeGuard";
       case Bailout_BaselineInfo:
         return "Bailout_BaselineInfo";
+      case Bailout_IonExceptionDebugMode:
+        return "Bailout_IonExceptionDebugMode";
       default:
         MOZ_ASSUME_UNREACHABLE("Invalid BailoutKind");
     }
 }
 
 static const uint32_t ELEMENT_TYPE_BITS = 5;
 static const uint32_t ELEMENT_TYPE_SHIFT = 0;
 static const uint32_t ELEMENT_TYPE_MASK = (1 << ELEMENT_TYPE_BITS) - 1;