Bug 1026139 - Fix patching already patched frames in debug mode OSR. r=jandem, a=lmandel
authorShu-yu Guo <shu@rfrn.org>
Thu, 19 Jun 2014 12:22:11 -0700
changeset 208021 0bca394227a75948b90591d532b34a15ab8085fc
parent 208020 9234fae30e387995cbd70ce44043729926bdd9c2
child 208022 dba7b5fdea5c6e825251eea656731df86bc03eed
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, lmandel
bugs1026139
milestone32.0a2
Bug 1026139 - Fix patching already patched frames in debug mode OSR. r=jandem, a=lmandel
js/src/jit/BaselineDebugModeOSR.cpp
js/src/jit/IonFrames.cpp
js/src/jit/JitFrameIterator.h
--- a/js/src/jit/BaselineDebugModeOSR.cpp
+++ b/js/src/jit/BaselineDebugModeOSR.cpp
@@ -45,33 +45,31 @@ struct DebugModeOSREntry
         newStub(nullptr),
         recompInfo(nullptr),
         pcOffset(icEntry.pcOffset()),
         frameKind(icEntry.kind())
     {
 #ifdef DEBUG
         MOZ_ASSERT(pcOffset == icEntry.pcOffset());
         MOZ_ASSERT(frameKind == icEntry.kind());
+#endif
+    }
 
-        // Assert that if we have a NonOp ICEntry, that there are no unsynced
-        // slots, since such a recompile could have only been triggered from
-        // either an interrupt check or a debug trap handler.
-        //
-        // If triggered from an interrupt check, the stack should be fully
-        // synced.
-        //
-        // If triggered from a debug trap handler, we must be recompiling for
-        // toggling debug mode on->off, in which case the old baseline script
-        // should have fully synced stack at every bytecode.
-        if (frameKind == ICEntry::Kind_NonOp) {
-            PCMappingSlotInfo slotInfo;
-            jsbytecode *pc = script->offsetToPC(pcOffset);
-            oldBaselineScript->nativeCodeForPC(script, pc, &slotInfo);
-            MOZ_ASSERT(slotInfo.numUnsynced() == 0);
-        }
+    DebugModeOSREntry(JSScript *script, BaselineDebugModeOSRInfo *info)
+      : script(script),
+        oldBaselineScript(script->baselineScript()),
+        oldStub(nullptr),
+        newStub(nullptr),
+        recompInfo(nullptr),
+        pcOffset(script->pcToOffset(info->pc)),
+        frameKind(info->frameKind)
+    {
+#ifdef DEBUG
+        MOZ_ASSERT(pcOffset == script->pcToOffset(info->pc));
+        MOZ_ASSERT(frameKind == info->frameKind);
 #endif
     }
 
     DebugModeOSREntry(DebugModeOSREntry &&other)
       : script(other.script),
         oldBaselineScript(other.oldBaselineScript),
         oldStub(other.oldStub),
         newStub(other.newStub),
@@ -94,17 +92,17 @@ struct DebugModeOSREntry
                 frameKind == ICEntry::Kind_DebugEpilogue);
     }
 
     bool recompiled() const {
         return oldBaselineScript != script->baselineScript();
     }
 
     BaselineDebugModeOSRInfo *takeRecompInfo() {
-        MOZ_ASSERT(recompInfo);
+        MOZ_ASSERT(needsRecompileInfo() && recompInfo);
         BaselineDebugModeOSRInfo *tmp = recompInfo;
         recompInfo = nullptr;
         return tmp;
     }
 
     bool allocateRecompileInfo(JSContext *cx) {
         MOZ_ASSERT(needsRecompileInfo());
 
@@ -170,21 +168,34 @@ CollectOnStackScripts(JSContext *cx, con
                       DebugModeOSREntryVector &entries)
 {
     ICStub *prevFrameStubPtr = nullptr;
     bool needsRecompileHandler = false;
     for (JitFrameIterator iter(activation); !iter.done(); ++iter) {
         switch (iter.type()) {
           case JitFrame_BaselineJS: {
             JSScript *script = iter.script();
-            uint8_t *retAddr = iter.returnAddressToFp();
-            ICEntry &entry = script->baselineScript()->icEntryFromReturnAddress(retAddr);
 
-            if (!entries.append(DebugModeOSREntry(script, entry)))
-                return false;
+            if (BaselineDebugModeOSRInfo *info = iter.baselineFrame()->getDebugModeOSRInfo()) {
+                // If patching a previously patched yet unpopped frame, we can
+                // use the BaselineDebugModeOSRInfo on the frame directly to
+                // patch. Indeed, we cannot use iter.returnAddressToFp(), as
+                // it points into the debug mode OSR handler and cannot be
+                // used to look up a corresponding ICEntry.
+                //
+                // See cases F and G in PatchBaselineFrameForDebugMode.
+                if (!entries.append(DebugModeOSREntry(script, info)))
+                    return false;
+            } else {
+                // Otherwise, use the return address to look up the ICEntry.
+                uint8_t *retAddr = iter.returnAddressToFp();
+                ICEntry &entry = script->baselineScript()->icEntryFromReturnAddress(retAddr);
+                if (!entries.append(DebugModeOSREntry(script, entry)))
+                    return false;
+            }
 
             if (entries.back().needsRecompileInfo()) {
                 if (!entries.back().allocateRecompileInfo(cx))
                     return false;
 
                 needsRecompileHandler |= true;
             }
 
@@ -279,24 +290,31 @@ PatchBaselineFramesForDebugMode(JSContex
     //  B. From a VM call (interrupt handler, debugger statement handler).
     //
     // On to Off:
     //  - All the ways above.
     //  C. From the debug trap handler.
     //  D. From the debug prologue.
     //  E. From the debug epilogue.
     //
+    // Off to On to Off:
+    //  F. Undo case B above on previously patched yet unpopped frames.
+    //
+    // On to Off to On:
+    //  G. Undo cases B, C, D, or E above on previously patched yet unpopped
+    //     frames.
+    //
     // In general, we patch the return address from the VM call to return to a
     // "continuation fixer" to fix up machine state (registers and stack
     // state). Specifics on what need to be done are documented below.
     //
 
     IonCommonFrameLayout *prev = nullptr;
     size_t entryIndex = *start;
-    DebugOnly<bool> expectedDebugMode = cx->compartment()->debugMode();
+    bool expectedDebugMode = cx->compartment()->debugMode();
 
     for (JitFrameIterator iter(activation); !iter.done(); ++iter) {
         DebugModeOSREntry &entry = entries[entryIndex];
 
         switch (iter.type()) {
           case JitFrame_BaselineJS: {
             // If the script wasn't recompiled, there's nothing to patch.
             if (!entry.recompiled()) {
@@ -327,22 +345,43 @@ PatchBaselineFramesForDebugMode(JSContex
                 // directly to the IC resume address.
                 uint8_t *retAddr = bl->returnAddressForIC(bl->icEntryFromPCOffset(pcOffset));
                 SpewPatchBaselineFrame(prev->returnAddress(), retAddr, script, kind, pc);
                 prev->setReturnAddress(retAddr);
                 entryIndex++;
                 break;
             }
 
-            bool popFrameReg;
+            // Cases F and G above.
+            //
+            // We undo a previous recompile by handling cases B, C, D, and E
+            // like normal, except that we retrieved the pc information via
+            // the previous OSR debug info stashed on the frame.
+            if (BaselineDebugModeOSRInfo *info = iter.baselineFrame()->getDebugModeOSRInfo()) {
+                MOZ_ASSERT(info->pc == pc);
+                MOZ_ASSERT(info->frameKind == kind);
+
+                // Case G, might need to undo B, C, D, or E.
+                MOZ_ASSERT_IF(expectedDebugMode, (kind == ICEntry::Kind_CallVM ||
+                                                  kind == ICEntry::Kind_DebugTrap ||
+                                                  kind == ICEntry::Kind_DebugPrologue ||
+                                                  kind == ICEntry::Kind_DebugEpilogue));
+                // Case F, should only need to undo case B.
+                MOZ_ASSERT_IF(!expectedDebugMode, kind == ICEntry::Kind_CallVM);
+
+                // We will have allocated a new recompile info, so delete the
+                // existing one.
+                iter.baselineFrame()->deleteDebugModeOSRInfo();
+            }
 
             // The RecompileInfo must already be allocated so that this
             // function may be infallible.
             BaselineDebugModeOSRInfo *recompInfo = entry.takeRecompInfo();
 
+            bool popFrameReg;
             switch (kind) {
               case ICEntry::Kind_CallVM:
                 // Case B above.
                 //
                 // Patching returns from an interrupt handler or the debugger
                 // statement handler is similar in that we can resume at the
                 // next op.
                 pc += GetBytecodeLength(pc);
--- a/js/src/jit/IonFrames.cpp
+++ b/js/src/jit/IonFrames.cpp
@@ -182,37 +182,42 @@ JitFrameIterator::script() const
     JS_ASSERT(isScripted());
     if (isBaselineJS())
         return baselineFrame()->script();
     JSScript *script = ScriptFromCalleeToken(calleeToken());
     JS_ASSERT(script);
     return script;
 }
 
+uint8_t *
+JitFrameIterator::resumeAddressToFp() const
+{
+    // If we are settled on a patched BaselineFrame due to debug mode OSR, get
+    // the real return address via the stashed DebugModeOSRInfo.
+    if (isBaselineJS() && baselineFrame()->getDebugModeOSRInfo())
+        return baselineFrame()->debugModeOSRInfo()->resumeAddr;
+    return returnAddressToFp();
+}
+
 void
 JitFrameIterator::baselineScriptAndPc(JSScript **scriptRes, jsbytecode **pcRes) const
 {
     JS_ASSERT(isBaselineJS());
     JSScript *script = this->script();
     if (scriptRes)
         *scriptRes = script;
-    uint8_t *retAddr = returnAddressToFp();
+    uint8_t *retAddr = resumeAddressToFp();
 
     // If we have unwound the scope due to exception handling to a different
     // pc, the frame should behave as if it were settled on that pc.
     if (jsbytecode *overridePc = baselineFrame()->getUnwoundScopeOverridePc()) {
         *pcRes = overridePc;
         return;
     }
 
-    // If we are in the middle of a recompile handler, get the real return
-    // address as stashed in the RecompileInfo.
-    if (BaselineDebugModeOSRInfo *info = baselineFrame()->getDebugModeOSRInfo())
-        retAddr = info->resumeAddr;
-
     if (pcRes) {
         // If the return address is into the prologue entry address or just
         // after the debug prologue, then assume start of script.
         if (retAddr == script->baselineScript()->prologueEntryAddr() ||
             retAddr == script->baselineScript()->postDebugPrologueAddr())
         {
             *pcRes = script->code();
             return;
--- a/js/src/jit/JitFrameIterator.h
+++ b/js/src/jit/JitFrameIterator.h
@@ -174,16 +174,20 @@ class JitFrameIterator
     Value *actualArgs() const;
 
     // Returns the return address of the frame above this one (that is, the
     // return address that returns back to the current frame).
     uint8_t *returnAddressToFp() const {
         return returnAddressToFp_;
     }
 
+    // Returns the resume address. As above, except taking
+    // BaselineDebugModeOSRInfo into account, if present.
+    uint8_t *resumeAddressToFp() const;
+
     // Previous frame information extracted from the current frame.
     inline size_t prevFrameLocalSize() const;
     inline FrameType prevType() const;
     uint8_t *prevFp() const;
 
     // Returns the stack space used by the current frame, in bytes. This does
     // not include the size of its fixed header.
     size_t frameSize() const {