From 5fd027c9ef11c4b053259db43c09a3c91abef769 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Wed, 16 Jun 2021 05:52:37 -0400 Subject: [PATCH] [mono][wasm] Fix the usage of function pointers in mixed mode. (#54098) * [mono][wasm] Fix the usage of function pointers in mixed mode. The llvm compiled code expects function pointers to be a MonoFtnDesc*, while the interpreter expects them to be a InterpMethod*. Use a MonoFtnDesc in both cases. * Reenable some tests. * Add caching. --- .../tests/TextEncoderSettingsTests.cs | 1 - src/mono/mono/metadata/icall.c | 2 +- src/mono/mono/metadata/object-internals.h | 2 + src/mono/mono/mini/interp/interp-internals.h | 1 + src/mono/mono/mini/interp/interp.c | 82 +++++++++++++++++++--- src/mono/mono/mini/interp/mintops.def | 1 + src/mono/mono/mini/interp/transform.c | 2 +- src/mono/mono/mini/llvmonly-runtime.c | 3 +- src/mono/mono/mini/method-to-ir.c | 5 +- src/mono/mono/mini/mini-runtime.c | 17 +++++ src/mono/mono/mini/mini.h | 3 + 11 files changed, 103 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Text.Encodings.Web/tests/TextEncoderSettingsTests.cs b/src/libraries/System.Text.Encodings.Web/tests/TextEncoderSettingsTests.cs index e8d39bc..b7945bf 100644 --- a/src/libraries/System.Text.Encodings.Web/tests/TextEncoderSettingsTests.cs +++ b/src/libraries/System.Text.Encodings.Web/tests/TextEncoderSettingsTests.cs @@ -31,7 +31,6 @@ namespace System.Text.Encodings.Web.Tests } } - [ActiveIssue("https://github.com/dotnet/runtime/issues/50965", TestPlatforms.Browser)] public class TextEncoderSettingsTests { [Fact] diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index f1d0d7c..b340e0b 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -6669,7 +6669,7 @@ ves_icall_System_Environment_get_TickCount64 (void) gpointer ves_icall_RuntimeMethodHandle_GetFunctionPointer (MonoMethod *method, MonoError *error) { - return mono_compile_method_checked (method, error); + return mono_get_runtime_callbacks ()->get_ftnptr (method, error); } MonoBoolean diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 025f250..0af9ed9 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -641,6 +641,8 @@ typedef struct { void (*metadata_update_published) (MonoAssemblyLoadContext *alc, uint32_t generation); void (*get_jit_stats)(gint64 *methods_compiled, gint64 *cil_code_size_bytes, gint64 *native_code_size_bytes); void (*get_exception_stats)(guint32 *exception_count); + // Same as compile_method, but returns a MonoFtnDesc in llvmonly mode + gpointer (*get_ftnptr)(MonoMethod *method, MonoError *error); } MonoRuntimeCallbacks; typedef gboolean (*MonoInternalStackWalk) (MonoStackFrameInfo *frame, MonoContext *ctx, gpointer data); diff --git a/src/mono/mono/mini/interp/interp-internals.h b/src/mono/mono/mini/interp/interp-internals.h index 2afac6e..465fd20 100644 --- a/src/mono/mono/mini/interp/interp-internals.h +++ b/src/mono/mono/mini/interp/interp-internals.h @@ -124,6 +124,7 @@ struct InterpMethod { MonoType *rtype; MonoType **param_types; MonoJitInfo *jinfo; + MonoFtnDesc *ftndesc; guint32 locals_size; guint32 alloca_size; diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index da9bed4..1c7468b 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -1620,6 +1620,9 @@ interp_init_delegate (MonoDelegate *del, MonoError *error) } if (del->method_ptr && !del->method) { /* Delegate created from methodInfo.MethodHandle.GetFunctionPointer() */ del->interp_method = (InterpMethod *)del->method_ptr; + if (mono_llvm_only) + // FIXME: + g_assert_not_reached (); } else if (del->method) { /* Delegate created dynamically */ del->interp_method = mono_interp_get_imethod (del->method, error); @@ -1659,13 +1662,57 @@ interp_init_delegate (MonoDelegate *del, MonoError *error) } } +/* Convert a function pointer for a managed method to an InterpMethod* */ +static InterpMethod* +ftnptr_to_imethod (gpointer addr) +{ + InterpMethod *imethod; + + if (mono_llvm_only) { + ERROR_DECL (error); + /* Function pointers are represented by a MonoFtnDesc structure */ + MonoFtnDesc *ftndesc = (MonoFtnDesc*)addr; + g_assert (ftndesc); + g_assert (ftndesc->method); + + imethod = ftndesc->interp_method; + if (!imethod) { + imethod = mono_interp_get_imethod (ftndesc->method, error); + mono_error_assert_ok (error); + mono_memory_barrier (); + ftndesc->interp_method = imethod; + } + } else { + /* Function pointers are represented by their InterpMethod */ + imethod = (InterpMethod*)addr; + } + return imethod; +} + +static gpointer +imethod_to_ftnptr (InterpMethod *imethod) +{ + if (mono_llvm_only) { + ERROR_DECL (error); + /* Function pointers are represented by a MonoFtnDesc structure */ + MonoFtnDesc *ftndesc = imethod->ftndesc; + if (!ftndesc) { + ftndesc = mini_llvmonly_load_method_ftndesc (imethod->method, FALSE, FALSE, error); + mono_error_assert_ok (error); + mono_memory_barrier (); + imethod->ftndesc = ftndesc; + } + return ftndesc; + } else { + return imethod; + } +} + static void interp_delegate_ctor (MonoObjectHandle this_obj, MonoObjectHandle target, gpointer addr, MonoError *error) { - /* - * addr is the result of an LDFTN opcode, i.e. an InterpMethod - */ - InterpMethod *imethod = (InterpMethod*)addr; + /* addr is the result of an LDFTN opcode */ + InterpMethod *imethod = ftnptr_to_imethod (addr); if (!(imethod->method->flags & METHOD_ATTRIBUTE_STATIC)) { MonoMethod *invoke = mono_get_delegate_invoke_internal (mono_handle_class (this_obj)); @@ -3420,7 +3467,9 @@ main_loop: csignature = (MonoMethodSignature*)frame->imethod->data_items [ip [4]]; - cmethod = LOCAL_VAR (ip [2], InterpMethod*); + /* In mixed mode, stay in the interpreter for simplicity even if there is an AOT version of the callee */ + cmethod = ftnptr_to_imethod (LOCAL_VAR (ip [2], gpointer)); + if (cmethod->method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) { cmethod = mono_interp_get_imethod (mono_marshal_get_native_wrapper (cmethod->method, FALSE, FALSE), error); mono_interp_error_cleanup (error); /* FIXME: don't swallow the error */ @@ -6455,25 +6504,36 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; #undef RELOP_FP #undef RELOP_CAST - MINT_IN_CASE(MINT_LDFTN) { + MINT_IN_CASE(MINT_LDFTN_ADDR) { LOCAL_VAR (ip [1], gpointer) = frame->imethod->data_items [ip [2]]; ip += 3; MINT_IN_BREAK; } + MINT_IN_CASE(MINT_LDFTN) { + InterpMethod *m = (InterpMethod*)frame->imethod->data_items [ip [2]]; + + LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (m); + ip += 3; + MINT_IN_BREAK; + } MINT_IN_CASE(MINT_LDVIRTFTN) { InterpMethod *m = (InterpMethod*)frame->imethod->data_items [ip [3]]; MonoObject *o = LOCAL_VAR (ip [2], MonoObject*); NULL_CHECK (o); - - LOCAL_VAR (ip [1], gpointer) = get_virtual_method (m, o->vtable); + + m = get_virtual_method (m, o->vtable); + LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (m); ip += 4; MINT_IN_BREAK; } MINT_IN_CASE(MINT_LDFTN_DYNAMIC) { error_init_reuse (error); - InterpMethod *m = mono_interp_get_imethod (LOCAL_VAR (ip [2], MonoMethod*), error); + + MonoMethod *cmethod = LOCAL_VAR (ip [2], MonoMethod*); + + InterpMethod *m = mono_interp_get_imethod (cmethod, error); mono_error_assert_ok (error); - LOCAL_VAR (ip [1], gpointer) = m; + LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (m); ip += 3; MINT_IN_BREAK; } @@ -6655,7 +6715,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; mono_error_assert_ok (error); } g_assert (del->interp_method); - LOCAL_VAR (ip [1], gpointer) = del->interp_method; + LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (del->interp_method); ip += 3; MINT_IN_BREAK; } diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index 1f55c92..0de8708 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -356,6 +356,7 @@ OPDEF(MINT_BOX_NULLABLE_PTR, "box.nullable.ptr", 4, 1, 1, MintOpShortInt) OPDEF(MINT_UNBOX, "unbox", 4, 1, 1, MintOpClassToken) OPDEF(MINT_LDTOKEN, "ldtoken", 3, 1, 0, MintOpShortInt) OPDEF(MINT_LDFTN, "ldftn", 3, 1, 0, MintOpMethodToken) +OPDEF(MINT_LDFTN_ADDR, "ldftn_addr", 3, 1, 0, MintOpMethodToken) OPDEF(MINT_LDFTN_DYNAMIC, "ldftn.dynamic", 3, 1, 1, MintOpMethodToken) OPDEF(MINT_LDVIRTFTN, "ldvirtftn", 4, 1, 1, MintOpMethodToken) OPDEF(MINT_CPOBJ, "cpobj", 4, 0, 2, MintOpClassToken) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 2736b64..29d1dec 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -6859,7 +6859,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, td->ip += 5; const gconstpointer func = mono_find_jit_icall_info ((MonoJitICallId)token)->func; - interp_add_ins (td, MINT_LDFTN); + interp_add_ins (td, MINT_LDFTN_ADDR); push_simple_type (td, STACK_TYPE_I); interp_ins_set_dreg (td->last_ins, td->sp [-1].local); td->last_ins->data [0] = get_data_item_index (td, (gpointer)func); diff --git a/src/mono/mono/mini/llvmonly-runtime.c b/src/mono/mono/mini/llvmonly-runtime.c index 9dd899b..d457dad 100644 --- a/src/mono/mono/mini/llvmonly-runtime.c +++ b/src/mono/mono/mini/llvmonly-runtime.c @@ -115,9 +115,10 @@ mini_llvmonly_get_delegate_arg (MonoMethod *method, gpointer method_ptr) MonoFtnDesc* mini_llvmonly_create_ftndesc (MonoMethod *m, gpointer addr, gpointer arg) { - MonoFtnDesc *ftndesc = (MonoFtnDesc*)m_method_alloc0 (m, 2 * sizeof (gpointer)); + MonoFtnDesc *ftndesc = (MonoFtnDesc*)m_method_alloc0 (m, sizeof (MonoFtnDesc)); ftndesc->addr = addr; ftndesc->arg = arg; + ftndesc->method = m; return ftndesc; } diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index dc22f3e..02ca3ba 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7155,7 +7155,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b goto calli_end; } } - ins = (MonoInst*)mini_emit_calli_full (cfg, fsig, sp, addr, NULL, NULL, tailcall); + if (cfg->llvm_only && !(cfg->method->wrapper_type && cfg->method->wrapper_type != MONO_WRAPPER_DYNAMIC_METHOD)) + ins = mini_emit_llvmonly_calli (cfg, fsig, sp, addr); + else + ins = (MonoInst*)mini_emit_calli_full (cfg, fsig, sp, addr, NULL, NULL, tailcall); goto calli_end; } case MONO_CEE_CALL: diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 2fd03d8..09944b3 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -2687,6 +2687,22 @@ mono_jit_compile_method_jit_only (MonoMethod *method, MonoError *error) return code; } +/* + * get_ftnptr_for_method: + * + * Return a function pointer for METHOD which is indirectly callable from managed code. + * On llvmonly, this returns a MonoFtnDesc, otherwise it returns a normal function pointer. + */ +static gpointer +get_ftnptr_for_method (MonoMethod *method, MonoError *error) +{ + if (!mono_llvm_only) { + return mono_jit_compile_method (method, error); + } else { + return mini_llvmonly_load_method_ftndesc (method, FALSE, FALSE, error); + } +} + #ifdef MONO_ARCH_HAVE_INVALIDATE_METHOD static void invalidated_delegate_trampoline (char *desc) @@ -4326,6 +4342,7 @@ mini_init (const char *filename, const char *runtime_version) callbacks.create_jit_trampoline = mono_create_jit_trampoline; callbacks.create_delegate_trampoline = mono_create_delegate_trampoline; callbacks.free_method = mono_jit_free_method; + callbacks.get_ftnptr = get_ftnptr_for_method; #endif callbacks.is_interpreter_enabled = mini_is_interpreter_enabled; callbacks.get_weak_field_indexes = mono_aot_get_weak_field_indexes; diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index ae8306b..f953cd2 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -1136,6 +1136,9 @@ typedef struct { gpointer addr; gpointer arg; + MonoMethod *method; + /* InterpMethod* */ + gpointer interp_method; } MonoFtnDesc; typedef enum { -- 2.7.4