Bug 877936 - IonMonkey: Disable compilation and inlining when too many arguments are specified, r=dvander
authorHannes Verschore <hv1989@gmail.com>
Fri, 25 Oct 2013 12:28:50 +0200
changeset 166903 86650bc9c33d9d302ad3c06839f540f1087226b3
parent 166902 a0a6698193002f77042dfa1ad4f3a2a3cde22109
child 166904 ba27b9e35d13d8d80f28a2b30eca2d1186b5b617
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs877936
milestone27.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
Bug 877936 - IonMonkey: Disable compilation and inlining when too many arguments are specified, r=dvander
js/src/jit-test/tests/ion/bug877936-2.js
js/src/jit-test/tests/ion/bug877936.js
js/src/jit/CompileInfo.h
js/src/jit/Ion.cpp
js/src/jit/Ion.h
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
js/src/jit/Snapshots.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug877936-2.js
@@ -0,0 +1,5 @@
+rex = RegExp("()()()()()()()()()()(z)?(y)");
+a = ["sub"];
+a[230] = '' + "a"
+f = Function.apply(null, a);
+"xyz".replace(rex, f);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug877936.js
@@ -0,0 +1,44 @@
+try{} catch(e){}
+
+function test(a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a) {
+  return 0;
+}
+
+
+test();
+test();
+
+/////////////////////
+
+function test2() {
+  return 0;
+}
+
+var a = 1;
+test2(a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a);
+test2(a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a);
+
+/////////////////////
+
+function test4() {
+  test3()
+}
+
+function test3(a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a) {
+  return 0;
+}
+
+test4();
+
+/////////////////////
+
+function test6() {
+  test5.apply({}, [])
+}
+
+function test5() {
+  return 0;
+}
+
+test6();
+test6();
--- a/js/src/jit/CompileInfo.h
+++ b/js/src/jit/CompileInfo.h
@@ -19,26 +19,29 @@ StartArgSlot(JSScript *script, JSFunctio
 {
     // Reserved slots:
     // Slot 0: Scope chain.
     // Slot 1: Return value.
 
     // When needed:
     // Slot 2: Argumentsobject.
 
+    // Note: when updating this, please also update the assert in SnapshotWriter::startFrame
     return 2 + (script->argumentsHasVarBinding() ? 1 : 0);
 }
 
 inline unsigned
 CountArgSlots(JSScript *script, JSFunction *fun)
 {
     // Slot x + 0: This value.
     // Slot x + 1: Argument 1.
     // ...
     // Slot x + n: Argument n.
+
+    // Note: when updating this, please also update the assert in SnapshotWriter::startFrame
     return StartArgSlot(script, fun) + (fun ? fun->nargs + 1 : 0);
 }
 
 // Contains information about the compilation source for IR being generated.
 class CompileInfo
 {
   public:
     CompileInfo(JSScript *script, JSFunction *fun, jsbytecode *osrPc, bool constructing,
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -1652,22 +1652,16 @@ IonCompile(JSContext *cx, JSScript *scri
     bool success = codegen->link(cx, builder->constraints());
 
     IonSpewEndFunction();
 
     return success ? AbortReason_NoAbort : AbortReason_Disable;
 }
 
 static bool
-TooManyArguments(unsigned nargs)
-{
-    return (nargs >= SNAPSHOT_MAX_NARGS || nargs > js_IonOptions.maxStackArgs);
-}
-
-static bool
 CheckFrame(BaselineFrame *frame)
 {
     JS_ASSERT(!frame->isGeneratorFrame());
     JS_ASSERT(!frame->isDebuggerFrame());
 
     // This check is to not overrun the stack.
     if (frame->isFunctionFrame() && TooManyArguments(frame->numActualArgs())) {
         IonSpew(IonSpew_Abort, "too many actual args");
@@ -1897,16 +1891,22 @@ jit::CanEnter(JSContext *cx, RunState &s
         InvokeState &invoke = *state.asInvoke();
 
         if (TooManyArguments(invoke.args().length())) {
             IonSpew(IonSpew_Abort, "too many actual args");
             ForbidCompilation(cx, script);
             return Method_CantCompile;
         }
 
+        if (TooManyArguments(invoke.args().callee().as<JSFunction>().nargs)) {
+            IonSpew(IonSpew_Abort, "too many args");
+            ForbidCompilation(cx, script);
+            return Method_CantCompile;
+        }
+
         if (invoke.constructing() && invoke.args().thisv().isPrimitive()) {
             RootedScript scriptRoot(cx, script);
             RootedObject callee(cx, &invoke.args().callee());
             RootedObject obj(cx, CreateThisForFunction(cx, callee,
                                                        invoke.useNewType()
                                                        ? SingletonObject
                                                        : GenericObject));
             if (!obj || !jit::IsIonEnabled(cx)) // Note: OOM under CreateThis can disable TI.
--- a/js/src/jit/Ion.h
+++ b/js/src/jit/Ion.h
@@ -379,16 +379,22 @@ IsIonEnabled(JSContext *cx)
 inline bool
 IsIonInlinablePC(jsbytecode *pc) {
     // CALL, FUNCALL, FUNAPPLY, EVAL, NEW (Normal Callsites)
     // GETPROP, CALLPROP, and LENGTH. (Inlined Getters)
     // SETPROP, SETNAME, SETGNAME (Inlined Setters)
     return IsCallPC(pc) || IsGetPropPC(pc) || IsSetPropPC(pc);
 }
 
+inline bool
+TooManyArguments(unsigned nargs)
+{
+    return (nargs >= SNAPSHOT_MAX_NARGS || nargs > js_IonOptions.maxStackArgs);
+}
+
 void ForbidCompilation(JSContext *cx, JSScript *script);
 void ForbidCompilation(JSContext *cx, JSScript *script, ExecutionMode mode);
 uint32_t UsesBeforeIonRecompile(JSScript *script, jsbytecode *pc);
 
 void PurgeCaches(JSScript *script, JS::Zone *zone);
 size_t SizeOfIonData(JSScript *script, mozilla::MallocSizeOf mallocSizeOf);
 void DestroyIonScripts(FreeOp *fop, JSScript *script);
 void TraceIonScripts(JSTracer* trc, JSScript *script);
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -261,17 +261,17 @@ IonBuilder::canEnterInlinedFunction(JSFu
     types::TypeObjectKey *targetType = types::TypeObjectKey::get(target);
     if (targetType->unknownProperties())
         return false;
 
     return true;
 }
 
 bool
-IonBuilder::canInlineTarget(JSFunction *target, bool constructing)
+IonBuilder::canInlineTarget(JSFunction *target, CallInfo &callInfo)
 {
     if (!target->isInterpreted()) {
         IonSpew(IonSpew_Inlining, "Cannot inline due to non-interpreted");
         return false;
     }
 
     if (target->getParent() != &script()->global()) {
         IonSpew(IonSpew_Inlining, "Cannot inline due to scope mismatch");
@@ -292,17 +292,17 @@ IonBuilder::canInlineTarget(JSFunction *
         }
     }
 
     if (!target->hasScript()) {
         IonSpew(IonSpew_Inlining, "Cannot inline due to lack of Non-Lazy script");
         return false;
     }
 
-    if (constructing && !target->isInterpretedConstructor()) {
+    if (callInfo.constructing() && !target->isInterpretedConstructor()) {
         IonSpew(IonSpew_Inlining, "Cannot inline because callee is not a constructor");
         return false;
     }
 
     JSScript *inlineScript = target->nonLazyScript();
     ExecutionMode executionMode = info().executionMode();
     if (!CanIonCompile(inlineScript, executionMode)) {
         IonSpew(IonSpew_Inlining, "%s:%d Cannot inline due to disable Ion compilation",
@@ -312,16 +312,28 @@ IonBuilder::canInlineTarget(JSFunction *
 
     // Don't inline functions which don't have baseline scripts.
     if (!inlineScript->hasBaselineScript()) {
         IonSpew(IonSpew_Inlining, "%s:%d Cannot inline target with no baseline jitcode",
                                   inlineScript->filename(), inlineScript->lineno);
         return false;
     }
 
+    if (TooManyArguments(target->nargs)) {
+        IonSpew(IonSpew_Inlining, "%s:%d Cannot inline too many args",
+                                  inlineScript->filename(), inlineScript->lineno);
+        return false;
+    }
+
+    if (TooManyArguments(callInfo.argc())) {
+        IonSpew(IonSpew_Inlining, "%s:%d Cannot inline too many args",
+                                  inlineScript->filename(), inlineScript->lineno);
+        return false;
+    }
+
     // Allow inlining of recursive calls, but only one level deep.
     IonBuilder *builder = callerBuilder_;
     while (builder) {
         if (builder->script() == inlineScript) {
             IonSpew(IonSpew_Inlining, "%s:%d Not inlining recursive call",
                                        inlineScript->filename(), inlineScript->lineno);
             return false;
         }
@@ -3942,17 +3954,17 @@ IonBuilder::makeInliningDecision(JSFunct
     if (target == nullptr)
         return false;
 
     // Native functions provide their own detection in inlineNativeCall().
     if (target->isNative())
         return true;
 
     // Determine whether inlining is possible at callee site
-    if (!canInlineTarget(target, callInfo.constructing()))
+    if (!canInlineTarget(target, callInfo))
         return false;
 
     // Heuristics!
     JSScript *targetScript = target->nonLazyScript();
 
     // Skip heuristics if we have an explicit hint to inline.
     if (!targetScript->shouldInline) {
         // Cap the inlining depth.
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -226,17 +226,17 @@ class IonBuilder : public MIRGenerator
 
     static bool inliningEnabled() {
         return js_IonOptions.inlining;
     }
 
     JSFunction *getSingleCallTarget(types::TemporaryTypeSet *calleeTypes);
     bool getPolyCallTargets(types::TemporaryTypeSet *calleeTypes, bool constructing,
                             ObjectVector &targets, uint32_t maxTargets, bool *gotLambda);
-    bool canInlineTarget(JSFunction *target, bool constructing);
+    bool canInlineTarget(JSFunction *target, CallInfo &callInfo);
 
     void popCfgStack();
     DeferredEdge *filterDeadDeferredEdges(DeferredEdge *edge);
     bool processDeferredContinues(CFGState &state);
     ControlStatus processControlEnd();
     ControlStatus processCfgStack();
     ControlStatus processCfgEntry(CFGState &state);
     ControlStatus processIfEnd(CFGState &state);
--- a/js/src/jit/Snapshots.cpp
+++ b/js/src/jit/Snapshots.cpp
@@ -296,17 +296,20 @@ SnapshotWriter::startSnapshot(uint32_t f
 
     writer_.writeUnsigned(bits);
     return lastStart_;
 }
 
 void
 SnapshotWriter::startFrame(JSFunction *fun, JSScript *script, jsbytecode *pc, uint32_t exprStack)
 {
-    JS_ASSERT(CountArgSlots(script, fun) < SNAPSHOT_MAX_NARGS);
+    // Test if we honor the maximum of arguments at all times.
+    // This is a sanity check and not an algorithm limit. So check might be a bit too loose.
+    // +4 to account for scope chain, return value, this value and maybe arguments_object.
+    JS_ASSERT(CountArgSlots(script, fun) < SNAPSHOT_MAX_NARGS + 4);
 
     uint32_t implicit = StartArgSlot(script, fun);
     uint32_t formalArgs = CountArgSlots(script, fun);
 
     nslots_ = formalArgs + script->nfixed + exprStack;
     slotsWritten_ = 0;
 
     IonSpew(IonSpew_Snapshots, "Starting frame; implicit %u, formals %u, fixed %u, exprs %u",