From 7cc97a8b5350660e909cfb4d2ad06f90c300e2d8 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 25 Nov 2024 20:12:00 +0200 Subject: [PATCH 1/3] [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. --- src/mono/mono/mini/interp/transform.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index d9efd0ddd29d82..89718c3110832c 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2692,8 +2692,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); @@ -3733,9 +3731,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); @@ -9770,7 +9778,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)); @@ -9778,7 +9787,8 @@ 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); } else { const char *name = method->name; From 60e9d62f6485d2dd7e7ea09cd3c59d7f0376cbec Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Wed, 27 Nov 2024 20:40:57 +0200 Subject: [PATCH 2/3] [mono][interp] Check resume state when returning from method compilation --- src/mono/mono/mini/interp/interp.c | 2 ++ src/mono/mono/mini/interp/transform.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 1dda6c21648917..b4fda976f5bf12 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -3916,6 +3916,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 +4373,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 89718c3110832c..7147e5e30b64de 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -9790,6 +9790,8 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon 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) { From e21ae98e1cad3e3a98d67b17ce8e8412d1753281 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 28 Nov 2024 18:23:01 +0200 Subject: [PATCH 3/3] [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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index b4fda976f5bf12..df0abac566ed6e 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