[INFER] Ensure that inlined scripts always have JIT code for expanding, bug 645666.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 29 Mar 2011 08:30:05 -0700
changeset 74863 f6a77f725bbca8a3b25038a400c092cf9b2645fa
parent 74862 85c24589029cbf7d4bc4e1b6b40d006c23303e58
child 74864 cfeb40109a60c284b7c8471fb2363253c0b5c818
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs645666
milestone2.0b13pre
[INFER] Ensure that inlined scripts always have JIT code for expanding, bug 645666.
js/src/jit-test/tests/jaeger/inline/bug645666.js
js/src/jscompartment.cpp
js/src/jsscript.h
js/src/methodjit/Compiler.cpp
js/src/methodjit/Compiler.h
js/src/methodjit/Retcon.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/inline/bug645666.js
@@ -0,0 +1,16 @@
+function f1() {
+    gc();
+}
+function f2() {
+    with(this) {};
+    f1();
+}
+function f3() {
+    f2();
+}
+function f4() {
+    f3();
+}
+f3();
+f3();
+f4();
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -415,16 +415,17 @@ JSCompartment::wrap(JSContext *cx, AutoI
     for (size_t n = 0; n < size_t(length); ++n) {
         if (!wrapId(cx, &vector[n]))
             return false;
     }
     return true;
 }
 
 #if defined JS_METHODJIT && defined JS_MONOIC
+
 /*
  * Check if the pool containing the code for jit should be destroyed, per the
  * heuristics in JSCompartment::sweep.
  */
 static inline bool
 ScriptPoolDestroyed(JSContext *cx, mjit::JITScript *jit,
                     uint32 releaseInterval, uint32 &counter)
 {
@@ -439,17 +440,44 @@ ScriptPoolDestroyed(JSContext *cx, mjit:
         pool->m_gcNumber = cx->runtime->gcNumber;
         if (--counter == 0) {
             pool->m_destroy = true;
             counter = releaseInterval;
         }
     }
     return pool->m_destroy;
 }
-#endif
+
+static inline void
+ScriptTryDestroyCode(JSContext *cx, JSScript *script, mjit::JITScript *jit,
+                     uint32 releaseInterval, uint32 &counter)
+{
+    /*
+     * Check if the JIT code for script should be destroyed. When JIT code has
+     * inlined frames, destroy the outer script if any of the inner scripts
+     * will need to be destroyed, preserving the invariant that we always have
+     * JIT code for any inlined frame which may need to be expanded.
+     */
+
+    if (ScriptPoolDestroyed(cx, jit, releaseInterval, counter)) {
+        mjit::ReleaseScriptCode(cx, script);
+        return;
+    }
+
+    for (unsigned i = 0; i < jit->nInlineFrames; i++) {
+        JSScript *inner = jit->inlineFrames()[i].fun->script();
+        JS_ASSERT(inner->jitNormal);
+        if (ScriptPoolDestroyed(cx, inner->jitNormal, releaseInterval, counter)) {
+            mjit::ReleaseScriptCode(cx, script);
+            return;
+        }
+    }
+}
+
+#endif // JS_METHODJIT && JS_MONOIC
 
 /*
  * This method marks pointers that cross compartment boundaries. It should be
  * called only for per-compartment GCs, since full GCs naturally follow pointers
  * across compartments.
  */
 void
 JSCompartment::markCrossCompartmentWrappers(JSTracer *trc)
@@ -536,25 +564,20 @@ JSCompartment::sweep(JSContext *cx, uint
     bool discardScripts = !active && releaseInterval != 0;
 
     for (JSCList *cursor = scripts.next; cursor != &scripts; cursor = cursor->next) {
         JSScript *script = reinterpret_cast<JSScript *>(cursor);
 
         if (script->hasJITCode()) {
             mjit::ic::SweepCallICs(cx, script, discardScripts);
             if (discardScripts) {
-                if (script->jitNormal &&
-                    ScriptPoolDestroyed(cx, script->jitNormal, releaseInterval, counter)) {
-                    mjit::ReleaseScriptCode(cx, script);
-                    continue;
-                }
-                if (script->jitCtor &&
-                    ScriptPoolDestroyed(cx, script->jitCtor, releaseInterval, counter)) {
-                    mjit::ReleaseScriptCode(cx, script);
-                }
+                if (script->jitNormal)
+                    ScriptTryDestroyCode(cx, script, script->jitNormal, releaseInterval, counter);
+                if (script->jitCtor)
+                    ScriptTryDestroyCode(cx, script, script->jitCtor, releaseInterval, counter);
             }
         }
     }
 
 #endif
 
     if (!types.inferenceDepth && types.inferenceEnabled) {
         for (JSCList *cursor = scripts.next; cursor != &scripts; cursor = cursor->next) {
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -458,16 +458,17 @@ struct JSScript {
     bool            hasSingletons:1;  /* script has singleton objects */
     bool            isCachedEval:1;   /* script came from eval() */
     bool            isUncachedEval:1; /* script came from EvaluateScript */
     bool            calledWithNew:1;  /* script has been called using 'new' */
     bool            analyzed:1;       /* script has been analyzed by type inference */
 #ifdef JS_METHODJIT
     bool            debugMode:1;      /* script was compiled in debug mode */
     bool            singleStepMode:1; /* compile script in single-step mode */
+    bool            inlineParents:1;  /* script may be inlined in other frames */
 #endif
 
     jsbytecode      *main;      /* main entry point, after predef'ing prolog */
     JSAtomMap       atomMap;    /* maps immediate index to literal struct */
     JSCompartment   *compartment; /* compartment the script was compiled for */
     const char      *filename;  /* source filename or null */
     uint32          lineno;     /* base line number of script */
     uint16          nslots;     /* vars plus maximum stack depth */
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -84,24 +84,25 @@ static const char *OpcodeNames[] = {
 #endif
 
 /*
  * Number of times a script must be called or had a backedge before we try to
  * inline its calls.
  */
 static const size_t CALLS_BACKEDGES_BEFORE_INLINING = 10000;
 
-mjit::Compiler::Compiler(JSContext *cx, JSStackFrame *fp,
+mjit::Compiler::Compiler(JSContext *cx, JSScript *outerScript,
+                         bool isConstructing, bool isEval, JSObject *globalObj,
                          const Vector<PatchableFrame> *patchFrames, bool recompiling)
   : BaseCompiler(cx),
-    fp(fp), outerScript(fp->script()),
+    outerScript(outerScript),
+    isConstructing(isConstructing),
+    isEval(isEval),
+    globalObj(globalObj),
     patchFrames(patchFrames),
-    scopeChain(&fp->scopeChain()),
-    globalObj(scopeChain->getGlobal()),
-    isConstructing(fp->isConstructing()),
     savedTraps(NULL),
     frame(cx, *this, masm, stubcc),
     a(NULL), outer(NULL), script(NULL), PC(NULL),
     inlineFrames(CompilerAllocPolicy(cx, *thisFromCtor())),
     branchPatches(CompilerAllocPolicy(cx, *thisFromCtor())),
 #if defined JS_MONOIC
     getGlobalNames(CompilerAllocPolicy(cx, *thisFromCtor())),
     setGlobalNames(CompilerAllocPolicy(cx, *thisFromCtor())),
@@ -278,18 +279,16 @@ mjit::Compiler::popActiveFrame()
                 return Compile_Error;                                \
             return status_;                                          \
         }                                                            \
     JS_END_MACRO
 
 CompileStatus
 mjit::Compiler::performCompilation(JITScript **jitp)
 {
-    outerScript = fp->script();
-
     JaegerSpew(JSpew_Scripts, "compiling script (file \"%s\") (line \"%d\") (length \"%d\")\n",
                outerScript->filename, outerScript->lineno, outerScript->length);
 
     if (inlining) {
         JaegerSpew(JSpew_Inlining, "inlining calls in script (file \"%s\") (line \"%d\")\n",
                    outerScript->filename, outerScript->lineno);
     }
 
@@ -319,16 +318,49 @@ mjit::Compiler::performCompilation(JITSc
                (*jitp)->code.m_code.executableAddress(), (*jitp)->code.m_size);
 
     if (!cx->compartment->types.checkPendingRecompiles(cx))
         return Compile_Error;
 
     if (!*jitp)
         return Compile_Abort;
 
+    /*
+     * Make sure any inlined scripts have JIT code associated that we can
+     * rejoin into if we expand the inlined frames.
+     */
+    for (unsigned i = 0; i < (*jitp)->nInlineFrames; i++) {
+        JSScript *script = (*jitp)->inlineFrames()[i].fun->script();
+
+        script->inlineParents = true;
+
+        /* We should have bailed out while inlining if the script is unjittable. */
+        JS_ASSERT(script->jitArityCheckNormal != JS_UNJITTABLE_SCRIPT);
+
+        if (script->jitNormal && !script->jitNormal->rejoinPoints) {
+            mjit::Recompiler recompiler(cx, script);
+            if (!recompiler.recompile()) {
+                ReleaseScriptCode(cx, outerScript);
+                return Compile_Error;
+            }
+        }
+
+        if (!script->jitNormal) {
+            CompileStatus status = Compile_Retry;
+            while (status == Compile_Retry) {
+                mjit::Compiler cc(cx, script, isConstructing, false, globalObj, NULL, true);
+                status = cc.compile();
+            }
+            if (status != Compile_Okay) {
+                ReleaseScriptCode(cx, outerScript);
+                return status;
+            }
+        }
+    }
+
     return Compile_Okay;
 }
 
 #undef CHECK_STATUS
 
 mjit::Compiler::ActiveFrame::ActiveFrame(JSContext *cx)
     : parent(NULL), parentPC(NULL), script(NULL), inlineIndex(uint32(-1)),
       jumpMap(NULL), hasThisType(false), argumentTypes(NULL), localTypes(NULL),
@@ -412,17 +444,18 @@ mjit::TryCompile(JSContext *cx, JSStackF
     if (fp->isConstructing() && !fp->script()->nslots)
         fp->script()->nslots++;
 
     // If there were recoverable compilation failures in the function from
     // static overflow or bad inline callees, try recompiling a few times
     // before giving up.
     CompileStatus status = Compile_Retry;
     for (unsigned i = 0; status == Compile_Retry && i < 5; i++) {
-        Compiler cc(cx, fp, NULL, false);
+        Compiler cc(cx, fp->script(), fp->isConstructing(), fp->isEvalFrame(),
+                    fp->scopeChain().getGlobal(), NULL, fp->script()->inlineParents);
         status = cc.compile();
     }
 
     return status;
 }
 
 bool
 mjit::Compiler::loadOldTraps(const Vector<CallSite> &sites)
@@ -696,16 +729,22 @@ mjit::Compiler::finishThisUp(JITScript *
         return Compile_Error;
     }
 
     JITScript *jit = new(cursor) JITScript;
     cursor += sizeof(JITScript);
 
     JS_ASSERT(outerScript == script);
 
+    /*
+     * We always need to remit rejoin points when compiling a script with inline parents,
+     * so we can expand inline frames at any point.
+     */
+    JS_ASSERT_IF(outerScript->inlineParents, recompiling);
+
     jit->script = script;
     jit->code = JSC::MacroAssemblerCodeRef(result, execPool, masm.size() + stubcc.size());
     jit->invokeEntry = result;
     jit->singleStepMode = script->singleStepMode;
     jit->rejoinPoints = recompiling;
     if (script->fun) {
         jit->arityCheckEntry = stubCode.locationOf(arityLabel).executableAddress();
         jit->fastEntry = fullCode.locationOf(invokeLabel).executableAddress();
@@ -2897,17 +2936,17 @@ mjit::Compiler::emitReturn(FrameEntry *f
 
             stubcc.leave();
             OOL_STUBCALL(stubs::PutActivationObjects);
 
             emitReturnValue(&stubcc.masm, fe);
             emitFinalReturn(stubcc.masm);
         }
     } else {
-        if (fp->isEvalFrame() && script->strictModeCode) {
+        if (isEval && script->strictModeCode) {
             /* There will always be a call object. */
             prepareStubCall(Uses(fe ? 1 : 0));
             INLINE_STUBCALL(stubs::PutStrictEvalCallObject);
         }
     }
 
     emitReturnValue(&masm, fe);
     emitFinalReturn(masm);
@@ -3570,40 +3609,41 @@ mjit::Compiler::inlineScriptedFunction(u
         JS_ASSERT(object);
 
         if (!object->singleton || !object->singleton->isFunction())
             return Compile_InlineAbort;
 
         JSFunction *fun = object->singleton->getFunctionPrivate();
         if (!fun->isInterpreted())
             return Compile_InlineAbort;
+        JSScript *script = fun->script();
 
         /*
          * The outer and inner scripts must have the same scope. This only
          * allows us to inline calls between non-inner functions. Also check
          * for consistent strictness between the functions.
          */
         if (!outerScript->compileAndGo ||
             (outerScript->fun && outerScript->fun->getParent() != globalObj) ||
-            !fun->script()->compileAndGo ||
+            !script->compileAndGo ||
             fun->getParent() != globalObj ||
-            outerScript->strictModeCode != fun->script()->strictModeCode) {
+            outerScript->strictModeCode != script->strictModeCode) {
             return Compile_InlineAbort;
         }
 
         /* We can't cope with inlining recursive functions yet. */
         ActiveFrame *checka = a;
         while (checka) {
-            if (checka->script == fun->script())
+            if (checka->script == script)
                 return Compile_InlineAbort;
             checka = checka->parent;
         }
 
         analyze::Script analysis;
-        analysis.analyze(cx, fun->script());
+        analysis.analyze(cx, script);
 
         if (analysis.OOM())
             return Compile_Error;
         if (analysis.failed())
             return Compile_Abort;
 
         if (!analysis.inlineable(argc))
             return Compile_InlineAbort;
@@ -4208,17 +4248,17 @@ mjit::Compiler::jsop_callprop_str(JSAtom
 
     /*
      * Bake in String.prototype. This is safe because of compileAndGo.
      * We must pass an explicit scope chain only because JSD calls into
      * here via the recompiler with a dummy context, and we need to use
      * the global object for the script we are now compiling.
      */
     JSObject *obj;
-    if (!js_GetClassPrototype(cx, &fp->scopeChain(), JSProto_String, &obj))
+    if (!js_GetClassPrototype(cx, globalObj, JSProto_String, &obj))
         return false;
 
     /* Force into a register because getprop won't expect a constant. */
     RegisterID reg = frame.allocReg();
 
     masm.move(ImmPtr(obj), reg);
     frame.pushTypedPayload(JSVAL_TYPE_OBJECT, reg);
 
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -346,25 +346,25 @@ class Compiler : public BaseCompiler
         size_t offsetIndex;
     };
 
     struct LoopEntry {
         uint32 pcOffset;
         Label label;
     };
 
-    JSStackFrame *fp;
     JSScript *outerScript;
+    bool isConstructing;
+    bool isEval;
+
+    JSObject *globalObj;
 
     /* Existing frames on the stack whose slots may need to be updated. */
     const Vector<PatchableFrame> *patchFrames;
 
-    JSObject *scopeChain;
-    JSObject *globalObj;
-    bool isConstructing;
     bool *savedTraps;
     Assembler masm;
     FrameState frame;
 
     /*
      * State for the current stack frame.
      *
      * When inlining function calls, we keep track of the state of each inline
@@ -447,17 +447,19 @@ class Compiler : public BaseCompiler
     Compiler *thisFromCtor() { return this; }
 
     friend class CompilerAllocPolicy;
   public:
     // Special atom index used to indicate that the atom is 'length'. This
     // follows interpreter usage in JSOP_LENGTH.
     enum { LengthAtomIndex = uint32(-2) };
 
-    Compiler(JSContext *cx, JSStackFrame *fp, const Vector<PatchableFrame> *patchFrames, bool recompiling);
+    Compiler(JSContext *cx, JSScript *outerScript, bool isConstructing, bool isEval,
+             JSObject *globalObj,
+             const Vector<PatchableFrame> *patchFrames, bool recompiling);
     ~Compiler();
 
     CompileStatus compile();
 
     Label getLabel() { return masm.label(); }
     bool knownJump(jsbytecode *pc);
     Label labelOf(jsbytecode *target, uint32 inlineIndex);
     void addCallSite(const InternalCallSite &callSite);
--- a/js/src/methodjit/Retcon.cpp
+++ b/js/src/methodjit/Retcon.cpp
@@ -247,34 +247,24 @@ Recompiler::expandInlineFrameChain(JSCon
         }
     }
 
     JSStackFrame *fp = (JSStackFrame *) ((uint8 *)outer + sizeof(Value) * inner->depth);
     fp->initInlineFrame(inner->fun, parent, inner->parentpc);
     uint32 pcOffset = inner->parentpc - parent->script()->code;
 
     /*
-     * The erased frame needs JIT code with rejoin points added. Note that the
-     * outer frame does not need to have rejoin points, as it is definitely at
-     * an inline call and rejoin points are always added for such calls.
+     * We should have ensured during compilation that the erased frame has JIT
+     * code with rejoin points added. We don't try to compile such code on
+     * demand as this can trigger recompilations and a reentrant invocation of
+     * expandInlineFrames. Note that the outer frame does not need to have
+     * rejoin points, as it is definitely at an inline call and rejoin points
+     * are always added for such calls.
      */
-    if (fp->jit() && !fp->jit()->rejoinPoints) {
-        mjit::Recompiler recompiler(cx, fp->script());
-        if (!recompiler.recompile())
-            return NULL; // FIXME
-    }
-    if (!fp->jit()) {
-        CompileStatus status = Compile_Retry;
-        while (status == Compile_Retry) {
-            mjit::Compiler cc(cx, fp, NULL, true);
-            status = cc.compile();
-        }
-        if (status != Compile_Okay)
-            return NULL; // FIXME
-    }
+    JS_ASSERT(fp->jit() && fp->jit()->rejoinPoints);
 
     PatchableAddress patch;
     patch.location = fp->addressOfNativeReturnAddress();
     patch.callSite.initialize(0, uint32(-1), pcOffset, CallSite::NCODE_RETURN_ID);
     applyPatch(parent->jit(), patch);
 
     return fp;
 }
@@ -517,16 +507,27 @@ Recompiler::recompile()
     }
 
     if (ctorFrames.length() &&
         !recompile(ctorFrames, ctorPatches, ctorSites, ctorNatives,
                    ctorRecompilations)) {
         return false;
     }
 
+    /* Make sure that scripts with inline parents still have JIT code. */
+    if (script->inlineParents && !script->jitNormal) {
+        CompileStatus status = Compile_Retry;
+        while (status == Compile_Retry) {
+            mjit::Compiler cc(cx, script, false, false, script->global, NULL, true);
+            status = cc.compile();
+        }
+        if (status != Compile_Okay)
+            return false;
+    }
+
     return true;
 }
 
 bool
 Recompiler::cleanup(JITScript *jit, Vector<CallSite> *sites, uint32 *recompilations)
 {
     while (!JS_CLIST_IS_EMPTY(&jit->callers)) {
         JaegerSpew(JSpew_Recompile, "Purging IC caller\n");
@@ -561,17 +562,18 @@ Recompiler::recompile(Vector<PatchableFr
 {
     JSStackFrame *fp = frames[0].fp;
 
     JaegerSpew(JSpew_Recompile, "On stack recompilation, %u patches, %u natives\n",
                patches.length(), natives.length());
 
     CompileStatus status = Compile_Retry;
     while (status == Compile_Retry) {
-        Compiler cc(cx, fp, &frames, true);
+        Compiler cc(cx, fp->script(), fp->isConstructing(), fp->isEvalFrame(),
+                    fp->scopeChain().getGlobal(), &frames, true);
         if (!cc.loadOldTraps(sites))
             return false;
         status = cc.compile();
     }
     if (status != Compile_Okay)
         return false;
 
     JITScript *jit = script->getJIT(fp->isConstructing());