[mono][jit] Emit profiler enter after jit attach; leave before detach (#44345)
authorAleksey Kliger (λgeek) <alklig@microsoft.com>
Tue, 10 Nov 2020 20:11:20 +0000 (15:11 -0500)
committerGitHub <noreply@github.com>
Tue, 10 Nov 2020 20:11:20 +0000 (15:11 -0500)
* [jit] Emit profiler enter after jit attach; leave before detach

profiler enter code (such as mono_trace_enter_method) must not be
called before a thread attaches to the runtime - otherwise calls like
`mono_domain_get()` will return NULL unexpectedly and then crash.
When detaching, we should call the profiler leave code before the
detach, and suppress it on return.

Simple repro (that relies on pal_signal.c SignalHandlerLoop - which is
a background thread from System.Native that calls back into managed
when there's a SIGCHLD): compile and run this program with `MONO_ENV_OPTIONS=--trace`

```csharp
using System;
using System.Diagnostics;

namespace Repro
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            using (Process myProcess = new Process())
            {
                    myProcess.StartInfo.UseShellExecute = true;
                    myProcess.StartInfo.FileName = "echo";
                    myProcess.StartInfo.Arguments = "hello from shell";
                    myProcess.StartInfo.CreateNoWindow = true;
                    myProcess.Start();
                    myProcess.WaitForExit();
            }
            Console.ReadKey ();
        }
    }
}
```

src/mono/mono/mini/method-to-ir.c

index 6829bcb..1586923 100644 (file)
@@ -6090,6 +6090,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
        MonoBitSet *seq_point_locs = NULL;
        MonoBitSet *seq_point_set_locs = NULL;
        gboolean emitted_funccall_seq_point = FALSE;
+       gboolean detached_before_ret = FALSE;
 
        cfg->disable_inline = (method->iflags & METHOD_IMPL_ATTRIBUTE_NOOPTIMIZATION) || is_jit_optimizer_disabled (method);
        cfg->current_method = method;
@@ -8070,7 +8071,8 @@ calli_end:
                        break;
                }
                case MONO_CEE_RET:
-                       mini_profiler_emit_leave (cfg, sig->ret->type != MONO_TYPE_VOID ? sp [-1] : NULL);
+                       if (!detached_before_ret)
+                               mini_profiler_emit_leave (cfg, sig->ret->type != MONO_TYPE_VOID ? sp [-1] : NULL);
 
                        g_assert (!method_does_not_return (method));
 
@@ -10618,11 +10620,17 @@ field_access_end:
 
                                /*
                                 * Parts of the initlocals code needs to come after this, since it might call methods like memset.
+                                * Also profiling needs to be after attach.
                                 */
                                init_localsbb2 = cfg->cbb;
                                NEW_BBLOCK (cfg, next_bb);
                                MONO_START_BB (cfg, next_bb);
                        } else {
+                               if (token == MONO_JIT_ICALL_mono_threads_detach_coop) {
+                                       /* can't emit profiling code after a detach, so emit it now */
+                                       mini_profiler_emit_leave (cfg, NULL);
+                                       detached_before_ret = TRUE;
+                               }
                                ins = mono_emit_jit_icall_id (cfg, jit_icall_id, sp);
                        }
 
@@ -10790,7 +10798,8 @@ mono_ldptr:
                        if (sp != stack_start)
                                UNVERIFIED;
 
-                       mini_profiler_emit_leave (cfg, sp [0]);
+                       if (!detached_before_ret)
+                               mini_profiler_emit_leave (cfg, sp [0]);
 
                        MONO_INST_NEW (cfg, ins, OP_BR);
                        ins->inst_target_bb = end_bblock;
@@ -11618,8 +11627,10 @@ mono_ldptr:
                emit_push_lmf (cfg);
        }
 
-       cfg->cbb = init_localsbb;
+       /* emit profiler enter code after a jit attach if there is one */
+       cfg->cbb = init_localsbb2;
        mini_profiler_emit_enter (cfg);
+       cfg->cbb = init_localsbb;
 
        if (seq_points) {
                MonoBasicBlock *bb;