Bug 865471 - Fix various sps profiler issues. r=jandem
authorKannan Vijayan <kvijayan@mozilla.com>
Fri, 03 May 2013 11:26:38 -0400
changeset 130750 6e0956c0dfe4c72ee7bad903cb3652d34605bc10
parent 130749 02fb8ac3f108be3bcc48a3783ccaa0a43cf79de2
child 130751 67a7fc2a14467b3ac64c3dc1ac27a09bd01a4367
push id1579
push userphilringnalda@gmail.com
push dateSat, 04 May 2013 04:38:04 +0000
treeherderfx-team@a56432a42a41 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs865471
milestone23.0a1
Bug 865471 - Fix various sps profiler issues. r=jandem
js/src/ion/BaselineCompiler.cpp
js/src/ion/IonFrames.cpp
js/src/ion/IonMacroAssembler.h
js/src/ion/VMFunctions.cpp
js/src/ion/arm/MacroAssembler-arm.h
js/src/ion/x64/MacroAssembler-x64.h
js/src/ion/x86/MacroAssembler-x86.h
js/src/jsprobes.h
js/src/vm/SPSProfiler.h
js/src/vm/Stack-inl.h
js/src/vm/Stack.h
--- a/js/src/ion/BaselineCompiler.cpp
+++ b/js/src/ion/BaselineCompiler.cpp
@@ -514,17 +514,17 @@ BaselineCompiler::emitSPSPush()
 
 void
 BaselineCompiler::emitSPSPop()
 {
     // If profiler entry was pushed on this frame, pop it.
     Label noPop;
     masm.branchTest32(Assembler::Zero, frame.addressOfFlags(),
                       Imm32(BaselineFrame::HAS_PUSHED_SPS_FRAME), &noPop);
-    masm.spsPopFrame(&cx->runtime->spsProfiler, R1.scratchReg());
+    masm.spsPopFrameSafe(&cx->runtime->spsProfiler, R1.scratchReg());
     masm.bind(&noPop);
 }
 
 MethodStatus
 BaselineCompiler::emitBody()
 {
     JS_ASSERT(pc == script->code);
 
--- a/js/src/ion/IonFrames.cpp
+++ b/js/src/ion/IonFrames.cpp
@@ -490,18 +490,22 @@ HandleException(ResumeFromException *rfe
             bool calledDebugEpilogue = false;
 
             HandleException(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(), NULL);
-
+            Probes::exitScript(cx, script, script->function(), iter.baselineFrame());
+            // 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.
                 BaselineFrame *frame = iter.baselineFrame();
                 if (ion::DebugEpilogue(cx, frame, false)) {
                     JS_ASSERT(frame->hasReturnValue());
                     rfe->kind = ResumeFromException::RESUME_FORCED_RETURN;
                     rfe->framePointer = iter.fp() - BaselineFrame::FramePointerOffset;
--- a/js/src/ion/IonMacroAssembler.h
+++ b/js/src/ion/IonMacroAssembler.h
@@ -738,32 +738,64 @@ class MacroAssembler : public MacroAssem
         branch32(Assembler::GreaterThanOrEqual, temp, Imm32(p->maxSize()), full);
 
         // 4 * sizeof(void*) * idx = idx << (2 + log(sizeof(void*)))
         JS_STATIC_ASSERT(sizeof(ProfileEntry) == 4 * sizeof(void*));
         lshiftPtr(Imm32(2 + (sizeof(void*) == 4 ? 2 : 3)), temp);
         addPtr(ImmWord(p->stack()), temp);
     }
 
+    // The safe version of the above method refrains from assuming that the fields
+    // of the SPSProfiler class are going to stay the same across different runs of
+    // the jitcode.  Ion can use the more efficient unsafe version because ion jitcode
+    // will not survive changes to to the profiler settings.  Baseline jitcode, however,
+    // can span these changes, so any hardcoded field values will be incorrect afterwards.
+    // All the sps-related methods used by baseline call |spsProfileEntryAddressSafe|.
+    void spsProfileEntryAddressSafe(SPSProfiler *p, int offset, Register temp,
+                                    Label *full)
+    {
+        movePtr(ImmWord(p->addressOfSizePointer()), temp);
+
+        // Load size pointer
+        loadPtr(Address(temp, 0), temp);
+
+        // Load size
+        load32(Address(temp, 0), temp);
+        if (offset != 0)
+            add32(Imm32(offset), temp);
+
+        // Test against max size.
+        branch32(Assembler::LessThanOrEqual, AbsoluteAddress(p->addressOfMaxSize()), temp, full);
+
+        // 4 * sizeof(void*) * idx = idx << (2 + log(sizeof(void*)))
+        JS_STATIC_ASSERT(sizeof(ProfileEntry) == 4 * sizeof(void*));
+        lshiftPtr(Imm32(2 + (sizeof(void*) == 4 ? 2 : 3)), temp);
+        push(temp);
+        movePtr(ImmWord(p->addressOfStack()), temp);
+        loadPtr(Address(temp, 0), temp);
+        addPtr(Address(StackPointer, 0), temp);
+        addPtr(Imm32(sizeof(size_t)), StackPointer);
+    }
+
   public:
 
     // These functions are needed by the IonInstrumentation interface defined in
     // vm/SPSProfiler.h.  They will modify the pseudostack provided to SPS to
     // perform the actual instrumentation.
 
     void spsUpdatePCIdx(SPSProfiler *p, int32_t idx, Register temp) {
         Label stackFull;
         spsProfileEntryAddress(p, -1, temp, &stackFull);
         store32(Imm32(idx), Address(temp, ProfileEntry::offsetOfPCIdx()));
         bind(&stackFull);
     }
 
     void spsUpdatePCIdx(SPSProfiler *p, Register idx, Register temp) {
         Label stackFull;
-        spsProfileEntryAddress(p, -1, temp, &stackFull);
+        spsProfileEntryAddressSafe(p, -1, temp, &stackFull);
         store32(idx, Address(temp, ProfileEntry::offsetOfPCIdx()));
         bind(&stackFull);
     }
 
     void spsPushFrame(SPSProfiler *p, const char *str, JSScript *s, Register temp) {
         Label stackFull;
         spsProfileEntryAddress(p, 0, temp, &stackFull);
 
@@ -777,42 +809,50 @@ class MacroAssembler : public MacroAssem
         movePtr(ImmWord(p->sizePointer()), temp);
         add32(Imm32(1), Address(temp, 0));
     }
 
     void spsPushFrame(SPSProfiler *p, const Address &str, const Address &script,
                       Register temp, Register temp2)
     {
         Label stackFull;
-        spsProfileEntryAddress(p, 0, temp, &stackFull);
+        spsProfileEntryAddressSafe(p, 0, temp, &stackFull);
 
         loadPtr(str, temp2);
         storePtr(temp2, Address(temp, ProfileEntry::offsetOfString()));
 
         loadPtr(script, temp2);
         storePtr(temp2, Address(temp, ProfileEntry::offsetOfScript()));
 
         storePtr(ImmWord((void*) 0), Address(temp, ProfileEntry::offsetOfStackAddress()));
 
         // Store 0 for PCIdx because that's what interpreter does.
         // (See Probes::enterScript, which calls spsProfiler.enter, which pushes an entry
         //  with 0 pcIdx).
         store32(Imm32(0), Address(temp, ProfileEntry::offsetOfPCIdx()));
 
         /* Always increment the stack size, whether or not we actually pushed. */
         bind(&stackFull);
-        movePtr(ImmWord(p->sizePointer()), temp);
+        movePtr(ImmWord(p->addressOfSizePointer()), temp);
+        loadPtr(Address(temp, 0), temp);
         add32(Imm32(1), Address(temp, 0));
     }
 
     void spsPopFrame(SPSProfiler *p, Register temp) {
         movePtr(ImmWord(p->sizePointer()), temp);
         add32(Imm32(-1), Address(temp, 0));
     }
 
+    // spsPropFrameSafe does not assume |profiler->sizePointer()| will stay constant.
+    void spsPopFrameSafe(SPSProfiler *p, Register temp) {
+        movePtr(ImmWord(p->addressOfSizePointer()), temp);
+        loadPtr(Address(temp, 0), temp);
+        add32(Imm32(-1), Address(temp, 0));
+    }
+
     void loadBaselineOrIonRaw(Register script, Register dest, ExecutionMode mode, Label *failure);
 
     void loadBaselineFramePtr(Register framePtr, Register dest);
 
     void pushBaselineFramePtr(Register framePtr, Register scratch) {
         loadBaselineFramePtr(framePtr, scratch);
         push(scratch);
     }
--- a/js/src/ion/VMFunctions.cpp
+++ b/js/src/ion/VMFunctions.cpp
@@ -637,19 +637,29 @@ DebugEpilogue(JSContext *cx, BaselineFra
     if (frame->isNonEvalFunctionFrame()) {
         JS_ASSERT_IF(ok, frame->hasReturnValue());
         DebugScopes::onPopCall(frame, cx);
     } else if (frame->isStrictEvalFrame()) {
         JS_ASSERT_IF(frame->hasCallObj(), frame->scopeChain()->asCall().isForEval());
         DebugScopes::onPopStrictEvalScope(frame);
     }
 
+    // If the frame has a pushed SPS frame, make sure to pop it.
+    if (frame->hasPushedSPSFrame()) {
+        cx->runtime->spsProfiler.exit(cx, frame->script(), frame->maybeFun());
+        // Unset the pushedSPSFrame flag because DebugEpilogue may get called before
+        // Probes::exitScript in baseline during exception handling, and we don't
+        // want to double-pop SPS frames.
+        frame->unsetPushedSPSFrame();
+    }
+
     if (!ok) {
         // Pop this frame by updating ionTop, so that the exception handling
         // code will start at the previous frame.
+
         IonJSFrameLayout *prefix = frame->framePrefix();
         EnsureExitFrame(prefix);
         cx->mainThread().ionTop = (uint8_t *)prefix;
     }
 
     return ok;
 }
 
--- a/js/src/ion/arm/MacroAssembler-arm.h
+++ b/js/src/ion/arm/MacroAssembler-arm.h
@@ -891,16 +891,21 @@ class MacroAssemblerARMCompat : public M
         ma_cmp(secondScratchReg_, ptr);
         ma_b(label, cond);
     }
     void branch32(Condition cond, const AbsoluteAddress &lhs, Imm32 rhs, Label *label) {
         loadPtr(lhs, secondScratchReg_); // ma_cmp will use the scratch register.
         ma_cmp(secondScratchReg_, rhs);
         ma_b(label, cond);
     }
+    void branch32(Condition cond, const AbsoluteAddress &lhs, const Register &rhs, Label *label) {
+        loadPtr(lhs, secondScratchReg_); // ma_cmp will use the scratch register.
+        ma_cmp(secondScratchReg_, rhs);
+        ma_b(label, cond);
+    }
 
     void loadUnboxedValue(Address address, MIRType type, AnyRegister dest) {
         if (dest.isFloat())
             loadInt32OrDouble(Operand(address), dest.fpu());
         else
             ma_ldr(address, dest.gpr());
     }
 
--- a/js/src/ion/x64/MacroAssembler-x64.h
+++ b/js/src/ion/x64/MacroAssembler-x64.h
@@ -427,16 +427,20 @@ class MacroAssemblerX64 : public MacroAs
     void subPtr(const Address &addr, const Register &dest) {
         subq(Operand(addr), dest);
     }
 
     void branch32(Condition cond, const AbsoluteAddress &lhs, Imm32 rhs, Label *label) {
         mov(ImmWord(lhs.addr), ScratchReg);
         branch32(cond, Address(ScratchReg, 0), rhs, label);
     }
+    void branch32(Condition cond, const AbsoluteAddress &lhs, Register rhs, Label *label) {
+        mov(ImmWord(lhs.addr), ScratchReg);
+        branch32(cond, Address(ScratchReg, 0), rhs, label);
+    }
 
     // Specialization for AbsoluteAddress.
     void branchPtr(Condition cond, const AbsoluteAddress &addr, const Register &ptr, Label *label) {
         JS_ASSERT(ptr != ScratchReg);
         mov(ImmWord(addr.addr), ScratchReg);
         branchPtr(cond, Operand(ScratchReg, 0x0), ptr, label);
     }
 
--- a/js/src/ion/x86/MacroAssembler-x86.h
+++ b/js/src/ion/x86/MacroAssembler-x86.h
@@ -451,16 +451,20 @@ class MacroAssemblerX86 : public MacroAs
     void subPtr(const Address &addr, const Register &dest) {
         subl(Operand(addr), dest);
     }
 
     void branch32(Condition cond, const AbsoluteAddress &lhs, Imm32 rhs, Label *label) {
         cmpl(Operand(lhs), rhs);
         j(cond, label);
     }
+    void branch32(Condition cond, const AbsoluteAddress &lhs, Register rhs, Label *label) {
+        cmpl(Operand(lhs), rhs);
+        j(cond, label);
+    }
 
     template <typename T, typename S>
     void branchPtr(Condition cond, T lhs, S ptr, Label *label) {
         cmpl(Operand(lhs), ptr);
         j(cond, label);
     }
 
     template <typename T>
--- a/js/src/jsprobes.h
+++ b/js/src/jsprobes.h
@@ -73,16 +73,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 *);
 
 /* Executing a script */
 bool startExecution(JSScript *script);
 
 /* Script has completed execution */
 bool stopExecution(JSScript *script);
 
@@ -200,17 +201,17 @@ Probes::enterScript(JSContext *cx, JSScr
         fp->setPushedSPSFrame();
     }
 
     return ok;
 }
 
 inline bool
 Probes::exitScript(JSContext *cx, JSScript *script, JSFunction *maybeFun,
-                   StackFrame *fp)
+                   AbstractFramePtr fp)
 {
     bool ok = true;
 
 #ifdef INCLUDE_MOZILLA_DTRACE
     if (JAVASCRIPT_FUNCTION_RETURN_ENABLED())
         DTraceExitJSFun(cx, maybeFun, script);
 #endif
 #ifdef MOZ_TRACE_JSCALLS
@@ -218,24 +219,28 @@ Probes::exitScript(JSContext *cx, JSScri
 #endif
 
     JSRuntime *rt = cx->runtime;
     /*
      * Coming from IonMonkey, the fp might not be known (fp == NULL), but
      * IonMonkey will only call exitScript() when absolutely necessary, so it is
      * guaranteed that fp->hasPushedSPSFrame() would have been true
      */
-    if ((fp == NULL && rt->spsProfiler.enabled()) ||
-        (fp != NULL && fp->hasPushedSPSFrame()))
-    {
+    if ((!fp && rt->spsProfiler.enabled()) || (fp && fp.hasPushedSPSFrame()))
         rt->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());
+}
+
 #ifdef INCLUDE_MOZILLA_DTRACE
 static const char *ObjectClassname(JSObject *obj) {
     if (!obj)
         return "(null object)";
     Class *clasp = obj->getClass();
     if (!clasp)
         return "(null)";
     const char *class_name = clasp->name;
--- a/js/src/vm/SPSProfiler.h
+++ b/js/src/vm/SPSProfiler.h
@@ -139,16 +139,28 @@ class SPSProfiler
                                    JSFunction *function);
     void push(const char *string, void *sp, JSScript *script, jsbytecode *pc);
     void pop();
 
   public:
     SPSProfiler(JSRuntime *rt);
     ~SPSProfiler();
 
+    uint32_t **addressOfSizePointer() {
+        return &size_;
+    }
+
+    uint32_t *addressOfMaxSize() {
+        return &max_;
+    }
+
+    ProfileEntry **addressOfStack() {
+        return &stack_;
+    }
+
     uint32_t *sizePointer() { return size_; }
     uint32_t maxSize() { return max_; }
     ProfileEntry *stack() { return stack_; }
 
     /* management of whether instrumentation is on or off */
     bool enabled() { JS_ASSERT_IF(enabled_, installed()); return enabled_; }
     bool installed() { return stack_ != NULL && size_ != NULL; }
     void enable(bool enabled);
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -622,16 +622,29 @@ AbstractFramePtr::setReturnValue(const V
     }
 #ifdef JS_ION
     asBaselineFrame()->setReturnValue(rval);
 #else
     JS_NOT_REACHED("Invalid frame");
 #endif
 }
 
+inline bool
+AbstractFramePtr::hasPushedSPSFrame() const
+{
+    if (isStackFrame())
+        return asStackFrame()->hasPushedSPSFrame();
+#ifdef JS_ION
+    return asBaselineFrame()->hasPushedSPSFrame();
+#else
+    JS_NOT_REACHED("Invalid frame");
+    return false;
+#endif
+}
+
 inline JSObject *
 AbstractFramePtr::scopeChain() const
 {
     if (isStackFrame())
         return asStackFrame()->scopeChain();
 #ifdef JS_ION
     return asBaselineFrame()->scopeChain();
 #else
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -326,16 +326,18 @@ class AbstractFramePtr
 
     JSObject *evalPrevScopeChain(JSRuntime *rt) const;
 
     inline void *maybeHookData() const;
     inline void setHookData(void *data) const;
     inline Value returnValue() const;
     inline void setReturnValue(const Value &rval) const;
 
+    inline bool hasPushedSPSFrame() const;
+
     inline void popBlock(JSContext *cx) const;
     inline void popWith(JSContext *cx) const;
 };
 
 class NullFramePtr : public AbstractFramePtr
 {
   public:
     NullFramePtr()