From: Alex Rønne Petersen Date: Wed, 23 Apr 2014 16:20:08 +0000 (+0200) Subject: Emit profiler enter/leave instrumentation calls as icalls. X-Git-Url: http://wien.tomnetworks.com/gitweb/?a=commitdiff_plain;h=7b48e454d7352466aa2b86dcf4a0025ef66e5425;p=mono.git Emit profiler enter/leave instrumentation calls as icalls. This is more portable than using the mono_arch_instrument_* functions and thus isn't prone to nasty arch-specific bugs. In particular, the old way of instrumenting function prologues/epilogues resulted in a somewhat elusive stack corruption bug on Android (ARM). Eventually, we should get rid of these functions entirely and only use icalls, with JIT opcodes for special cases (such as saving arguments for --trace). This new way of doing things also makes profiler instrumentation AOT-safe, since we're now going through the proper channels to emit references to the runtime functions (mono_profiler_method_enter/leave) and to embed MonoMethod pointers. Since we're using icalls without wrappers to do this, there should be no visible performance impact from this commit. --- diff --git a/mono/mini/method-to-ir.c b/mono/mini/method-to-ir.c index 1ecf79c9ac3..f57da5f405f 100644 --- a/mono/mini/method-to-ir.c +++ b/mono/mini/method-to-ir.c @@ -2027,6 +2027,24 @@ emit_pop_lmf (MonoCompile *cfg) } } +static void +emit_instrumentation_call (MonoCompile *cfg, void *func) +{ + MonoInst *iargs [1]; + + /* + * Avoid instrumenting inlined methods since it can + * distort profiling results. + */ + if (cfg->method != cfg->current_method) + return; + + if (cfg->prof_options & MONO_PROFILE_ENTER_LEAVE) { + EMIT_NEW_METHODCONST (cfg, iargs [0], cfg->method); + mono_emit_jit_icall (cfg, func, iargs); + } +} + static int ret_type_to_call_opcode (MonoType *type, int calli, int virt, MonoGenericSharingContext *gsctx) { @@ -2482,9 +2500,11 @@ mono_emit_call_args (MonoCompile *cfg, MonoMethodSignature *sig, int i; #endif - if (tail) + if (tail) { + emit_instrumentation_call (cfg, mono_profiler_method_leave); + MONO_INST_NEW_CALL (cfg, call, OP_TAILCALL); - else + } else MONO_INST_NEW_CALL (cfg, call, ret_type_to_call_opcode (sig->ret, calli, virtual, cfg->generic_sharing_context)); call->args = args; @@ -7746,6 +7766,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (mono_security_cas_enabled ()) CHECK_CFG_EXCEPTION; + emit_instrumentation_call (cfg, mono_profiler_method_leave); + if (ARCH_HAVE_OP_TAIL_CALL) { MonoMethodSignature *fsig = mono_method_signature (cmethod); int i, n; @@ -8555,6 +8577,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b /* Handle tail calls similarly to normal calls */ tail_call = TRUE; } else { + emit_instrumentation_call (cfg, mono_profiler_method_leave); + MONO_INST_NEW_CALL (cfg, call, OP_JMP); call->tail_call = TRUE; call->method = cmethod; @@ -8682,6 +8706,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b cfg->ret_var_set = TRUE; } } else { + emit_instrumentation_call (cfg, mono_profiler_method_leave); + if (cfg->lmf_var && cfg->cbb->in_count) emit_pop_lmf (cfg); @@ -12179,6 +12205,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b emit_push_lmf (cfg); } + cfg->cbb = init_localsbb; + emit_instrumentation_call (cfg, mono_profiler_method_enter); + if (seq_points) { MonoBasicBlock *bb; diff --git a/mono/mini/mini-amd64.c b/mono/mini/mini-amd64.c index 9b1c7bdb181..b5fed14d8d2 100644 --- a/mono/mini/mini-amd64.c +++ b/mono/mini/mini-amd64.c @@ -4826,10 +4826,6 @@ mono_arch_output_basic_block (MonoCompile *cfg, MonoBasicBlock *bb) MonoCallInst *call = (MonoCallInst*)ins; int i, save_area_offset; - /* FIXME: no tracing support... */ - if (cfg->prof_options & MONO_PROFILE_ENTER_LEAVE) - code = mono_arch_instrument_epilog_full (cfg, mono_profiler_method_leave, code, FALSE, TRUE); - g_assert (!cfg->method->save_lmf); /* Restore callee saved registers */ diff --git a/mono/mini/mini-x86.c b/mono/mini/mini-x86.c index d34fc64190b..02a6d1daa78 100644 --- a/mono/mini/mini-x86.c +++ b/mono/mini/mini-x86.c @@ -3186,9 +3186,6 @@ mono_arch_output_basic_block (MonoCompile *cfg, MonoBasicBlock *bb) ins->flags |= MONO_INST_GC_CALLSITE; ins->backend.pc_offset = code - cfg->native_code; - /* FIXME: no tracing support... */ - if (cfg->prof_options & MONO_PROFILE_ENTER_LEAVE) - code = mono_arch_instrument_epilog (cfg, mono_profiler_method_leave, code, FALSE); /* reset offset to make max_len work */ offset = code - cfg->native_code; diff --git a/mono/mini/mini.c b/mono/mini/mini.c index 3a9ff6b7c27..c1d56871323 100644 --- a/mono/mini/mini.c +++ b/mono/mini/mini.c @@ -4102,9 +4102,6 @@ mono_codegen (MonoCompile *cfg) code = mono_arch_emit_prolog (cfg); - if (cfg->prof_options & MONO_PROFILE_ENTER_LEAVE) - code = mono_arch_instrument_prolog (cfg, mono_profiler_method_enter, code, FALSE); - cfg->code_len = code - cfg->native_code; cfg->prolog_end = cfg->code_len; @@ -4120,14 +4117,6 @@ mono_codegen (MonoCompile *cfg) if (bb == cfg->bb_exit) { cfg->epilog_begin = cfg->code_len; - - if (cfg->prof_options & MONO_PROFILE_ENTER_LEAVE) { - code = cfg->native_code + cfg->code_len; - code = mono_arch_instrument_epilog (cfg, mono_profiler_method_leave, code, FALSE); - cfg->code_len = code - cfg->native_code; - g_assert (cfg->code_len < cfg->code_size); - } - mono_arch_emit_epilog (cfg); } } @@ -7429,8 +7418,17 @@ mini_init (const char *filename, const char *runtime_version) mono_marshal_init (); mono_arch_register_lowlevel_calls (); - register_icall (mono_profiler_method_enter, "mono_profiler_method_enter", NULL, TRUE); - register_icall (mono_profiler_method_leave, "mono_profiler_method_leave", NULL, TRUE); + + /* + * It's important that we pass `TRUE` as the last argument here, as + * it causes the JIT to omit a wrapper for these icalls. If the JIT + * *did* emit a wrapper, we'd be looking at infinite recursion since + * the wrapper would call the icall which would call the wrapper and + * so on. + */ + register_icall (mono_profiler_method_enter, "mono_profiler_method_enter", "void ptr", TRUE); + register_icall (mono_profiler_method_leave, "mono_profiler_method_leave", "void ptr", TRUE); + register_icall (mono_trace_enter_method, "mono_trace_enter_method", NULL, TRUE); register_icall (mono_trace_leave_method, "mono_trace_leave_method", NULL, TRUE); register_icall (mono_get_lmf_addr, "mono_get_lmf_addr", "ptr", TRUE);