Ensure code is discarded appropriately when kicking frames into the interpreter, bug 719674. r=dvander
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 23 Jan 2012 16:50:23 -0800
changeset 87606 6c5229914ef986dc2cc98bdf9336c6b5fde26c67
parent 87605 a6a01e576efc4a0e6cbdfa9b58644e287a8ffa79
child 87607 93602e4ac4d9f8bce5594f72bfdfaa2190da2c24
push id674
push userffxbld
push dateTue, 13 Mar 2012 21:17:50 +0000
treeherdermozilla-beta@e3c4c92dec31 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs719674
milestone12.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
Ensure code is discarded appropriately when kicking frames into the interpreter, bug 719674. r=dvander
js/src/jsinfer.cpp
js/src/jsscript.cpp
js/src/methodjit/Compiler.cpp
js/src/methodjit/InvokeHelpers.cpp
js/src/methodjit/MethodJIT.h
js/src/methodjit/Retcon.cpp
js/src/methodjit/Retcon.h
js/src/methodjit/StubCalls.cpp
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -2148,20 +2148,18 @@ TypeCompartment::processPendingRecompile
 
 #ifdef JS_METHODJIT
 
     mjit::ExpandInlineFrames(cx->compartment);
 
     for (unsigned i = 0; i < pending->length(); i++) {
         const RecompileInfo &info = (*pending)[i];
         mjit::JITScript *jit = info.script->getJIT(info.constructing);
-        if (jit && jit->chunkDescriptor(info.chunkIndex).chunk) {
-            mjit::Recompiler::clearStackReferences(cx, info.script);
-            jit->destroyChunk(cx, info.chunkIndex);
-        }
+        if (jit && jit->chunkDescriptor(info.chunkIndex).chunk)
+            mjit::Recompiler::clearStackReferencesAndChunk(cx, info.script, jit, info.chunkIndex);
     }
 
 #endif /* JS_METHODJIT */
 
     cx->delete_(pending);
 }
 
 void
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -1741,17 +1741,17 @@ JSScript::ensureHasDebug(JSContext *cx)
     return true;
 }
 
 bool
 JSScript::recompileForStepMode(JSContext *cx)
 {
 #ifdef JS_METHODJIT
     if (jitNormal || jitCtor) {
-        mjit::ClearAllFrames(cx->compartment);
+        mjit::Recompiler::clearStackReferences(cx, this);
         mjit::ReleaseScriptCode(cx, this);
     }
 #endif
     return true;
 }
 
 bool
 JSScript::tryNewStepMode(JSContext *cx, uint32_t newValue)
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -727,20 +727,16 @@ MakeJITScript(JSContext *cx, JSScript *s
                      */
                     preserveChunk = true;
                 } else {
                     if (!currentEdges.append(edge))
                         return NULL;
                 }
             }
 
-            /*
-             * Watch for cross-chunk edges in a table switch. Don't handle
-             * lookup switches, as these are always stubbed.
-             */
             if (op == JSOP_TABLESWITCH) {
                 jsbytecode *pc2 = pc;
                 unsigned defaultOffset = offset + GET_JUMP_OFFSET(pc);
                 pc2 += JUMP_OFFSET_LEN;
                 jsint low = GET_JUMP_OFFSET(pc2);
                 pc2 += JUMP_OFFSET_LEN;
                 jsint high = GET_JUMP_OFFSET(pc2);
                 pc2 += JUMP_OFFSET_LEN;
@@ -763,16 +759,41 @@ MakeJITScript(JSContext *cx, JSScript *s
                         edge.target = targetOffset;
                         if (!currentEdges.append(edge))
                             return NULL;
                     }
                     pc2 += JUMP_OFFSET_LEN;
                 }
             }
 
+            if (op == JSOP_LOOKUPSWITCH) {
+                unsigned defaultOffset = offset + GET_JUMP_OFFSET(pc);
+                jsbytecode *pc2 = pc + JUMP_OFFSET_LEN;
+                unsigned npairs = GET_UINT16(pc2);
+                pc2 += UINT16_LEN;
+
+                CrossChunkEdge edge;
+                edge.source = offset;
+                edge.target = defaultOffset;
+                if (!currentEdges.append(edge))
+                    return NULL;
+
+                while (npairs) {
+                    pc2 += INDEX_LEN;
+                    unsigned targetOffset = offset + GET_JUMP_OFFSET(pc2);
+                    CrossChunkEdge edge;
+                    edge.source = offset;
+                    edge.target = targetOffset;
+                    if (!currentEdges.append(edge))
+                        return NULL;
+                    pc2 += JUMP_OFFSET_LEN;
+                    npairs--;
+                }
+            }
+
             if (unsigned(offset - chunkStart) > CHUNK_LIMIT)
                 finishChunk = true;
 
             if (nextOffset >= script->length || !analysis->maybeCode(nextOffset)) {
                 /* Ensure that chunks do not start on unreachable opcodes. */
                 preserveChunk = true;
             } else {
                 /*
@@ -2678,17 +2699,17 @@ mjit::Compiler::generateMethod()
              */
             if (script->pcCounters)
                 updatePCCounters(PC, &codeStart, &countersUpdated);
 #if defined JS_CPU_ARM /* Need to implement jump(BaseIndex) for ARM */
             frame.syncAndKillEverything();
             masm.move(ImmPtr(PC), Registers::ArgReg1);
 
             /* prepareStubCall() is not needed due to syncAndForgetEverything() */
-            INLINE_STUBCALL(stubs::TableSwitch, REJOIN_JUMP);
+            INLINE_STUBCALL(stubs::TableSwitch, REJOIN_NONE);
             frame.pop();
 
             masm.jump(Registers::ReturnReg);
 #else
             if (!jsop_tableswitch(PC))
                 return Compile_Error;
 #endif
             PC += js_GetVariableBytecodeLength(PC);
@@ -2697,17 +2718,17 @@ mjit::Compiler::generateMethod()
 
           BEGIN_CASE(JSOP_LOOKUPSWITCH)
             if (script->pcCounters)
                 updatePCCounters(PC, &codeStart, &countersUpdated);
             frame.syncAndForgetEverything();
             masm.move(ImmPtr(PC), Registers::ArgReg1);
 
             /* prepareStubCall() is not needed due to syncAndForgetEverything() */
-            INLINE_STUBCALL(stubs::LookupSwitch, REJOIN_JUMP);
+            INLINE_STUBCALL(stubs::LookupSwitch, REJOIN_NONE);
             frame.pop();
 
             masm.jump(Registers::ReturnReg);
             PC += js_GetVariableBytecodeLength(PC);
             break;
           END_CASE(JSOP_LOOKUPSWITCH)
 
           BEGIN_CASE(JSOP_CASE)
@@ -7353,17 +7374,17 @@ mjit::Compiler::jsop_tableswitch(jsbytec
     JS_ASSERT(numJumps >= 0);
 
     FrameEntry *fe = frame.peek(-1);
     if (fe->isNotType(JSVAL_TYPE_INT32) || numJumps > 256) {
         frame.syncAndForgetEverything();
         masm.move(ImmPtr(originalPC), Registers::ArgReg1);
 
         /* prepareStubCall() is not needed due to forgetEverything() */
-        INLINE_STUBCALL(stubs::TableSwitch, REJOIN_JUMP);
+        INLINE_STUBCALL(stubs::TableSwitch, REJOIN_NONE);
         frame.pop();
         masm.jump(Registers::ReturnReg);
         return true;
     }
 
     RegisterID dataReg;
     if (fe->isConstant()) {
         JS_ASSERT(fe->isType(JSVAL_TYPE_INT32));
@@ -7400,17 +7421,17 @@ mjit::Compiler::jsop_tableswitch(jsbytec
     Jump defaultCase = masm.branch32(Assembler::AboveOrEqual, dataReg, Imm32(numJumps));
     BaseIndex jumpTarget(reg, dataReg, Assembler::ScalePtr);
     masm.jump(jumpTarget);
 
     if (notInt.isSet()) {
         stubcc.linkExitDirect(notInt.get(), stubcc.masm.label());
         stubcc.leave();
         stubcc.masm.move(ImmPtr(originalPC), Registers::ArgReg1);
-        OOL_STUBCALL(stubs::TableSwitch, REJOIN_JUMP);
+        OOL_STUBCALL(stubs::TableSwitch, REJOIN_NONE);
         stubcc.masm.jump(Registers::ReturnReg);
     }
     frame.pop();
     return jumpAndRun(defaultCase, originalPC + defaultTarget);
 #endif
 }
 
 void
--- a/js/src/methodjit/InvokeHelpers.cpp
+++ b/js/src/methodjit/InvokeHelpers.cpp
@@ -867,21 +867,16 @@ js_InternalInterpret(void *returnData, v
         if (script->hasBreakpointsAt(pc))
             skipTrap = true;
         break;
 
       case REJOIN_FALLTHROUGH:
         f.regs.pc = nextpc;
         break;
 
-      case REJOIN_JUMP:
-        f.regs.pc = (jsbytecode *) returnReg;
-        JS_ASSERT(unsigned(f.regs.pc - script->code) < script->length);
-        break;
-
       case REJOIN_NATIVE:
       case REJOIN_NATIVE_LOWERED:
       case REJOIN_NATIVE_GETTER: {
         /*
          * We don't rejoin until after the native stub finishes execution, in
          * which case the return value will be in memory. For lowered natives,
          * the return value will be in the 'this' value's slot.
          */
--- a/js/src/methodjit/MethodJIT.h
+++ b/js/src/methodjit/MethodJIT.h
@@ -304,19 +304,16 @@ enum RejoinState {
      * State is coherent for the start of the current bytecode, which is a TRAP
      * that has already been invoked and should not be invoked again.
      */
     REJOIN_TRAP,
 
     /* State is coherent for the start of the next (fallthrough) bytecode. */
     REJOIN_FALLTHROUGH,
 
-    /* State is coherent for the start of the bytecode returned by the call. */
-    REJOIN_JUMP,
-
     /*
      * As for REJOIN_FALLTHROUGH, but holds a reference on the compartment's
      * orphaned native pools which needs to be reclaimed by InternalInterpret.
      * The return value needs to be adjusted if REJOIN_NATIVE_LOWERED, and
      * REJOIN_NATIVE_GETTER is for ABI calls made for property accesses.
      */
     REJOIN_NATIVE,
     REJOIN_NATIVE_LOWERED,
--- a/js/src/methodjit/Retcon.cpp
+++ b/js/src/methodjit/Retcon.cpp
@@ -451,13 +451,46 @@ Recompiler::clearStackReferences(JSConte
         }
 
         patchFrame(cx->compartment, f, script);
     }
 
     cx->compartment->types.recompilations++;
 }
 
+void
+Recompiler::clearStackReferencesAndChunk(JSContext *cx, JSScript *script,
+                                         JITScript *jit, size_t chunkIndex,
+                                         bool resetUses)
+{
+    Recompiler::clearStackReferences(cx, script);
+
+    bool releaseChunk = true;
+    if (jit->nchunks > 1) {
+        // If we are in the middle of a native call from a native or getter IC,
+        // we need to make sure all JIT code for the script is purged, as
+        // otherwise we will have orphaned the native stub but pointers to it
+        // still exist in the containing chunk.
+        for (VMFrame *f = cx->compartment->jaegerCompartment()->activeFrame();
+             f != NULL;
+             f = f->previous) {
+            if (f->fp()->script() == script) {
+                JS_ASSERT(f->stubRejoin != REJOIN_NATIVE &&
+                          f->stubRejoin != REJOIN_NATIVE_LOWERED &&
+                          f->stubRejoin != REJOIN_NATIVE_GETTER);
+                if (f->stubRejoin == REJOIN_NATIVE_PATCHED) {
+                    mjit::ReleaseScriptCode(cx, script);
+                    releaseChunk = false;
+                    break;
+                }
+            }
+        }
+    }
+
+    if (releaseChunk)
+        jit->destroyChunk(cx, chunkIndex, resetUses);
+}
+
 } /* namespace mjit */
 } /* namespace js */
 
 #endif /* JS_METHODJIT */
 
--- a/js/src/methodjit/Retcon.h
+++ b/js/src/methodjit/Retcon.h
@@ -60,19 +60,28 @@ namespace mjit {
  * and patching up those existing frames to go into the interpreter. If you
  * ever change the code associated with a JSScript, or otherwise would cause
  * existing JITed code to be incorrect, you /must/ use this to invalidate the
  * JITed code, fixing up the stack in the process.
  */
 class Recompiler {
 public:
 
+    // Clear all uses of compiled code for script on the stack. This must be
+    // followed by destroying all JIT code for the script.
     static void
     clearStackReferences(JSContext *cx, JSScript *script);
 
+    // Clear all uses of compiled code for script on the stack, along with
+    // the specified compiled chunk.
+    static void
+    clearStackReferencesAndChunk(JSContext *cx, JSScript *script,
+                                 JITScript *jit, size_t chunkIndex,
+                                 bool resetUses = true);
+
     static void
     expandInlineFrames(JSCompartment *compartment, StackFrame *fp, mjit::CallSite *inlined,
                        StackFrame *next, VMFrame *f);
 
     static void patchFrame(JSCompartment *compartment, VMFrame *f, JSScript *script);
 
 private:
 
--- a/js/src/methodjit/StubCalls.cpp
+++ b/js/src/methodjit/StubCalls.cpp
@@ -891,33 +891,18 @@ stubs::Interrupt(VMFrame &f, jsbytecode 
     if (!js_HandleExecutionInterrupt(f.cx))
         THROW();
 }
 
 void JS_FASTCALL
 stubs::RecompileForInline(VMFrame &f)
 {
     ExpandInlineFrames(f.cx->compartment);
-    Recompiler::clearStackReferences(f.cx, f.script());
-
-    bool releaseChunk = true;
-    if (f.jit()->nchunks > 1) {
-        StackFrame *fp = f.fp();
-        for (FrameRegsIter i(f.cx); !i.done(); ++i) {
-            StackFrame *xfp = i.fp();
-            if (xfp->script() == fp->script() && xfp != fp) {
-                mjit::ReleaseScriptCode(f.cx, fp->script());
-                releaseChunk = false;
-                break;
-            }
-        }
-    }
-
-    if (releaseChunk)
-        f.jit()->destroyChunk(f.cx, f.chunkIndex(), /* resetUses = */ false);
+    Recompiler::clearStackReferencesAndChunk(f.cx, f.script(), f.jit(), f.chunkIndex(),
+                                             /* resetUses = */ false);
 }
 
 void JS_FASTCALL
 stubs::Trap(VMFrame &f, uint32_t trapTypes)
 {
     Value rval;
 
     /*
@@ -1496,23 +1481,28 @@ stubs::LeaveBlock(VMFrame &f)
 
 inline void *
 FindNativeCode(VMFrame &f, jsbytecode *target)
 {
     void* native = f.fp()->script()->nativeCodeForPC(f.fp()->isConstructing(), target);
     if (native)
         return native;
 
-    CompileStatus status = CanMethodJIT(f.cx, f.script(), target, f.fp()->isConstructing(),
-                                        CompileRequest_Interpreter);
-    if (status == Compile_Error)
-        THROWV(NULL);
+    uint32_t sourceOffset = f.pc() - f.script()->code;
+    uint32_t targetOffset = target - f.script()->code;
 
-    mjit::ClearAllFrames(f.cx->compartment);
-    return target;
+    CrossChunkEdge *edges = f.jit()->edges();
+    for (size_t i = 0; i < f.jit()->nedges; i++) {
+        const CrossChunkEdge &edge = edges[i];
+        if (edge.source == sourceOffset && edge.target == targetOffset)
+            return edge.shimLabel;
+    }
+
+    JS_NOT_REACHED("Missing edge");
+    return NULL;
 }
 
 void * JS_FASTCALL
 stubs::LookupSwitch(VMFrame &f, jsbytecode *pc)
 {
     jsbytecode *jpc = pc;
     JSScript *script = f.fp()->script();