Bug 914561: Record EnterJIT trampoline frame address on SPS pseudostack. r=nbp

js/src/builtin/TestingFunctions.cpp

@@ -980,6 +980,9 @@ EnableSPSProfilingAssertions(JSContext *cx, unsigned argc, jsval *vp)
     static ProfileEntry stack[1000];
     static uint32_t stack_size = 0;
 
+    // Disable before re-enabling; see the assertion in |SPSProfiler::setProfilingStack|.
+    if (cx->runtime()->spsProfiler.installed())
+        cx->runtime()->spsProfiler.enable(false);
     SetRuntimeProfilingStack(cx->runtime(), stack, &stack_size, 1000);
     cx->runtime()->spsProfiler.enableSlowAssertions(args[0].toBoolean());
     cx->runtime()->spsProfiler.enable(true);

js/src/jit-test/tests/profiler/enterjit-osr-disabling-earlyret.js

@@ -0,0 +1,13 @@
+setJitCompilerOption("baseline.usecount.trigger", 10);
+setJitCompilerOption("ion.usecount.trigger", 20);
+
+enableSPSProfilingAssertions(true);
+(function() {
+   var n = 50;
+   while (n--) {
+       disableSPSProfiling();
+       if (!n)
+	   return;
+       enableSPSProfilingAssertions(true);
+   }
+})();

js/src/jit-test/tests/profiler/enterjit-osr-disabling.js

@@ -0,0 +1,9 @@
+setJitCompilerOption("baseline.usecount.trigger", 10);
+setJitCompilerOption("ion.usecount.trigger", 20);
+
+enableSPSProfilingAssertions(true);
+(function() {
+   disableSPSProfiling();
+   var n = 50;
+   while (n--);
+})();

js/src/jit-test/tests/profiler/enterjit-osr-enabling-earlyret.js

@@ -0,0 +1,12 @@
+setJitCompilerOption("baseline.usecount.trigger", 10);
+setJitCompilerOption("ion.usecount.trigger", 20);
+
+(function() {
+   var n = 50;
+   while (n--) {
+       enableSPSProfilingAssertions(true);
+       if (!n)
+	   return;
+       disableSPSProfiling();
+   }
+})();

js/src/jit-test/tests/profiler/enterjit-osr-enabling.js

@@ -0,0 +1,8 @@
+setJitCompilerOption("baseline.usecount.trigger", 10);
+setJitCompilerOption("ion.usecount.trigger", 20);
+
+(function() {
+   enableSPSProfilingAssertions(true);
+   var n = 50;
+   while (n--);
+})();

js/src/jit-test/tests/profiler/enterjit-osr.js

@@ -0,0 +1,8 @@
+setJitCompilerOption("baseline.usecount.trigger", 10);
+setJitCompilerOption("ion.usecount.trigger", 20);
+
+enableSPSProfilingAssertions(true);
+(function() {
+   var n = 50;
+   while (n--);
+})();

js/src/jit/BaselineFrame.cpp

@@ -178,8 +178,18 @@ BaselineFrame::initForOsr(InterpreterFrame *fp, uint32_t numStackValues)
     if (fp->hasReturnValue())
         setReturnValue(fp->returnValue());
 
-    if (fp->hasPushedSPSFrame())
+    // If the interpreter pushed an SPS frame when it entered the function, the
+    // interpreter will pop it after the OSR trampoline returns.  In order for
+    // the Baseline frame to have its SPS flag set, it must have its own SPS
+    // frame, which the Baseline code will pop on return.  Note that the
+    // profiler may have been enabled or disabled after the function was entered
+    // but before OSR.
+    JSContext *cx = GetJSContextFromJitCode();
+    SPSProfiler *p = &(cx->runtime()->spsProfiler);
+    if (p->enabled()) {
+        p->enter(fp->script(), fp->maybeFun());
         flags_ |= BaselineFrame::HAS_PUSHED_SPS_FRAME;
+    }
 
     frameSize_ = BaselineFrame::FramePointerOffset +
         BaselineFrame::Size() +
@@ -190,7 +200,6 @@ BaselineFrame::initForOsr(InterpreterFrame *fp, uint32_t numStackValues)
     for (uint32_t i = 0; i < numStackValues; i++)
         *valueSlot(i) = fp->slots()[i];
 
-    JSContext *cx = GetJSContextFromJitCode();
     if (cx->compartment()->debugMode()) {
         // In debug mode, update any Debugger.Frame objects for the
         // InterpreterFrame to point to the BaselineFrame.

js/src/jit/IonMacroAssembler.cpp

@@ -1904,3 +1904,54 @@ MacroAssembler::branchEqualTypeIfNeeded(MIRType type, MDefinition *maybeDef, Reg
         }
     }
 }
+
+
+// If a pseudostack frame has this as its label, its stack pointer
+// field points to the registers saved on entry to JIT code.  A native
+// stack unwinder could use that information to continue unwinding
+// past that point.
+const char MacroAssembler::enterJitLabel[] = "EnterJIT";
+
+// Creates an enterJIT pseudostack frame, as described above.  Pushes
+// a word to the stack to indicate whether this was done.  |framePtr| is
+// the pointer to the machine-dependent saved state.
+void
+MacroAssembler::spsMarkJit(SPSProfiler *p, Register framePtr, Register temp)
+{
+    Label spsNotEnabled;
+    uint32_t *enabledAddr = p->addressOfEnabled();
+    load32(AbsoluteAddress(enabledAddr), temp);
+    push(temp); // +4: Did we push an sps frame.
+    branchTest32(Assembler::Equal, temp, temp, &spsNotEnabled);
+
+    Label stackFull;
+    // We always need the "safe" versions, because these are used in trampolines
+    // and won't be regenerated when SPS state changes.
+    spsProfileEntryAddressSafe(p, 0, temp, &stackFull);
+
+    storePtr(ImmPtr(enterJitLabel), Address(temp, ProfileEntry::offsetOfString()));
+    storePtr(framePtr,              Address(temp, ProfileEntry::offsetOfStackAddress()));
+    storePtr(ImmWord(uintptr_t(0)), Address(temp, ProfileEntry::offsetOfScript()));
+    store32(Imm32(ProfileEntry::NullPCIndex), Address(temp, ProfileEntry::offsetOfPCIdx()));
+
+    /* Always increment the stack size, whether or not we actually pushed. */
+    bind(&stackFull);
+    loadPtr(AbsoluteAddress(p->addressOfSizePointer()), temp);
+    add32(Imm32(1), Address(temp, 0));
+
+    bind(&spsNotEnabled);
+}
+
+// Pops the word pushed by spsMarkJit and, if spsMarkJit pushed an SPS
+// frame, pops it.
+void
+MacroAssembler::spsUnmarkJit(SPSProfiler *p, Register temp)
+{
+    Label spsNotEnabled;
+    pop(temp); // -4: Was the profiler enabled.
+    branchTest32(Assembler::Equal, temp, temp, &spsNotEnabled);
+
+    spsPopFrameSafe(p, temp);
+
+    bind(&spsNotEnabled);
+}

js/src/jit/IonMacroAssembler.h

@@ -1021,7 +1021,6 @@ class MacroAssembler : public MacroAssemblerSpecific
     }
 
   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.
@@ -1092,6 +1091,10 @@ class MacroAssembler : public MacroAssemblerSpecific
         add32(Imm32(-1), Address(temp, 0));
     }
 
+    static const char enterJitLabel[];
+    void spsMarkJit(SPSProfiler *p, Register framePtr, Register temp);
+    void spsUnmarkJit(SPSProfiler *p, Register temp);
+
     void loadBaselineOrIonRaw(Register script, Register dest, ExecutionMode mode, Label *failure);
     void loadBaselineOrIonNoArgCheck(Register callee, Register dest, ExecutionMode mode, Label *failure);
 

js/src/jit/arm/Trampoline-arm.cpp

@@ -33,13 +33,13 @@ static const FloatRegisterSet NonVolatileFloatRegs =
                      (1 << FloatRegisters::d15));
 
 static void
-GenerateReturn(MacroAssembler &masm, int returnCode)
+GenerateReturn(MacroAssembler &masm, int returnCode, SPSProfiler *prof)
 {
     // Restore non-volatile floating point registers
     masm.transferMultipleByRuns(NonVolatileFloatRegs, IsLoad, StackPointer, IA);
 
-    // Get rid of the bogus r0 push.
-    masm.as_add(sp, sp, Imm8(4));
+    // Unwind the sps mark.
+    masm.spsUnmarkJit(prof, r8);
 
     // Set up return value
     masm.ma_mov(Imm32(returnCode), r0);
@@ -71,7 +71,7 @@ struct EnterJITStack
     double d14;
     double d15;
 
-    void *r0; // alignment.
+    size_t hasSPSMark;
 
     // non-volatile registers.
     void *r4;
@@ -119,19 +119,24 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
     // rather than the JIT'd code, because they are scanned by the conservative
     // scanner.
     masm.startDataTransferM(IsStore, sp, DB, WriteBack);
-    masm.transferReg(r0); // [sp,0]
-    masm.transferReg(r4); // [sp,4]
-    masm.transferReg(r5); // [sp,8]
-    masm.transferReg(r6); // [sp,12]
-    masm.transferReg(r7); // [sp,16]
-    masm.transferReg(r8); // [sp,20]
-    masm.transferReg(r9); // [sp,24]
-    masm.transferReg(r10); // [sp,28]
-    masm.transferReg(r11); // [sp,32]
+    masm.transferReg(r4); // [sp,0]
+    masm.transferReg(r5); // [sp,4]
+    masm.transferReg(r6); // [sp,8]
+    masm.transferReg(r7); // [sp,12]
+    masm.transferReg(r8); // [sp,16]
+    masm.transferReg(r9); // [sp,20]
+    masm.transferReg(r10); // [sp,24]
+    masm.transferReg(r11); // [sp,28]
     // The abi does not expect r12 (ip) to be preserved
-    masm.transferReg(lr);  // [sp,36]
-    // The 5th argument is located at [sp, 40]
+    masm.transferReg(lr);  // [sp,32]
+    // The 5th argument is located at [sp, 36]
     masm.finishDataTransfer();
+
+    // Push the EnterJIT sps mark.  "Frame pointer" = start of saved core regs.
+    masm.movePtr(sp, r8);
+    masm.spsMarkJit(&cx->runtime()->spsProfiler, r8, r9);
+
+    // Push the float registers.
     masm.transferMultipleByRuns(NonVolatileFloatRegs, IsStore, sp, DB);
 
     // Save stack pointer into r8
@@ -324,7 +329,7 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
     //                   JSReturnReg_Data, EDtrAddr(r5, EDtrOffImm(0)));
 
     // Restore non-volatile registers and return.
-    GenerateReturn(masm, true);
+    GenerateReturn(masm, true, &cx->runtime()->spsProfiler);
 
     Linker linker(masm);
     JitCode *code = linker.newCode<NoGC>(cx, JSC::OTHER_CODE);

js/src/jit/x64/Trampoline-x64.cpp

@@ -81,6 +81,9 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
     masm.movdqa(xmm15, Operand(rsp, 16 * 9));
 #endif
 
+    // Push the EnterJIT sps mark.
+    masm.spsMarkJit(&cx->runtime()->spsProfiler, rbp, rbx);
+
     // Save arguments passed in registers needed after function call.
     masm.push(result);
 
@@ -263,6 +266,9 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
     masm.pop(r12); // vp
     masm.storeValue(JSReturnOperand, Operand(r12, 0));
 
+    // Unwind the sps mark.
+    masm.spsUnmarkJit(&cx->runtime()->spsProfiler, rbx);
+
     // Restore non-volatile registers.
 #if defined(_WIN64)
     masm.movdqa(Operand(rsp, 16 * 0), xmm6);

js/src/jit/x86/Trampoline-x86.cpp

@@ -61,6 +61,12 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
     masm.push(ebx);
     masm.push(esi);
     masm.push(edi);
+
+    // Push the EnterJIT sps mark.
+    masm.spsMarkJit(&cx->runtime()->spsProfiler, ebp, ebx);
+
+    // Keep track of the stack which has to be unwound after returning from the
+    // compiled function.
     masm.movl(esp, esi);
 
     // eax <- 8*argc, eax is now the offset betwen argv and the last
@@ -77,7 +83,7 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
     //   +4 for pushing the return address
     masm.movl(esp, ecx);
     masm.subl(eax, ecx);
-    masm.subl(Imm32(12), ecx);
+    masm.subl(Imm32(4 * 3), ecx);
 
     // ecx = ecx & 15, holds alignment.
     masm.andl(Imm32(15), ecx);
@@ -253,18 +259,22 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
 
     // |ebp| could have been clobbered by the inner function.
     // Grab the address for the Value result from the argument stack.
-    //  +18 ... arguments ...
-    //  +14 <return>
-    //  +10 ebp <- original %ebp pointing here.
-    //  +8  ebx
-    //  +4  esi
-    //  +0  edi
-    masm.loadPtr(Address(esp, ARG_RESULT + 3 * sizeof(void *)), eax);
+    //  +24 ... arguments ...
+    //  +20 <return>
+    //  +16 ebp <- original %ebp pointing here.
+    //  +12 ebx
+    //  +8  esi
+    //  +4  edi
+    //  +0  hasSPSFrame
+    masm.loadPtr(Address(esp, ARG_RESULT + 4 * sizeof(void *)), eax);
     masm.storeValue(JSReturnOperand, Operand(eax, 0));
 
     /**************************************************************
         Return stack and registers to correct state
     **************************************************************/
+    // Unwind the sps mark.
+    masm.spsUnmarkJit(&cx->runtime()->spsProfiler, ebx);
+
     // Restore non-volatile registers
     masm.pop(edi);
     masm.pop(esi);

js/src/vm/Interpreter.cpp

@@ -1711,6 +1711,7 @@ CASE(JSOP_LOOPENTRY)
         if (status == jit::Method_Error)
             goto error;
         if (status == jit::Method_Compiled) {
+            bool wasSPS = REGS.fp()->hasPushedSPSFrame();
             jit::IonExecStatus maybeOsr = jit::EnterBaselineAtBranch(cx, REGS.fp(), REGS.pc);
 
             // We failed to call into baseline at all, so treat as an error.
@@ -1719,6 +1720,11 @@ CASE(JSOP_LOOPENTRY)
 
             interpReturnOK = (maybeOsr == jit::IonExec_Ok);
 
+            // Pop the SPS frame pushed by the interpreter.  (The compiled version of the
+            // function popped a copy of the frame pushed by the OSR trampoline.)
+            if (wasSPS)
+                cx->runtime()->spsProfiler.exit(script, script->functionNonDelazifying());
+
             if (activation.entryFrame() != REGS.fp())
                 goto jit_return_pop_frame;
             goto leave_on_safe_point;

tools/profiler/tests/test_enterjit_osr.js

@@ -0,0 +1,58 @@
+// Check that the EnterJIT frame, added by the JIT trampoline and
+// usable by a native unwinder to resume unwinding after encountering
+// JIT code, is pushed as expected.
+function run_test() {
+    let p = Cc["@mozilla.org/tools/profiler;1"];
+    // Just skip the test if the profiler component isn't present.
+    if (!p)
+	return;
+    p = p.getService(Ci.nsIProfiler);
+    if (!p)
+	return;
+
+    // This test assumes that it's starting on an empty SPS stack.
+    // (Note that the other profiler tests also assume the profiler
+    // isn't already started.)
+    do_check_true(!p.IsActive());
+
+    const ms = 5;
+    p.StartProfiler(100, ms, ["js"], 1);
+    let profile = (function arbitrary_name(){
+	// A frame for |arbitrary_name| has been pushed.
+	let then = Date.now();
+	do {
+	    let n = 10000;
+	    while (--n); // OSR happens here
+	    // Spin until we're sure we have a sample.
+	} while (Date.now() - then < ms * 2.5);
+	return p.getProfileData().threads[0].samples;
+    })();
+    do_check_neq(profile.length, 0);
+    let stack = profile[profile.length - 1].frames.map(f => f.location);
+    stack = stack.slice(stack.indexOf("js::RunScript") + 1);
+
+    do_print(stack);
+    // This test needs to not break on platforms and configurations
+    // where IonMonkey isn't available / enabled.
+    if (stack.length < 2 || stack[1] != "EnterJIT") {
+	do_print("No JIT?");
+	// Try to check what we can....
+	do_check_eq(Math.min(stack.length, 1), 1);
+	let thisInterp = stack[0];
+	do_check_eq(thisInterp.split(" ")[0], "arbitrary_name");
+	if (stack.length >= 2) {
+	    let nextFrame = stack[1];
+	    do_check_neq(nextFrame.split(" ")[0], "arbitrary_name");
+	}
+    } else {
+	do_check_eq(Math.min(stack.length, 3), 3);
+	let thisInterp = stack[0];
+	let enterJit = stack[1];
+	let thisBC = stack[2];
+	do_check_eq(thisInterp.split(" ")[0], "arbitrary_name");
+	do_check_eq(enterJit, "EnterJIT");
+	do_check_eq(thisBC.split(" ")[0], "arbitrary_name");
+    }
+
+    p.StopProfiler();
+}

tools/profiler/tests/test_enterjit_osr_disabling.js

@@ -0,0 +1,21 @@
+function run_test() {
+    let p = Cc["@mozilla.org/tools/profiler;1"];
+    // Just skip the test if the profiler component isn't present.
+    if (!p)
+	return;
+    p = p.getService(Ci.nsIProfiler);
+    if (!p)
+	return;
+
+    do_check_true(!p.IsActive());
+
+    p.StartProfiler(100, 10, ["js"], 1);
+    // The function is entered with the profiler enabled
+    (function (){
+	p.StopProfiler();
+	let n = 10000;
+	while (--n);  // OSR happens here with the profiler disabled.
+	// An assertion will fail when this function returns, if the
+	// SPS stack was misbalanced.
+    })();
+}

tools/profiler/tests/test_enterjit_osr_enabling.js

@@ -0,0 +1,21 @@
+function run_test() {
+    let p = Cc["@mozilla.org/tools/profiler;1"];
+    // Just skip the test if the profiler component isn't present.
+    if (!p)
+	return;
+    p = p.getService(Ci.nsIProfiler);
+    if (!p)
+	return;
+
+    do_check_true(!p.IsActive());
+
+    // The function is entered with the profiler disabled.
+    (function (){
+	p.StartProfiler(100, 10, ["js"], 1);
+	let n = 10000;
+	while (--n); // OSR happens here with the profiler enabled.
+	// An assertion will fail when this function returns, if the
+	// SPS stack was misbalanced.
+    })();
+    p.StopProfiler();
+}

tools/profiler/tests/xpcshell.ini

@@ -9,3 +9,8 @@ skip-if = true
 [test_run.js]
 skip-if = true
 [test_pause.js]
+[test_enterjit_osr.js]
+[test_enterjit_osr_disabling.js]
+skip-if = !debug
+[test_enterjit_osr_enabling.js]
+skip-if = !debug