From a22f4e9c765dc76dc844b71741ce4b054775818f Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 4 Apr 2024 21:33:54 +0200 Subject: [PATCH] JIT: Fix profiler enter callback init reg trash logic (#100637) During prolog generation we sometimes generate code to call the profiler enter callback. This may trash the "initReg" that we expect to keep zeroed during the prolog. The logic to check if the initReg was being trashed was wrong in a couple of cases: - Most backends did not take into account that the logic also trashes the registers used for arguments to the enter callback - SysV x64 thought that the enter callback trashed the parameter registers, but it does not This generally did not cause issues because `genFnPrologCalleeRegArgs` is unnecessarily conservative around whether or not it trashes `initReg`, and it comes after the profiler callback in the prolog. However, with the rewrite of the homing function that is not going to be the case anymore. --- src/coreclr/jit/codegenarm.cpp | 5 ++++- src/coreclr/jit/codegenarm64.cpp | 6 +++++- src/coreclr/jit/codegenloongarch64.cpp | 6 +++++- src/coreclr/jit/codegenriscv64.cpp | 6 +++++- src/coreclr/jit/codegenxarch.cpp | 12 ++++++++---- src/coreclr/jit/targetamd64.h | 15 +++++++++------ 6 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/codegenarm.cpp b/src/coreclr/jit/codegenarm.cpp index bddc03e0b41a54..2c010f116a2657 100644 --- a/src/coreclr/jit/codegenarm.cpp +++ b/src/coreclr/jit/codegenarm.cpp @@ -1710,7 +1710,10 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed) 0, // argSize. Again, we have to lie about it EA_UNKNOWN); // retSize - if (initReg == argReg) + // If initReg is trashed, either because it was an arg to the enter + // callback, or because the enter callback itself trashes it, then it needs + // to be zero'ed again before using. + if (((RBM_PROFILER_ENTER_TRASH | RBM_PROFILER_ENTER_ARG) & genRegMask(initReg)) != RBM_NONE) { *pInitRegZeroed = false; } diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 74258cdd55a73a..cd1b1558d93e64 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -5455,7 +5455,11 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed) genEmitHelperCall(CORINFO_HELP_PROF_FCN_ENTER, 0, EA_UNKNOWN); - if ((genRegMask(initReg) & RBM_PROFILER_ENTER_TRASH) != RBM_NONE) + // If initReg is trashed, either because it was an arg to the enter + // callback, or because the enter callback itself trashes it, then it needs + // to be zero'ed again before using. + if (((RBM_PROFILER_ENTER_TRASH | RBM_PROFILER_ENTER_ARG_FUNC_ID | RBM_PROFILER_ENTER_ARG_CALLER_SP) & + genRegMask(initReg)) != RBM_NONE) { *pInitRegZeroed = false; } diff --git a/src/coreclr/jit/codegenloongarch64.cpp b/src/coreclr/jit/codegenloongarch64.cpp index d6f18005c767c5..94329e3486100a 100644 --- a/src/coreclr/jit/codegenloongarch64.cpp +++ b/src/coreclr/jit/codegenloongarch64.cpp @@ -8551,7 +8551,11 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed) genEmitHelperCall(CORINFO_HELP_PROF_FCN_ENTER, 0, EA_UNKNOWN); - if ((genRegMask(initReg) & RBM_PROFILER_ENTER_TRASH) != RBM_NONE) + // If initReg is trashed, either because it was an arg to the enter + // callback, or because the enter callback itself trashes it, then it needs + // to be zero'ed again before using. + if (((RBM_PROFILER_ENTER_TRASH | RBM_PROFILER_ENTER_ARG_FUNC_ID | RBM_PROFILER_ENTER_ARG_CALLER_SP) & + genRegMask(initReg)) != RBM_NONE) { *pInitRegZeroed = false; } diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 17d30e5ada2754..0df6f56c5b76de 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -8541,7 +8541,11 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed) genEmitHelperCall(CORINFO_HELP_PROF_FCN_ENTER, 0, EA_UNKNOWN); - if ((genRegMask(initReg) & RBM_PROFILER_ENTER_TRASH)) + // If initReg is trashed, either because it was an arg to the enter + // callback, or because the enter callback itself trashes it, then it needs + // to be zero'ed again before using. + if (((RBM_PROFILER_ENTER_TRASH | RBM_PROFILER_ENTER_ARG_FUNC_ID | RBM_PROFILER_ENTER_ARG_CALLER_SP) & + genRegMask(initReg)) != RBM_NONE) { *pInitRegZeroed = false; } diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 77045cce0875ad..f25e5bb046d29d 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -9491,8 +9491,10 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed) } } - // If initReg is one of RBM_CALLEE_TRASH, then it needs to be zero'ed before using. - if ((RBM_CALLEE_TRASH & genRegMask(initReg)) != 0) + // If initReg is trashed, either because it was an arg to the enter + // callback, or because the enter callback itself trashes it, then it needs + // to be zero'ed again before using. + if (((RBM_PROFILER_ENTER_TRASH | RBM_ARG_0 | RBM_ARG_1) & genRegMask(initReg)) != 0) { *pInitRegZeroed = false; } @@ -9528,8 +9530,10 @@ void CodeGen::genProfilingEnterCallback(regNumber initReg, bool* pInitRegZeroed) // "mov r11, helper addr; call r11" genEmitHelperCall(CORINFO_HELP_PROF_FCN_ENTER, 0, EA_UNKNOWN, REG_DEFAULT_PROFILER_CALL_TARGET); - // If initReg is one of RBM_CALLEE_TRASH, then it needs to be zero'ed before using. - if ((RBM_CALLEE_TRASH & genRegMask(initReg)) != 0) + // If initReg is trashed, either because it was an arg to the enter + // callback, or because the enter callback itself trashes it, then it needs + // to be zero'ed again before using. + if (((RBM_PROFILER_ENTER_TRASH | RBM_PROFILER_ENTER_ARG_0 | RBM_PROFILER_ENTER_ARG_1) & genRegMask(initReg)) != 0) { *pInitRegZeroed = false; } diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index 5d37870d03b03a..7d1a2c8f08039f 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -511,12 +511,6 @@ #define RBM_FLTARG_REGS (RBM_FLTARG_0|RBM_FLTARG_1|RBM_FLTARG_2|RBM_FLTARG_3) #endif // !UNIX_AMD64_ABI - // The registers trashed by profiler enter/leave/tailcall hook - // See vm\amd64\asmhelpers.asm for more details. - #define RBM_PROFILER_ENTER_TRASH RBM_CALLEE_TRASH - - #define RBM_PROFILER_TAILCALL_TRASH RBM_PROFILER_LEAVE_TRASH - // The registers trashed by the CORINFO_HELP_STOP_FOR_GC helper. #ifdef UNIX_AMD64_ABI // See vm\amd64\unixasmhelpers.S for more details. @@ -525,11 +519,20 @@ // The return registers could be any two from the set { RAX, RDX, XMM0, XMM1 }. // STOP_FOR_GC helper preserves all the 4 possible return registers. #define RBM_STOP_FOR_GC_TRASH (RBM_CALLEE_TRASH & ~(RBM_FLOATRET | RBM_INTRET | RBM_FLOATRET_1 | RBM_INTRET_1)) + + // The registers trashed by profiler enter/leave/tailcall hook + // See vm\amd64\asmhelpers.S for more details. + #define RBM_PROFILER_ENTER_TRASH (RBM_CALLEE_TRASH & ~(RBM_ARG_REGS|RBM_FLTARG_REGS)) #define RBM_PROFILER_LEAVE_TRASH (RBM_CALLEE_TRASH & ~(RBM_FLOATRET | RBM_INTRET | RBM_FLOATRET_1 | RBM_INTRET_1)) + #define RBM_PROFILER_TAILCALL_TRASH RBM_PROFILER_LEAVE_TRASH + #else // See vm\amd64\asmhelpers.asm for more details. #define RBM_STOP_FOR_GC_TRASH (RBM_CALLEE_TRASH & ~(RBM_FLOATRET | RBM_INTRET)) + + #define RBM_PROFILER_ENTER_TRASH RBM_CALLEE_TRASH #define RBM_PROFILER_LEAVE_TRASH (RBM_CALLEE_TRASH & ~(RBM_FLOATRET | RBM_INTRET)) + #define RBM_PROFILER_TAILCALL_TRASH RBM_PROFILER_LEAVE_TRASH #endif // The registers trashed by the CORINFO_HELP_INIT_PINVOKE_FRAME helper.