[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

diff --git a/src/mono/mono/mini/interp/interp-internals.h b/src/mono/mono/mini/interp/interp-internals.h
index c4f666a..4c24b9c 100644 (file)
--- 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..ff55fe5 100644 (file)
--- 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;