Bug 925308 - Dont pop profiler frames for stack frames pushed by invalidated IonScripts which did not have profiler instrumentation. r=jandem, a=bajaj
authorKannan Vijayan <kvijayan@mozilla.com>
Tue, 12 Nov 2013 11:28:03 -0500
changeset 167640 37df29dd2f8f1e6ab099f3fffb9b3d59fb4b6adb
parent 167639 e072083c4a0d7be1a31b9ce38c0d61bd3a004b23
child 167641 e1144ef584de750602f07e77a2f622de9159fa15
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)
reviewersjandem, bajaj
bugs925308
milestone27.0a2
Bug 925308 - Dont pop profiler frames for stack frames pushed by invalidated IonScripts which did not have profiler instrumentation. r=jandem, a=bajaj
js/src/jit-test/tests/ion/bug925308.js
js/src/jit/IonFrames.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Probes-inl.h
js/src/vm/Probes.h
js/src/vm/Stack.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug925308.js
@@ -0,0 +1,18 @@
+// |jit-test| error: ReferenceError
+
+var lfcode = new Array();
+lfcode.push("3");
+lfcode.push("enableSPSProfilingAssertions(false);foo();");
+while (true) {
+  var file = lfcode.shift(); if (file == undefined) { break; }
+  loadFile(file)
+}
+function loadFile(lfVarx) {
+    if (lfVarx.substr(-3) != ".js" && lfVarx.length != 1) {
+        switch (lfRunTypeId) {
+            default: function newFunc(x) { new Function(x)(); }; newFunc(lfVarx); break;
+        }
+    } else if (!isNaN(lfVarx)) {
+        lfRunTypeId = parseInt(lfVarx);
+    }
+}
--- a/js/src/jit/IonFrames.cpp
+++ b/js/src/jit/IonFrames.cpp
@@ -534,53 +534,65 @@ HandleException(ResumeFromException *rfe
 
     IonFrameIterator iter(cx);
     while (!iter.isEntry()) {
         bool overrecursed = false;
         if (iter.isOptimizedJS()) {
             // Search each inlined frame for live iterator objects, and close
             // them.
             InlineFrameIterator frames(cx, &iter);
+
+            // Invalidation state will be the same for all inlined scripts in the frame.
+            IonScript *ionScript = nullptr;
+            bool invalidated = iter.checkInvalidation(&ionScript);
+
             for (;;) {
                 HandleExceptionIon(cx, frames, rfe, &overrecursed);
 
                 if (rfe->kind == ResumeFromException::RESUME_BAILOUT) {
-                    IonScript *ionScript = nullptr;
-                    if (iter.checkInvalidation(&ionScript))
+                    if (invalidated)
                         ionScript->decref(cx->runtime()->defaultFreeOp());
                     return;
                 }
 
                 JS_ASSERT(rfe->kind == ResumeFromException::RESUME_ENTRY_FRAME);
 
+                // Figure out whether SPS frame was pushed for this frame or not.
+                // Even if profiler is enabled, the frame being popped might have
+                // been entered prior to SPS being enabled, and thus not have
+                // a pushed SPS frame.
+                bool popSPSFrame = cx->runtime()->spsProfiler.enabled();
+                if (invalidated)
+                    popSPSFrame = ionScript->hasSPSInstrumentation();
+
                 // When profiling, each frame popped needs a notification that
                 // the function has exited, so invoke the probe that a function
                 // is exiting.
                 JSScript *script = frames.script();
-                probes::ExitScript(cx, script, script->function(), nullptr);
+                probes::ExitScript(cx, script, script->function(), popSPSFrame);
                 if (!frames.more())
                     break;
                 ++frames;
             }
 
-            IonScript *ionScript = nullptr;
-            if (iter.checkInvalidation(&ionScript))
+            if (invalidated)
                 ionScript->decref(cx->runtime()->defaultFreeOp());
 
         } else if (iter.isBaselineJS()) {
             // It's invalid to call DebugEpilogue twice for the same frame.
             bool calledDebugEpilogue = false;
 
             HandleExceptionBaseline(cx, iter, rfe, &calledDebugEpilogue);
             if (rfe->kind != ResumeFromException::RESUME_ENTRY_FRAME)
                 return;
 
             // Unwind profiler pseudo-stack
             JSScript *script = iter.script();
-            probes::ExitScript(cx, script, script->function(), iter.baselineFrame());
+            probes::ExitScript(cx, script, script->function(),
+                               iter.baselineFrame()->hasPushedSPSFrame());
             // After this point, any pushed SPS frame would have been popped if it needed
             // to be.  Unset the flag here so that if we call DebugEpilogue below,
             // it doesn't try to pop the SPS frame again.
             iter.baselineFrame()->unsetPushedSPSFrame();
  
             if (cx->compartment()->debugMode() && !calledDebugEpilogue) {
                 // If DebugEpilogue returns |true|, we have to perform a forced
                 // return, e.g. return frame->returnValue() to the caller.
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1620,17 +1620,17 @@ BEGIN_CASE(JSOP_STOP)
 #endif
 
         if (cx->compartment()->debugMode())
             interpReturnOK = ScriptDebugEpilogue(cx, regs.fp(), interpReturnOK);
 
         if (!regs.fp()->isYielding())
             regs.fp()->epilogue(cx);
         else
-            probes::ExitScript(cx, script, script->function(), regs.fp());
+            probes::ExitScript(cx, script, script->function(), regs.fp()->hasPushedSPSFrame());
 
 #if defined(JS_ION)
   jit_return_pop_frame:
 #endif
 
         activation.popInlineFrame(regs.fp());
         SET_SCRIPT(regs.fp()->script());
 
@@ -3398,17 +3398,17 @@ default:
         goto inline_return;
 
   exit:
     if (cx->compartment()->debugMode())
         interpReturnOK = ScriptDebugEpilogue(cx, regs.fp(), interpReturnOK);
     if (!regs.fp()->isYielding())
         regs.fp()->epilogue(cx);
     else
-        probes::ExitScript(cx, script, script->function(), regs.fp());
+        probes::ExitScript(cx, script, script->function(), regs.fp()->hasPushedSPSFrame());
 
     gc::MaybeVerifyBarriers(cx, true);
 
 #if JS_TRACE_LOGGING
         TraceLogging::defaultLogger()->log(TraceLogging::SCRIPT_STOP);
 #endif
 
 #ifdef JS_ION
--- a/js/src/vm/Probes-inl.h
+++ b/js/src/vm/Probes-inl.h
@@ -58,48 +58,35 @@ probes::EnterScript(JSContext *cx, JSScr
         JS_ASSERT_IF(!fp->isGeneratorFrame(), !fp->hasPushedSPSFrame());
         fp->setPushedSPSFrame();
     }
 
     return ok;
 }
 
 inline bool
-probes::ExitScript(JSContext *cx, JSScript *script, JSFunction *maybeFun,
-                   AbstractFramePtr fp)
+probes::ExitScript(JSContext *cx, JSScript *script, JSFunction *maybeFun, bool popSPSFrame)
 {
     bool ok = true;
 
 #ifdef INCLUDE_MOZILLA_DTRACE
     if (JAVASCRIPT_FUNCTION_RETURN_ENABLED())
         DTraceExitJSFun(cx, maybeFun, script);
 #endif
 #ifdef MOZ_TRACE_JSCALLS
     cx->doFunctionCallback(maybeFun, script, 0);
 #endif
 
-    JSRuntime *rt = cx->runtime();
-    /*
-     * Coming from IonMonkey, the fp might not be known (fp == nullptr), but
-     * IonMonkey will only call exitScript() when absolutely necessary, so it is
-     * guaranteed that fp->hasPushedSPSFrame() would have been true
-     */
-    if ((!fp && rt->spsProfiler.enabled()) || (fp && fp.hasPushedSPSFrame()))
-        rt->spsProfiler.exit(cx, script, maybeFun);
+    if (popSPSFrame)
+        cx->runtime()->spsProfiler.exit(cx, script, maybeFun);
+
     return ok;
 }
 
 inline bool
-probes::ExitScript(JSContext *cx, JSScript *script, JSFunction *maybeFun,
-                   StackFrame *fp)
-{
-    return probes::ExitScript(cx, script, maybeFun, fp ? AbstractFramePtr(fp) : AbstractFramePtr());
-}
-
-inline bool
 probes::StartExecution(JSScript *script)
 {
     bool ok = true;
 
 #ifdef INCLUDE_MOZILLA_DTRACE
     if (JAVASCRIPT_EXECUTE_START_ENABLED())
         JAVASCRIPT_EXECUTE_START((script->filename() ? (char *)script->filename() : nullName),
                                  script->lineno);
--- a/js/src/vm/Probes.h
+++ b/js/src/vm/Probes.h
@@ -64,18 +64,17 @@ bool CallTrackingActive(JSContext *);
  * This information will not be collected otherwise.
  */
 bool WantNativeAddressInfo(JSContext *);
 
 /* Entering a JS function */
 bool EnterScript(JSContext *, JSScript *, JSFunction *, StackFrame *);
 
 /* About to leave a JS function */
-bool ExitScript(JSContext *, JSScript *, JSFunction *, AbstractFramePtr);
-bool ExitScript(JSContext *, JSScript *, JSFunction *, StackFrame *);
+bool ExitScript(JSContext *, JSScript *, JSFunction *, bool popSPSFrame);
 
 /* Executing a script */
 bool StartExecution(JSScript *script);
 
 /* Script has completed execution */
 bool StopExecution(JSScript *script);
 
 /*
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -294,17 +294,17 @@ StackFrame::prologue(JSContext *cx)
 
 void
 StackFrame::epilogue(JSContext *cx)
 {
     JS_ASSERT(!isYielding());
     JS_ASSERT(!hasBlockChain());
 
     RootedScript script(cx, this->script());
-    probes::ExitScript(cx, script, script->function(), this);
+    probes::ExitScript(cx, script, script->function(), hasPushedSPSFrame());
 
     if (isEvalFrame()) {
         if (isStrictEvalFrame()) {
             JS_ASSERT_IF(hasCallObj(), scopeChain()->as<CallObject>().isForEval());
             if (cx->compartment()->debugMode())
                 DebugScopes::onPopStrictEvalScope(this);
         } else if (isDirectEvalFrame()) {
             if (isDebuggerFrame())