Skip to content

Commit

Permalink
JIT: Fix profiler enter callback init reg trash logic (dotnet#100637)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jakobbotsch authored and matouskozak committed Apr 30, 2024
1 parent 61bd842 commit a22f4e9
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 14 deletions.
5 changes: 4 additions & 1 deletion src/coreclr/jit/codegenarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/codegenloongarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/codegenriscv64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 8 additions & 4 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
15 changes: 9 additions & 6 deletions src/coreclr/jit/targetamd64.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down

0 comments on commit a22f4e9

Please sign in to comment.