[INFER] Reserve and check extra space when inlining frames, bug 646004.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 29 Mar 2011 17:45:14 -0700
changeset 74866 9575a4e04a4e9b2ab3599caf9f04929afdb36229
parent 74865 2d030f5157e584940335880a3a5d99d05796d13f
child 74867 453c2dcce09ef0ec9cd060ee3eb10783ef4b6a93
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs646004
milestone2.0b13pre
[INFER] Reserve and check extra space when inlining frames, bug 646004.
js/src/jit-test/tests/jaeger/inline/bug646004.js
js/src/jscntxt.h
js/src/jscntxtinlines.h
js/src/methodjit/Compiler.cpp
js/src/methodjit/InvokeHelpers.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/inline/bug646004.js
@@ -0,0 +1,9 @@
+function reportCompare (expected, actual, description) {}
+function f()
+{
+  f(f, 0x09AA, 0x09B0, f);
+}
+try {
+  reportCompare ("outer", f(),
+                 "Inner function statement should not have been called.");
+} catch (e) {}
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -651,16 +651,22 @@ class StackSpace
      *
      * Worst case, if an average size script (<=9 slots) over recurses, it'll
      * effectively be the same as having increased the old inline call count
      * to <= 5,000.
      */
     static const size_t STACK_QUOTA    = (VALUES_PER_STACK_FRAME + 18) *
                                          JS_MAX_INLINE_CALL_COUNT;
 
+    /*
+     * Extra space to reserve on the stack before invoking the method JIT.
+     * This may be used for inlined stack frames.
+     */
+    static const size_t STACK_EXTRA    = (VALUES_PER_STACK_FRAME + 18) * 10;
+
     /* Kept as a member of JSThreadData; cannot use constructor/destructor. */
     bool init();
     void finish();
 
 #ifdef DEBUG
     template <class T>
     bool contains(T *t) const {
         char *v = (char *)t;
--- a/js/src/jscntxtinlines.h
+++ b/js/src/jscntxtinlines.h
@@ -282,18 +282,18 @@ InvokeArgsGuard::~InvokeArgsGuard()
 template <class Check>
 JS_REQUIRES_STACK JS_ALWAYS_INLINE JSStackFrame *
 StackSpace::getCallFrame(JSContext *cx, Value *firstUnused, uintN nactual,
                          JSFunction *fun, JSScript *script, uint32 *flags,
                          Check check) const
 {
     JS_ASSERT(fun->script() == script);
 
-    /* Include an extra sizeof(JSStackFrame) for the method-jit. */
-    uintN nvals = VALUES_PER_STACK_FRAME + script->nslots;
+    /* Include extra space for inlining by the method-jit. */
+    uintN nvals = STACK_EXTRA + script->nslots;
     uintN nformal = fun->nargs;
 
     /* Maintain layout invariant: &formalArgs[0] == ((Value *)fp) - nformal. */
 
     if (nactual == nformal) {
         if (JS_UNLIKELY(!check(*this, cx, firstUnused, nvals)))
             return NULL;
         return reinterpret_cast<JSStackFrame *>(firstUnused);
@@ -442,22 +442,22 @@ StackSpace::getStackLimit(JSContext *cx)
      * reserve the minimum required space: enough for the nslots + an
      * additional stack frame.
      */
 #ifdef XP_WIN
     if (JS_LIKELY(limit <= commitEnd))
         return limit;
     if (ensureSpace(NULL /* don't report error */, sp, STACK_QUOTA))
         return limit;
-    uintN minimum = cx->fp()->numSlots() + VALUES_PER_STACK_FRAME;
+    uintN minimum = cx->fp()->numSlots() + STACK_EXTRA;
     return ensureSpace(cx, sp, minimum) ? sp + minimum : NULL;
 #else
     if (JS_LIKELY(limit <= end))
         return limit;
-    uintN minimum = cx->fp()->numSlots() + VALUES_PER_STACK_FRAME;
+    uintN minimum = cx->fp()->numSlots() + STACK_EXTRA;
     return ensureSpace(cx, sp, minimum) ? sp + minimum : NULL;
 #endif
 }
 
 JS_REQUIRES_STACK inline
 FrameRegsIter::FrameRegsIter(JSContext *cx)
   : cx(cx)
 {
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -533,24 +533,23 @@ mjit::Compiler::generatePrologue()
                 this->argsCheckFallthrough = stubcc.masm.label();
 #endif
             }
 
             stubcc.crossJump(stubcc.masm.jump(), fastPath);
         }
 
         /*
-         * Guard that there is enough stack space. Note we include the size of
-         * a second frame, to ensure we can create a frame from call sites.
-         * :FIXME: this check does not currently account for space from inlined frames,
-         * nor do checks made when pushing the frame from the interpreter.
+         * Guard that there is enough stack space. Note we reserve space for
+         * any inline frames we end up generating, or a callee's stack frame
+         * we write to before the callee checks the stack.
          */
-        masm.addPtr(Imm32((script->nslots + VALUES_PER_STACK_FRAME * 2) * sizeof(Value)),
-                    JSFrameReg,
-                    Registers::ReturnReg);
+        JS_STATIC_ASSERT(StackSpace::STACK_EXTRA >= VALUES_PER_STACK_FRAME);
+        uint32 nvals = script->nslots + VALUES_PER_STACK_FRAME + StackSpace::STACK_EXTRA;
+        masm.addPtr(Imm32(nvals * sizeof(Value)), JSFrameReg, Registers::ReturnReg);
         Jump stackCheck = masm.branchPtr(Assembler::AboveOrEqual, Registers::ReturnReg,
                                          FrameAddress(offsetof(VMFrame, stackLimit)));
 
         /* If the stack check fails... */
         {
             stubcc.linkExitDirect(stackCheck, stubcc.masm.label());
             OOL_STUBCALL(stubs::HitStackQuota);
             stubcc.crossJump(stubcc.masm.jump(), masm.label());
@@ -3592,16 +3591,23 @@ mjit::Compiler::inlineScriptedFunction(u
     types::ObjectKind kind = types->getKnownObjectKind(cx, outerScript);
     if (kind != types::OBJECT_INLINEABLE_FUNCTION)
         return Compile_InlineAbort;
 
     if (types->objectCount >= INLINE_SITE_LIMIT)
         return Compile_InlineAbort;
 
     /*
+     * Compute the maximum height we can grow the stack for inlined frames.
+     * We always reserve space for an extra stack frame pushed when making
+     * a call from the deepest inlined frame.
+     */
+    uint32 stackLimit = outerScript->nslots + StackSpace::STACK_EXTRA - VALUES_PER_STACK_FRAME;
+
+    /*
      * Scan each of the possible callees for other conditions precluding
      * inlining. We only inline at a call site if all callees are inlineable.
      */
     for (unsigned i = 0; i < types->objectCount; i++) {
         types::TypeObject *object;
         if (types->objectCount == 1)
             object = (types::TypeObject *) types->objectSet;
         else
@@ -3632,16 +3638,20 @@ mjit::Compiler::inlineScriptedFunction(u
         /* We can't cope with inlining recursive functions yet. */
         ActiveFrame *checka = a;
         while (checka) {
             if (checka->script == script)
                 return Compile_InlineAbort;
             checka = checka->parent;
         }
 
+        /* Watch for excessively deep nesting of inlined frames. */
+        if (frame.totalDepth() + VALUES_PER_STACK_FRAME + fun->script()->nslots >= stackLimit)
+            return Compile_InlineAbort;
+
         analyze::Script analysis;
         analysis.analyze(cx, script);
 
         if (analysis.OOM())
             return Compile_Error;
         if (analysis.failed())
             return Compile_Abort;
 
--- a/js/src/methodjit/InvokeHelpers.cpp
+++ b/js/src/methodjit/InvokeHelpers.cpp
@@ -221,18 +221,18 @@ RemovePartialFrame(JSContext *cx, JSStac
 
 /*
  * HitStackQuota is called after the early prologue pushing the new frame would
  * overflow f.stackLimit.
  */
 void JS_FASTCALL
 stubs::HitStackQuota(VMFrame &f)
 {
-    /* Include space to push another frame. */
-    uintN nvals = f.fp()->script()->nslots + VALUES_PER_STACK_FRAME;
+    /* Include space for any inline frames. */
+    uintN nvals = f.fp()->script()->nslots + StackSpace::STACK_EXTRA;
     JS_ASSERT(f.regs.sp == f.fp()->base());
     if (f.cx->stack().bumpCommitAndLimit(f.entryfp, f.regs.sp, nvals, &f.stackLimit))
         return;
 
     /* Remove the current partially-constructed frame before throwing. */
     RemovePartialFrame(f.cx, f.fp());
     js_ReportOverRecursed(f.cx);
     THROW();