Bug 865471 - Fix various sps profiler issues. r=jandem

js/src/ion/BaselineCompiler.cpp

@@ -519,7 +519,7 @@ BaselineCompiler::emitSPSPop()
     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);
 }
 

js/src/ion/IonFrames.cpp

@@ -495,8 +495,12 @@ HandleException(ResumeFromException *rfe)
 
             // 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.

js/src/ion/IonMacroAssembler.h

@@ -743,6 +743,38 @@ class MacroAssembler : public MacroAssemblerSpecific
         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
@@ -758,7 +790,7 @@ class MacroAssembler : public MacroAssemblerSpecific
 
     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);
     }
@@ -782,7 +814,7 @@ class MacroAssembler : public MacroAssemblerSpecific
                       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()));
@@ -799,7 +831,8 @@ class MacroAssembler : public MacroAssemblerSpecific
 
         /* 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));
     }
 
@@ -808,6 +841,13 @@ class MacroAssembler : public MacroAssemblerSpecific
         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);

js/src/ion/VMFunctions.cpp

@@ -642,9 +642,19 @@ DebugEpilogue(JSContext *cx, BaselineFrame *frame, JSBool ok)
         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;

js/src/ion/arm/MacroAssembler-arm.h

@@ -896,6 +896,11 @@ class MacroAssemblerARMCompat : public MacroAssemblerARM
         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())

js/src/ion/x64/MacroAssembler-x64.h

@@ -432,6 +432,10 @@ class MacroAssemblerX64 : public MacroAssemblerX86Shared
         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/src/ion/x86/MacroAssembler-x86.h

@@ -456,6 +456,10 @@ class MacroAssemblerX86 : public MacroAssemblerX86Shared
         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) {

js/src/jsprobes.h

@@ -78,6 +78,7 @@ bool wantNativeAddressInfo(JSContext *);
 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 */
@@ -205,7 +206,7 @@ Probes::enterScript(JSContext *cx, JSScript *script, JSFunction *maybeFun,
 
 inline bool
 Probes::exitScript(JSContext *cx, JSScript *script, JSFunction *maybeFun,
-                   StackFrame *fp)
+                   AbstractFramePtr fp)
 {
     bool ok = true;
 
@@ -223,14 +224,18 @@ Probes::exitScript(JSContext *cx, JSScript *script, JSFunction *maybeFun,
      * 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)

js/src/vm/SPSProfiler.h

@@ -144,6 +144,18 @@ class SPSProfiler
     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_; }

js/src/vm/Stack-inl.h

@@ -627,6 +627,19 @@ AbstractFramePtr::setReturnValue(const Value &rval) const
 #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
 {

js/src/vm/Stack.h

@@ -331,6 +331,8 @@ class AbstractFramePtr
     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;
 };