Bug 1418971 - Remove rematerialized frames after bailouts and exceptions. r=jandem a=lizzard
authorNicolas B. Pierron <nicolas.b.pierron@gmail.com>
Mon, 28 May 2018 15:41:23 +0000
changeset 480657 c2571cc8a086ca412c3797af972038ae90a392f4
parent 480656 f15db10c801ca0df2210d54a4ee2a8f6d4386e47
child 480658 f9114bfa6fcb0647f15f6f783038206f7fef34ad
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, lizzard
bugs1418971
milestone62.0
Bug 1418971 - Remove rematerialized frames after bailouts and exceptions. r=jandem a=lizzard
js/src/jit/BaselineBailouts.cpp
js/src/jit/JitFrames.cpp
js/src/vm/Stack.cpp
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -1836,16 +1836,24 @@ jit::FinishBailoutToBaseline(BaselineBai
     topFrame->setOverridePc(bailoutInfo->resumePC);
 
     jsbytecode* faultPC = bailoutInfo->faultPC;
     jsbytecode* tryPC = bailoutInfo->tryPC;
     uint32_t numFrames = bailoutInfo->numFrames;
     MOZ_ASSERT(numFrames > 0);
     BailoutKind bailoutKind = bailoutInfo->bailoutKind;
     bool checkGlobalDeclarationConflicts = bailoutInfo->checkGlobalDeclarationConflicts;
+    uint8_t* incomingStack = bailoutInfo->incomingStack;
+
+    // We have to get rid of the rematerialized frame, whether it is
+    // restored or unwound.
+    auto guardRemoveRematerializedFramesFromDebugger = mozilla::MakeScopeExit([&] {
+        JitActivation* act = cx->activation()->asJit();
+        act->removeRematerializedFramesFromDebugger(cx, incomingStack);
+    });
 
     // Free the bailout buffer.
     js_free(bailoutInfo);
     bailoutInfo = nullptr;
 
     if (topFrame->environmentChain()) {
         // Ensure the frame has a call object if it needs one. If the env chain
         // is nullptr, we will enter baseline code at the prologue so no need to do
@@ -1909,16 +1917,17 @@ jit::FinishBailoutToBaseline(BaselineBai
             }
 
             if (frameno == 0)
                 innerScript = frame->script();
 
             if (frameno == numFrames - 1) {
                 outerScript = frame->script();
                 outerFp = iter.fp();
+                MOZ_ASSERT(outerFp == incomingStack);
             }
 
             frameno++;
         }
 
         ++iter;
     }
 
@@ -1927,36 +1936,40 @@ jit::FinishBailoutToBaseline(BaselineBai
     MOZ_ASSERT(outerFp);
 
     // If we rematerialized Ion frames due to debug mode toggling, copy their
     // values into the baseline frame. We need to do this even when debug mode
     // is off, as we should respect the mutations made while debug mode was
     // on.
     JitActivation* act = cx->activation()->asJit();
     if (act->hasRematerializedFrame(outerFp)) {
-        JSJitFrameIter iter(cx->activation()->asJit());
+        JSJitFrameIter iter(act);
         size_t inlineDepth = numFrames;
         bool ok = true;
         while (inlineDepth > 0) {
             if (iter.isBaselineJS()) {
                 // We must attempt to copy all rematerialized frames over,
                 // even if earlier ones failed, to invoke the proper frame
                 // cleanup in the Debugger.
-                ok = CopyFromRematerializedFrame(cx, act, outerFp, --inlineDepth,
-                                                 iter.baselineFrame());
+                if (!CopyFromRematerializedFrame(cx, act, outerFp, --inlineDepth,
+                                                 iter.baselineFrame()))
+                {
+                    ok = false;
+                }
             }
             ++iter;
         }
 
+        if (!ok)
+            return false;
+
         // After copying from all the rematerialized frames, remove them from
         // the table to keep the table up to date.
+        guardRemoveRematerializedFramesFromDebugger.release();
         act->removeRematerializedFrame(outerFp);
-
-        if (!ok)
-            return false;
     }
 
     // If we are catching an exception, we need to unwind scopes.
     // See |SettleOnTryNote|
     if (cx->isExceptionPending() && faultPC) {
         EnvironmentIter ei(cx, topFrame, faultPC);
         UnwindEnvironment(cx, ei, tryPC);
     }
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -711,17 +711,22 @@ HandleException(ResumeFromException* rfe
                 if (!frames.more()) {
                     TraceLogStopEvent(logger, TraceLogger_IonMonkey);
                     TraceLogStopEvent(logger, TraceLogger_Scripts);
                     break;
                 }
                 ++frames;
             }
 
+            // Remove left-over state which might have been needed for bailout.
             activation->removeIonFrameRecovery(frame.jsFrame());
+            activation->removeRematerializedFrame(frame.fp());
+
+            // If invalidated, decrement the number of frames remaining on the
+            // stack for the given IonScript.
             if (invalidated)
                 ionScript->decrementInvalidationCount(cx->runtime()->defaultFreeOp());
 
         } else if (frame.isBaselineJS()) {
             // Set a flag on the frame to signal to DebugModeOSR that we're
             // handling an exception. Also ensure the frame has an override
             // pc. We clear the frame's override pc when we leave this block,
             // this is fine because we're either:
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -1712,16 +1712,18 @@ jit::JitActivation::removeRematerialized
     // Ion bailout can fail due to overrecursion and OOM. In such cases we
     // cannot honor any further Debugger hooks on the frame, and need to
     // ensure that its Debugger.Frame entry is cleaned up.
     if (!cx->realm()->isDebuggee() || !rematerializedFrames_)
         return;
     if (RematerializedFrameTable::Ptr p = rematerializedFrames_->lookup(top)) {
         for (uint32_t i = 0; i < p->value().length(); i++)
             Debugger::handleUnrecoverableIonBailoutError(cx, p->value()[i]);
+        RematerializedFrame::FreeInVector(p->value());
+        rematerializedFrames_->remove(p);
     }
 }
 
 void
 jit::JitActivation::traceRematerializedFrames(JSTracer* trc)
 {
     if (!rematerializedFrames_)
         return;