From aa631aaa03031e3e8e3a015a346d00a5d026880a Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 5 Sep 2019 23:10:13 +0300 Subject: [PATCH] [interp] Kill InterpFrame.ex (mono/mono#16666) * [interp] Reduce unnecessary uses of frame->ex * [interp] Remove ex from InterpFrame We save stack space and move the exception where it belongs, in ThreadContext. It was confusing what frame->ex really meant, it was probably not used / initialized properly. We move it now to ThreadContext where it represents the current exception being thrown during EH (when we have a resume state set). Commit migrated from https://github.com/mono/mono/commit/f0dabc6618931ccd4c544657ed1a7f3d3d803c25 --- src/mono/mono/mini/interp/interp-internals.h | 7 +- src/mono/mono/mini/interp/interp.c | 181 ++++++++++++--------------- 2 files changed, 83 insertions(+), 105 deletions(-) diff --git a/src/mono/mono/mini/interp/interp-internals.h b/src/mono/mono/mini/interp/interp-internals.h index c4f666a..4c24b9c 100644 --- a/src/mono/mono/mini/interp/interp-internals.h +++ b/src/mono/mono/mini/interp/interp-internals.h @@ -145,20 +145,21 @@ struct _InterpFrame { #endif /* exception info */ const unsigned short *ip; - MonoException *ex; }; #define frame_locals(frame) (((guchar*)((frame)->stack)) + (frame)->imethod->stack_size + (frame)->imethod->vt_stack_size) typedef struct { - /* Resume state for resuming execution in mixed mode */ - gboolean has_resume_state; + /* Lets interpreter know it has to resume execution after EH */ + gboolean has_resume_state; /* Frame to resume execution at */ InterpFrame *handler_frame; /* IP to resume execution at */ const guint16 *handler_ip; /* Clause that we are resuming to */ MonoJitExceptionInfo *handler_ei; + /* Exception that is being thrown. Set with rest of resume state */ + guint32 exc_gchandle; } ThreadContext; typedef struct { diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index e6d2c6b..ff55fe56 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -105,7 +105,6 @@ init_frame (InterpFrame *frame, InterpFrame *parent_frame, InterpMethod *rmethod frame->stack_args = method_args; frame->retval = method_retval; frame->imethod = rmethod; - frame->ex = NULL; frame->ip = NULL; } @@ -232,17 +231,19 @@ int mono_interp_traceopt = 0; // FIXME The inlining of this needs to be reevaluated in the context of later changes. // Also there is now only one caller, so consider inlining it manually. static MONO_NEVER_INLINE GSList* // Inlining this causes caller to use more stack. -set_resume_state (ThreadContext *context, InterpFrame *frame, GSList* finally_ips) +clear_resume_state (ThreadContext *context, InterpFrame *frame, GSList* finally_ips) { /* We have thrown an exception from a finally block. Some of the leave targets were unwound already */ while (finally_ips && finally_ips->data >= context->handler_ei->try_start && finally_ips->data < context->handler_ei->try_end) finally_ips = g_slist_remove (finally_ips, finally_ips->data); - frame->ex = NULL; context->has_resume_state = 0; context->handler_frame = NULL; context->handler_ei = NULL; + g_assert (context->exc_gchandle); + mono_gchandle_free_internal (context->exc_gchandle); + context->exc_gchandle = 0; return finally_ips; } @@ -893,7 +894,6 @@ interp_throw (ThreadContext *context, MonoException *ex, InterpFrame *frame, con interp_push_lmf (&ext, frame); frame->ip = ip; - frame->ex = ex; if (mono_object_isinst_checked ((MonoObject *) ex, mono_defaults.exception_class, error)) { MonoException *mono_ex = ex; @@ -982,9 +982,8 @@ ves_array_create (MonoDomain *domain, MonoClass *klass, int param_count, stackva } static gint32 -ves_array_calculate_index (MonoArray *ao, stackval *sp, InterpFrame *frame, gboolean safe) +ves_array_calculate_index (MonoArray *ao, stackval *sp, gboolean safe) { - g_assert (!frame->ex); MonoClass *ac = ((MonoObject *) ao)->vtable->klass; guint32 pos = 0; @@ -993,23 +992,19 @@ ves_array_calculate_index (MonoArray *ao, stackval *sp, InterpFrame *frame, gboo guint32 idx = sp [i].data.i; guint32 lower = ao->bounds [i].lower_bound; guint32 len = ao->bounds [i].length; - if (safe && (idx < lower || (idx - lower) >= len)) { - frame->ex = mono_get_exception_index_out_of_range (); + if (safe && (idx < lower || (idx - lower) >= len)) return -1; - } pos = (pos * len) + idx - lower; } } else { pos = sp [0].data.i; - if (safe && pos >= ao->max_length) { - frame->ex = mono_get_exception_index_out_of_range (); + if (safe && pos >= ao->max_length) return -1; - } } return pos; } -static MONO_NEVER_INLINE void +static MONO_NEVER_INLINE MonoException* ves_array_set (InterpFrame *frame, stackval *sp, MonoMethodSignature *sig) { MonoObject *o = sp->data.o; @@ -1018,19 +1013,17 @@ ves_array_set (InterpFrame *frame, stackval *sp, MonoMethodSignature *sig) g_assert (m_class_get_rank (ac) >= 1); - gint32 pos = ves_array_calculate_index (ao, sp + 1, frame, TRUE); - if (frame->ex) - return; + gint32 pos = ves_array_calculate_index (ao, sp + 1, TRUE); + if (pos == -1) + return mono_get_exception_index_out_of_range (); int val_index = 1 + m_class_get_rank (ac); if (sp [val_index].data.p && !m_class_is_valuetype (m_class_get_element_class (mono_object_class (o)))) { ERROR_DECL (error); MonoObject *isinst = mono_object_isinst_checked (sp [val_index].data.o, m_class_get_element_class (mono_object_class (o)), error); mono_error_cleanup (error); - if (!isinst) { - frame->ex = mono_get_exception_array_type_mismatch (); - return; - } + if (!isinst) + return mono_get_exception_array_type_mismatch (); } gint32 esize = mono_array_element_size (ac); @@ -1038,9 +1031,10 @@ ves_array_set (InterpFrame *frame, stackval *sp, MonoMethodSignature *sig) MonoType *mt = sig->params [m_class_get_rank (ac)]; stackval_to_data (mt, &sp [val_index], ea, FALSE); + return NULL; } -static void +static MonoException* ves_array_get (InterpFrame *frame, stackval *sp, stackval *retval, MonoMethodSignature *sig, gboolean safe) { MonoObject *o = sp->data.o; @@ -1049,34 +1043,34 @@ ves_array_get (InterpFrame *frame, stackval *sp, stackval *retval, MonoMethodSig g_assert (m_class_get_rank (ac) >= 1); - gint32 pos = ves_array_calculate_index (ao, sp + 1, frame, safe); - if (frame->ex) - return; + gint32 pos = ves_array_calculate_index (ao, sp + 1, safe); + if (pos == -1) + return mono_get_exception_index_out_of_range (); gint32 esize = mono_array_element_size (ac); gconstpointer ea = mono_array_addr_with_size_fast (ao, esize, pos); MonoType *mt = sig->ret; stackval_from_data (mt, retval, ea, FALSE); + return NULL; } -static MONO_NEVER_INLINE gpointer +static MONO_NEVER_INLINE MonoException* ves_array_element_address (InterpFrame *frame, MonoClass *required_type, MonoArray *ao, stackval *sp, gboolean needs_typecheck) { MonoClass *ac = ((MonoObject *) ao)->vtable->klass; g_assert (m_class_get_rank (ac) >= 1); - gint32 pos = ves_array_calculate_index (ao, sp, frame, TRUE); - if (frame->ex) - return NULL; + gint32 pos = ves_array_calculate_index (ao, sp, TRUE); + if (pos == -1) + return mono_get_exception_index_out_of_range (); - if (needs_typecheck && !mono_class_is_assignable_from_internal (m_class_get_element_class (mono_object_class ((MonoObject *) ao)), required_type)) { - frame->ex = mono_get_exception_array_type_mismatch (); - return NULL; - } + if (needs_typecheck && !mono_class_is_assignable_from_internal (m_class_get_element_class (mono_object_class ((MonoObject *) ao)), required_type)) + return mono_get_exception_array_type_mismatch (); gint32 esize = mono_array_element_size (ac); - return mono_array_addr_with_size_fast (ao, esize, pos); + sp [-1].data.p = mono_array_addr_with_size_fast (ao, esize, pos); + return NULL; } #ifdef MONO_ARCH_HAVE_INTERP_ENTRY_TRAMPOLINE @@ -1371,8 +1365,6 @@ ves_pinvoke_method (InterpFrame *frame, MonoMethodSignature *sig, MonoFuncV addr MonoLMFExt ext; gpointer args; - frame->ex = NULL; - g_assert (!frame->imethod); static MonoPIFunc entry_func = NULL; @@ -1409,13 +1401,13 @@ ves_pinvoke_method (InterpFrame *frame, MonoMethodSignature *sig, MonoFuncV addr interp_pop_lmf (&ext); #ifdef MONO_ARCH_HAVE_INTERP_PINVOKE_TRAMP - if (!frame->ex) + if (!context->has_resume_state) mono_arch_get_native_call_context_ret (&ccontext, frame, sig); if (ccontext.stack != NULL) g_free (ccontext.stack); #else - if (!frame->ex && !MONO_TYPE_ISSTRUCT (sig->ret)) + if (!context->has_resume_state && !MONO_TYPE_ISSTRUCT (sig->ret)) stackval_from_data (sig->ret, frame->retval, (char*)&frame->retval->data.p, sig->pinvoke); g_free (margs->iargs); @@ -1514,7 +1506,7 @@ interp_delegate_ctor (MonoObjectHandle this_obj, MonoObjectHandle target, gpoint * runtime specifies that the implementation of the method is automatically * provided by the runtime and is primarily used for the methods of delegates. */ -static MONO_NEVER_INLINE void +static MONO_NEVER_INLINE MonoException* ves_imethod (InterpFrame *frame, MonoMethod *method, MonoMethodSignature *sig, stackval *sp, stackval *retval) { const char *name = method->name; @@ -1524,26 +1516,18 @@ ves_imethod (InterpFrame *frame, MonoMethod *method, MonoMethodSignature *sig, s if (!strcmp (name, "UnsafeMov")) { /* TODO: layout checks */ stackval_from_data (sig->ret, retval, (char*) sp, FALSE); - return; - } - if (!strcmp (name, "UnsafeLoad")) { - ves_array_get (frame, sp, retval, sig, FALSE); - return; + return NULL; } + if (!strcmp (name, "UnsafeLoad")) + return ves_array_get (frame, sp, retval, sig, FALSE); } else if (mini_class_is_system_array (method->klass)) { MonoObject *obj = (MonoObject*) sp->data.p; - if (!obj) { - frame->ex = mono_get_exception_null_reference (); - return; - } - if (*name == 'S' && (strcmp (name, "Set") == 0)) { - ves_array_set (frame, sp, sig); - return; - } - if (*name == 'G' && (strcmp (name, "Get") == 0)) { - ves_array_get (frame, sp, retval, sig, TRUE); - return; - } + if (!obj) + return mono_get_exception_null_reference (); + if (*name == 'S' && (strcmp (name, "Set") == 0)) + return ves_array_set (frame, sp, sig); + if (*name == 'G' && (strcmp (name, "Get") == 0)) + return ves_array_get (frame, sp, retval, sig, TRUE); } g_error ("Don't know how to exec runtime method %s.%s::%s", @@ -1735,8 +1719,6 @@ interp_runtime_invoke (MonoMethod *method, void *obj, void **params, MonoObject if (exc) *exc = NULL; - frame.ex = NULL; - MonoDomain *domain = mono_domain_get (); if (method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) @@ -1762,12 +1744,13 @@ interp_runtime_invoke (MonoMethod *method, void *obj, void **params, MonoObject interp_exec_method (&frame, context, error); - if (frame.ex) { - if (exc) { - *exc = (MonoObject*) frame.ex; - return NULL; - } - mono_error_set_exception_instance (error, frame.ex); + if (context->has_resume_state) { + // This can happen on wasm !? + MonoException *thrown_exc = (MonoException*) mono_gchandle_get_target_internal (context->exc_gchandle); + if (exc) + *exc = (MonoObject*)thrown_exc; + else + mono_error_set_exception_instance (error, thrown_exc); return NULL; } return (MonoObject*)result.data.p; @@ -1815,8 +1798,6 @@ interp_entry (InterpEntryData *data) //printf ("%s\n", mono_method_full_name (method, 1)); - frame.ex = NULL; - args = g_newa (stackval, sig->param_count + (sig->hasthis ? 1 : 0)); if (sig->hasthis) args [0].data.p = data->this_arg; @@ -1868,14 +1849,16 @@ interp_entry (InterpEntryData *data) ERROR_DECL (error); interp_exec_method (&frame, context, error); + g_assert (!context->has_resume_state); + if (rmethod->needs_thread_attach) mono_threads_detach_coop (orig_domain, &attach_cookie); if (mono_llvm_only) { - if (frame.ex) - mono_llvm_reraise_exception (frame.ex); + if (context->has_resume_state) + mono_llvm_reraise_exception ((MonoException*)mono_gchandle_get_target_internal (context->exc_gchandle)); } else { - g_assert (frame.ex == NULL); + g_assert (!context->has_resume_state); } type = rmethod->rtype; @@ -2335,7 +2318,7 @@ do_debugger_tramp (void (*tramp) (void), InterpFrame *frame) interp_pop_lmf (&ext); } -static MONO_NEVER_INLINE void +static MONO_NEVER_INLINE MonoException* do_transform_method (InterpFrame *frame, ThreadContext *context) { MonoLMFExt ext; @@ -2348,10 +2331,11 @@ do_transform_method (InterpFrame *frame, ThreadContext *context) interp_push_lmf (&ext, frame->parent); mono_interp_transform_method (frame->imethod, context, error); - frame->ex = mono_error_convert_to_exception (error); if (push_lmf) interp_pop_lmf (&ext); + + return mono_error_convert_to_exception (error); } static MONO_NEVER_INLINE guchar* @@ -2577,8 +2561,6 @@ interp_entry_from_trampoline (gpointer ccontext_untyped, gpointer rmethod_untype method = rmethod->method; sig = mono_method_signature_internal (method); - frame.ex = NULL; - args = (stackval*)alloca (sizeof (stackval) * (sig->param_count + (sig->hasthis ? 1 : 0))); init_frame (&frame, NULL, rmethod, args, &result); @@ -2598,12 +2580,11 @@ interp_entry_from_trampoline (gpointer ccontext_untyped, gpointer rmethod_untype ERROR_DECL (error); interp_exec_method (&frame, context, error); + g_assert (!context->has_resume_state); + if (rmethod->needs_thread_attach) mono_threads_detach_coop (orig_domain, &attach_cookie); - // FIXME: - g_assert (frame.ex == NULL); - /* Write back the return value */ mono_arch_set_native_call_context_ret (ccontext, &frame, sig); } @@ -3273,7 +3254,6 @@ interp_exec_method_full (InterpFrame *frame, ThreadContext *context, FrameClause }; #endif - frame->ex = NULL; #if DEBUG_INTERP debug_enter (frame, &tracing); #endif @@ -3286,9 +3266,9 @@ interp_exec_method_full (InterpFrame *frame, ThreadContext *context, FrameClause #endif frame->ip = NULL; - do_transform_method (frame, context); - if (frame->ex) - THROW_EX (frame->ex, NULL); + MonoException *ex = do_transform_method (frame, context); + if (ex) + THROW_EX (ex, NULL); EXCEPTION_CHECKPOINT; } @@ -3467,9 +3447,9 @@ main_loop: frame->ip = ip; mono_interp_transform_method (new_method, context, error); - frame->ex = mono_error_convert_to_exception (error); - if (frame->ex) - goto exit_frame; + MonoException *ex = mono_error_convert_to_exception (error); + if (ex) + THROW_EX (ex, ip); } ip += 2; const gboolean realloc_frame = new_method->alloca_size > frame->imethod->alloca_size; @@ -3727,9 +3707,9 @@ main_loop: if (sig->hasthis) sp--; - ves_imethod (frame, target_method, sig, sp, retval); - if (frame->ex) - THROW_EX (frame->ex, ip); + MonoException *ex = ves_imethod (frame, target_method, sig, sp, retval); + if (ex) + THROW_EX (ex, ip); if (sig->ret->type != MONO_TYPE_VOID) { *sp = *retval; @@ -4773,7 +4753,6 @@ main_loop: ip += 2; // FIXME: Do this after throw? child_frame.ip = NULL; - child_frame.ex = NULL; child_frame.imethod = (InterpMethod*)frame->imethod->data_items [token]; MonoMethodSignature* const csig = mono_method_signature_internal (child_frame.imethod->method); @@ -5453,9 +5432,9 @@ main_loop: MonoClass *klass = (MonoClass*)frame->imethod->data_items [*(guint16 *) (ip - 3 + 1)]; const gboolean needs_typecheck = ip [-3] == MINT_LDELEMA_TC; - sp->data.p = ves_array_element_address (frame, klass, (MonoArray *) o, &sp [1], needs_typecheck); - if (frame->ex) - THROW_EX (frame->ex, ip); + MonoException *ex = ves_array_element_address (frame, klass, (MonoArray *) o, &sp [1], needs_typecheck); + if (ex) + THROW_EX (ex, ip); ++sp; MINT_IN_BREAK; @@ -5967,7 +5946,7 @@ main_loop: finally_ips = g_slist_prepend (finally_ips, (gpointer) ip); #if DEBUG_INTERP if (tracing) - g_print ("* Found finally at IL_%04x with exception: %s\n", clause->handler_offset, frame->ex? "yes": "no"); + g_print ("* Found finally at IL_%04x with exception: %s\n", clause->handler_offset, context->has_resume_state ? "yes": "no"); #endif } } @@ -6624,16 +6603,13 @@ resume: /* spec says stack should be empty at endfinally so it should be at the start too */ sp = frame->stack; vt_sp = (guchar*)sp + frame->imethod->stack_size; - if (frame->ex) { - sp->data.p = frame->ex; - ++sp; - } + g_assert (context->exc_gchandle); + sp->data.p = mono_gchandle_get_target_internal (context->exc_gchandle); + ++sp; - // FIXME Reevaluate if this should be inlined. - finally_ips = set_resume_state (context, frame, finally_ips); + finally_ips = clear_resume_state (context, frame, finally_ips); // goto main_loop instead of MINT_IN_DISPATCH helps the compiler and therefore conserves stack. // This is a slow/rare path and conserving stack is preferred over its performance otherwise. - // goto main_loop; } // fall through @@ -6643,7 +6619,7 @@ exit_frame: if (clause_args && clause_args->base_frame) memcpy (clause_args->base_frame->stack, frame->stack, frame->imethod->alloca_size); - if (!frame->ex && MONO_PROFILER_ENABLED (method_leave) && + if (!context->has_resume_state && MONO_PROFILER_ENABLED (method_leave) && frame->imethod->prof_flags & MONO_PROFILER_CALL_INSTRUMENTATION_LEAVE) { MonoProfilerCallContext *prof_ctx = NULL; @@ -6669,8 +6645,8 @@ exit_frame: MONO_PROFILER_RAISE (method_leave, (frame->imethod->method, prof_ctx)); g_free (prof_ctx); - } else if (frame->ex && frame->imethod->prof_flags & MONO_PROFILER_CALL_INSTRUMENTATION_EXCEPTION_LEAVE) - MONO_PROFILER_RAISE (method_exception_leave, (frame->imethod->method, &frame->ex->object)); + } else if (context->has_resume_state && frame->imethod->prof_flags & MONO_PROFILER_CALL_INSTRUMENTATION_EXCEPTION_LEAVE) + MONO_PROFILER_RAISE (method_exception_leave, (frame->imethod->method, mono_gchandle_get_target_internal (context->exc_gchandle))); DEBUG_LEAVE (); } @@ -6715,8 +6691,9 @@ interp_set_resume_state (MonoJitTlsData *jit_tls, MonoException *ex, MonoJitExce context->has_resume_state = TRUE; context->handler_frame = (InterpFrame*)interp_frame; context->handler_ei = ei; - /* This is on the stack, so it doesn't need a wbarrier */ - context->handler_frame->ex = ex; + if (context->exc_gchandle) + mono_gchandle_free_internal (context->exc_gchandle); + context->exc_gchandle = mono_gchandle_new_internal ((MonoObject*)ex, FALSE); /* Ditto */ if (ei) *(MonoException**)(frame_locals (context->handler_frame) + ei->exvar_offset) = ex; -- 2.7.4