From aba819956355c47a9dddbcba8a1aca591f176166 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 29 Nov 2024 13:53:13 +0200 Subject: [PATCH] [mono][interp] Properly handle exceptions thrown by mono_marshal_get_native_wrapper (#110232) * [mono][interp] Defer calls to mono_marshal_get_native_wrapper at execution This can end up calling into managed where exceptions can be thrown. Throwing exceptions while method compilation happens in not valid behavior. This follows the same approach from the jit side in https://github.com/mono/mono/pull/20177. * [mono][interp] Check resume state when returning from method compilation * [mono][interp] Fix tiering disable condition We always optimize managed wrappers. If we are attempting to compile and execute a pinvoke method, we will use the m2n wrapper instead. Make sure we check for this when disabling tiering, since swift interop relies on these wrappers to be always optimized. --- src/mono/mono/mini/interp/interp.c | 5 ++++- src/mono/mono/mini/interp/transform.c | 22 +++++++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 1dda6c2164891..df0abac566ed6 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -502,8 +502,9 @@ mono_interp_get_imethod (MonoMethod *method) imethod->is_invoke = (m_class_get_parent (method->klass) == mono_defaults.multicastdelegate_class) && !strcmp(method->name, "Invoke"); // always optimize code if tiering is disabled // always optimize wrappers - if (!mono_interp_tiering_enabled () || method->wrapper_type != MONO_WRAPPER_NONE) + if (!mono_interp_tiering_enabled () || method->wrapper_type != MONO_WRAPPER_NONE || (method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL)) imethod->optimized = TRUE; + if (imethod->method->string_ctor) imethod->rtype = m_class_get_byval_arg (mono_defaults.string_class); else @@ -3916,6 +3917,7 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause if (method_entry_ex) THROW_EX (method_entry_ex, NULL); EXCEPTION_CHECKPOINT; + CHECK_RESUME_STATE (context); } if (!clause_args) { @@ -4372,6 +4374,7 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause if (call_ex) THROW_EX (call_ex, NULL); EXCEPTION_CHECKPOINT; + CHECK_RESUME_STATE (context); } context->stack_pointer = (guchar*)frame->stack + cmethod->alloca_size; diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index d12857066f968..b515c2f7d9dc2 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2691,8 +2691,6 @@ static MonoMethod* interp_transform_internal_calls (MonoMethod *method, MonoMethod *target_method, MonoMethodSignature *csignature, gboolean is_virtual) { if (((method->wrapper_type == MONO_WRAPPER_NONE) || (method->wrapper_type == MONO_WRAPPER_DYNAMIC_METHOD)) && target_method != NULL) { - if (target_method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) - target_method = mono_marshal_get_native_wrapper (target_method, FALSE, FALSE); if (!is_virtual && target_method->iflags & METHOD_IMPL_ATTRIBUTE_SYNCHRONIZED) target_method = mono_marshal_get_synchronized_wrapper (target_method); @@ -3748,9 +3746,19 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target } } - if (op == -1) + if (op == -1) { target_method = interp_transform_internal_calls (method, target_method, csignature, is_virtual); + if (((method->wrapper_type == MONO_WRAPPER_NONE) || (method->wrapper_type == MONO_WRAPPER_DYNAMIC_METHOD)) && target_method != NULL) { + if (target_method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) { + // Avoid calling mono_marshal_get_native_wrapper () too early, it might call managed callbacks. + csignature = mono_metadata_signature_dup_mempool (td->mempool, csignature); + csignature->pinvoke = FALSE; + native = FALSE; + } + } + } + if (csignature->call_convention == MONO_CALL_VARARG) csignature = mono_method_get_signature_checked (target_method, image, token, generic_context, error); @@ -9898,7 +9906,8 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon generic_context = &generic_container->context; } - if (method->iflags & (METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL | METHOD_IMPL_ATTRIBUTE_RUNTIME)) { + if ((method->iflags & (METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL | METHOD_IMPL_ATTRIBUTE_RUNTIME)) || + (method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL)) { MonoMethod *nm = NULL; if (imethod->transformed) { MONO_PROFILER_RAISE (jit_done, (method, imethod->jinfo)); @@ -9906,8 +9915,11 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon } /* assumes all internal calls with an array this are built in... */ - if (method->iflags & METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL && (! mono_method_signature_internal (method)->hasthis || m_class_get_rank (method->klass) == 0)) { + if ((method->iflags & METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL && (! mono_method_signature_internal (method)->hasthis || m_class_get_rank (method->klass) == 0)) || + (method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL)) { nm = mono_marshal_get_native_wrapper (method, FALSE, FALSE); + if (context->has_resume_state) + return; } else { const char *name = method->name; if (m_class_get_parent (method->klass) == mono_defaults.multicastdelegate_class) {