From 81285fb3620586c1cb495b3f191aafc6106b22cd Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 25 Apr 2024 16:30:53 -0400 Subject: [PATCH 01/30] WIP HACK: always AOT wrappers, even for generics, not the actual accessor --- src/mono/mono/mini/aot-compiler.c | 111 ++++++++++++++++++- src/mono/mono/mini/mini.c | 16 ++- src/mono/sample/HelloWorld/HelloWorld.csproj | 3 + src/mono/sample/HelloWorld/Makefile | 7 +- src/mono/sample/HelloWorld/Program.cs | 33 ++++-- 5 files changed, 155 insertions(+), 15 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 9aa0760231dd1..6dda6fac6e72c 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -4316,6 +4316,14 @@ add_method_with_index (MonoAotCompile *acfg, MonoMethod *method, int index, gboo { g_assert (method); if (!g_hash_table_lookup (acfg->method_indexes, method)) { + { + char * method_name = mono_method_get_full_name (method); + g_print ("inserting %s into acfg\n", method_name); + if (strstr(method_name, "W_REF& Program:AccessBoxmethods, method); g_hash_table_insert (acfg->method_indexes, method, GUINT_TO_POINTER (index + 1)); acfg->nmethods = acfg->methods->len + 1; @@ -5312,6 +5320,7 @@ add_full_aot_wrappers (MonoAotCompile *acfg) } } +#if 0 /* unsafe accessor wrappers */ rows = table_info_get_rows (&acfg->image->tables [MONO_TABLE_METHOD]); for (int i = 0; i < rows; ++i) { @@ -5325,10 +5334,11 @@ add_full_aot_wrappers (MonoAotCompile *acfg) char *member_name = NULL; int accessor_kind = -1; - if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + if (!mono_aot_mode_is_full (&acfg->aot_opts) && mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { add_extra_method (acfg, mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name)); } } +#endif } @@ -6024,6 +6034,33 @@ add_generic_instances (MonoAotCompile *acfg) continue; } + if (mono_aot_mode_is_full (&acfg->aot_opts) && !mono_method_metadata_has_header (method)) { + { + char * method_name = mono_method_get_full_name (method); + g_print ("ADDING %s generic instances\n", method_name); + g_free (method_name); + } + + + // if the method has no body, see if it's an [UnsafeAccessor] method and replace it by the wrapper. + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + method = wrapper; + { + char * method_name = mono_method_get_full_name (method); + g_print ("REPLACED by %s in generic instances\n", method_name); + g_free (method_name); + } + + } else if (!is_ok (error)) { + aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + mono_error_cleanup (error); + continue; + } + } + if (m_class_get_image (method->klass) != acfg->image) continue; @@ -9355,6 +9392,36 @@ add_referenced_patch (MonoAotCompile *acfg, MonoJumpInfo *patch_info, int depth) if (mono_aot_mode_is_full (&acfg->aot_opts) && !method_has_type_vars (m)) add_extra_method_with_depth (acfg, mono_marshal_get_native_wrapper (m, TRUE, TRUE), depth + 1); } else { + if (mono_aot_mode_is_full (&acfg->aot_opts) && !mono_method_metadata_has_header (m)) { + ERROR_DECL(error); + MonoMethod *method = m; + { + char * method_name = mono_method_get_full_name (method); + g_print ("ADDING %s from referenced patch\n", method_name); + g_free (method_name); + } + + + // if the method has no body, see if it's an [UnsafeAccessor] method and replace it by the wrapper. + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + method = wrapper; + { + char * method_name = mono_method_get_full_name (method); + g_print ("REPLACED by %s in referenced patch\n", method_name); + g_free (method_name); + } + + } else if (!is_ok (error)) { + aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + mono_error_cleanup (error); + return; + } + m = method; + } + add_extra_method_with_depth (acfg, m, depth + 1); add_types_from_method_header (acfg, m); } @@ -9460,7 +9527,7 @@ compile_method (MonoAotCompile *acfg, MonoMethod *method) if ((method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) || (method->iflags & METHOD_IMPL_ATTRIBUTE_RUNTIME) || (method->flags & METHOD_ATTRIBUTE_ABSTRACT)) { - //printf ("Skip (impossible): %s\n", mono_method_full_name (method, TRUE)); + printf ("Skip (impossible): %s\n", mono_method_full_name (method, TRUE)); return; } @@ -13003,6 +13070,32 @@ collect_methods (MonoAotCompile *acfg) method = wrapper; } + if (mono_aot_mode_is_full (&acfg->aot_opts) && !mono_method_metadata_has_header (method)) { + // if the method has no body, see if it's an [UnsafeAccessor] method and replace it by the wrapper. + { + char * method_name = mono_method_get_full_name (method); + g_print ("ADDING %s toplevel collect\n", method_name); + g_free (method_name); + } + + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + method = wrapper; + { + char * method_name = mono_method_get_full_name (method); + g_print ("REPLACING %s toplevel collect\n", method_name); + g_free (method_name); + } + + } else if (!is_ok (error)) { + aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + mono_error_cleanup (error); + return FALSE; + } + } + /* FIXME: Some mscorlib methods don't have debug info */ /* if (acfg->aot_opts.soft_debug && !method->wrapper_type) { @@ -13047,6 +13140,20 @@ collect_methods (MonoAotCompile *acfg) method = mono_get_method_checked (acfg->image, token, NULL, NULL, error); report_loader_error (acfg, error, TRUE, "Failed to load method token 0x%x due to %s\n", i, mono_error_get_message (error)); + if (mono_aot_mode_is_full (&acfg->aot_opts) && !mono_method_metadata_has_header (method)) { + // if the method has no body, see if it's an [UnsafeAccessor] method and replace it by the wrapper. + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + method = wrapper; + } else if (!is_ok (error)) { + aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + mono_error_cleanup (error); + return FALSE; + } + } + if ((method->is_generic || mono_class_is_gtd (method->klass)) && should_emit_gsharedvt_method (acfg, method)) { MonoMethod *gshared; diff --git a/src/mono/mono/mini/mini.c b/src/mono/mono/mini/mini.c index b467c2d446620..7e53d8f583200 100644 --- a/src/mono/mono/mini/mini.c +++ b/src/mono/mono/mini/mini.c @@ -3196,7 +3196,9 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts cfg->jit_mm = jit_mm_for_method (cfg->method); cfg->mem_manager = m_method_get_mem_manager (cfg->method); - if (cfg->method->wrapper_type == MONO_WRAPPER_ALLOC || cfg->method->wrapper_type == MONO_WRAPPER_NATIVE_TO_MANAGED) { + char *method_name = mono_method_get_full_name (cfg->method); + + if (cfg->method->wrapper_type == MONO_WRAPPER_ALLOC || cfg->method->wrapper_type == MONO_WRAPPER_NATIVE_TO_MANAGED) { /* We can't have seq points inside gc critical regions or native-to-managed wrapper */ cfg->gen_seq_points = FALSE; cfg->gen_sdb_seq_points = FALSE; @@ -3308,14 +3310,26 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts return cfg; } + { + g_print ("START converting %s%s%s%smethod %s\n", COMPILE_LLVM (cfg) ? "llvm " : "", cfg->gsharedvt ? "gsharedvt " : "", (cfg->gshared && !cfg->gsharedvt) ? "gshared " : "", cfg->interp_entry_only ? "interp only " : "", method_name); + if (strstr(method_name, "W& Program:AccessBox") != NULL) { + printf ("blaps\n"); + } + } + header = cfg->header = mono_method_get_header_checked (cfg->method, cfg->error); if (!header) { mono_cfg_set_exception (cfg, MONO_EXCEPTION_MONO_ERROR); if (MONO_METHOD_COMPILE_END_ENABLED ()) MONO_PROBE_METHOD_COMPILE_END (method, FALSE); + g_print ("NO HEADER, returning, %s\n", method_name); + if (strstr(method_name, "AccessBox") != 0) { + printf ("oops\n"); + } return cfg; } + if (cfg->llvm_only && cfg->interp && !cfg->interp_entry_only && header->num_clauses) { gboolean can_deopt = TRUE; /* diff --git a/src/mono/sample/HelloWorld/HelloWorld.csproj b/src/mono/sample/HelloWorld/HelloWorld.csproj index cc052494b398f..07bf3acd09ee7 100644 --- a/src/mono/sample/HelloWorld/HelloWorld.csproj +++ b/src/mono/sample/HelloWorld/HelloWorld.csproj @@ -1,9 +1,11 @@ + Exe $(NetCoreAppCurrent) true + true diff --git a/src/mono/sample/HelloWorld/Makefile b/src/mono/sample/HelloWorld/Makefile index afdfa16f3512c..3a40630813a65 100644 --- a/src/mono/sample/HelloWorld/Makefile +++ b/src/mono/sample/HelloWorld/Makefile @@ -5,14 +5,17 @@ DOTNET_Q_ARGS=--nologo -v:q -consoleloggerparameters:NoSummary MONO_CONFIG?=Debug MONO_ARCH?=$(shell . $(TOP)eng/common/native/init-os-and-arch.sh && echo $${arch}) TARGET_OS?=$(shell . $(TOP)eng/common/native/init-os-and-arch.sh && echo $${os}) -AOT?=false +AOT?=true USE_LLVM?=false StripILCode?=false TrimmingEligibleMethodsOutputDirectory?= # #MIBC_PROFILE_PATH= -MONO_ENV_OPTIONS ?= +MONO_ENV_OPTIONS ?= +ifeq ($(AOT),true) +MONO_ENV_OPTIONS += --full-aot +endif publish: $(DOTNET) publish \ diff --git a/src/mono/sample/HelloWorld/Program.cs b/src/mono/sample/HelloWorld/Program.cs index 0a65da4203a6d..9f4dea608e8ba 100644 --- a/src/mono/sample/HelloWorld/Program.cs +++ b/src/mono/sample/HelloWorld/Program.cs @@ -2,18 +2,31 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Runtime.CompilerServices; -namespace HelloWorld +public class MyBox { - internal class Program + private T _value; + public T Value { get => _value; private set { _value = value; }} + public MyBox(T inp) { Value = inp; } +} + +public class Program +{ + public static void Main() { - private static void Main(string[] args) - { - bool isMono = typeof(object).Assembly.GetType("Mono.RuntimeStructs") != null; - Console.WriteLine($"Hello World {(isMono ? "from Mono!" : "from CoreCLR!")}"); - Console.WriteLine(typeof(object).Assembly.FullName); - Console.WriteLine(System.Reflection.Assembly.GetEntryAssembly ()); - Console.WriteLine(System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription); - } + var box = new MyBox("xyz"); + RunIt(box, "abc"); + Console.WriteLine (box.Value); } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void RunIt (MyBox dest, H input) + { + ref H boxWriter = ref AccessBox(dest); + boxWriter = input; + } + + [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] + private static extern ref W AccessBox(MyBox x); } From 3d598332b1a9854c772a2cf79373415f8b077803 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 29 Apr 2024 12:34:48 -0400 Subject: [PATCH 02/30] cleanup --- src/mono/mono/mini/aot-compiler.c | 150 +++++++++++++----------------- 1 file changed, 63 insertions(+), 87 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 6dda6fac6e72c..731494855a30c 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -4829,6 +4829,52 @@ mono_aot_can_enter_interp (MonoMethod *method) return FALSE; } +/** + * Replaces some extern \c method by a wrapper. + * + * Unsafe accessor methods are static extern methods with no header. Calls to + * them are replaced by calls to a wrapper. So during AOT compilation when we + * collect methods to AOT, we replace these methods by the wrappers, too. + * + * Returns the wrapper method, or \c NULL if it doesn't need to be replaced. + * On error returns NULL and sets \c error. + */ +static MonoMethod* +replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *error) +{ + if (G_LIKELY (mono_method_metadata_has_header (method))) + return NULL; + + { + char * method_name = mono_method_get_full_name (method); + g_print ("ADDING no header %s generic instances\n", method_name); + g_free (method_name); + } + + /* Unsafe accessors methods. Replace attempts to compile the accessor method by + * its wrapper. + */ + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + // FIXME: if `method` was inflated, get the uninflated wrapper and then re-inflate + // it. + MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + { + char * method_name = mono_method_get_full_name (wrapper); + g_print ("REPLACED by %s in generic instances\n", method_name); + g_free (method_name); + } + return wrapper; + } + + if (!is_ok (error)) { + aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + } + + return NULL; +} + static void add_full_aot_wrappers (MonoAotCompile *acfg) { @@ -5319,27 +5365,6 @@ add_full_aot_wrappers (MonoAotCompile *acfg) add_method (acfg, mono_marshal_get_ptr_to_struct (klass)); } } - -#if 0 - /* unsafe accessor wrappers */ - rows = table_info_get_rows (&acfg->image->tables [MONO_TABLE_METHOD]); - for (int i = 0; i < rows; ++i) { - ERROR_DECL (error); - token = MONO_TOKEN_METHOD_DEF | (i + 1); - method = mono_get_method_checked (acfg->image, token, NULL, NULL, error); - report_loader_error (acfg, error, TRUE, "Failed to load method token 0x%x due to %s\n", i, mono_error_get_message (error)); - - if (G_LIKELY (mono_method_metadata_has_header (method))) - continue; - - char *member_name = NULL; - int accessor_kind = -1; - if (!mono_aot_mode_is_full (&acfg->aot_opts) && mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - add_extra_method (acfg, mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name)); - } - } -#endif - } static void @@ -6034,30 +6059,13 @@ add_generic_instances (MonoAotCompile *acfg) continue; } - if (mono_aot_mode_is_full (&acfg->aot_opts) && !mono_method_metadata_has_header (method)) { - { - char * method_name = mono_method_get_full_name (method); - g_print ("ADDING %s generic instances\n", method_name); - g_free (method_name); - } - - - // if the method has no body, see if it's an [UnsafeAccessor] method and replace it by the wrapper. - char *member_name = NULL; - int accessor_kind = -1; - if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); - method = wrapper; - { - char * method_name = mono_method_get_full_name (method); - g_print ("REPLACED by %s in generic instances\n", method_name); - g_free (method_name); - } - - } else if (!is_ok (error)) { - aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + if (mono_aot_mode_is_full (&acfg->aot_opts)) { + MonoMethod *gen = replace_generated_method (acfg, method, error); + if (!is_ok (error)) { mono_error_cleanup (error); continue; + } else if (gen != NULL) { + method = gen; } } @@ -6156,6 +6164,8 @@ add_generic_instances (MonoAotCompile *acfg) add_extra_method (acfg, method); } + // TODO: [UnsafeAccessor] if there's a typespec from another assembly that has an + // unsafe accessor, we need to add it here. rows = table_info_get_rows (&acfg->image->tables [MONO_TABLE_TYPESPEC]); for (int i = 0; i < rows; ++i) { ERROR_DECL (error); @@ -9392,34 +9402,15 @@ add_referenced_patch (MonoAotCompile *acfg, MonoJumpInfo *patch_info, int depth) if (mono_aot_mode_is_full (&acfg->aot_opts) && !method_has_type_vars (m)) add_extra_method_with_depth (acfg, mono_marshal_get_native_wrapper (m, TRUE, TRUE), depth + 1); } else { - if (mono_aot_mode_is_full (&acfg->aot_opts) && !mono_method_metadata_has_header (m)) { + if (mono_aot_mode_is_full (&acfg->aot_opts)) { ERROR_DECL(error); - MonoMethod *method = m; - { - char * method_name = mono_method_get_full_name (method); - g_print ("ADDING %s from referenced patch\n", method_name); - g_free (method_name); - } - - - // if the method has no body, see if it's an [UnsafeAccessor] method and replace it by the wrapper. - char *member_name = NULL; - int accessor_kind = -1; - if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); - method = wrapper; - { - char * method_name = mono_method_get_full_name (method); - g_print ("REPLACED by %s in referenced patch\n", method_name); - g_free (method_name); - } - - } else if (!is_ok (error)) { - aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + MonoMethod *gen = replace_generated_method (acfg, m, error); + if (!is_ok (error)) { mono_error_cleanup (error); return; + } else if (gen != NULL) { + m = gen; } - m = method; } add_extra_method_with_depth (acfg, m, depth + 1); @@ -13070,29 +13061,14 @@ collect_methods (MonoAotCompile *acfg) method = wrapper; } - if (mono_aot_mode_is_full (&acfg->aot_opts) && !mono_method_metadata_has_header (method)) { + if (mono_aot_mode_is_full (&acfg->aot_opts)) { // if the method has no body, see if it's an [UnsafeAccessor] method and replace it by the wrapper. - { - char * method_name = mono_method_get_full_name (method); - g_print ("ADDING %s toplevel collect\n", method_name); - g_free (method_name); - } - - char *member_name = NULL; - int accessor_kind = -1; - if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); - method = wrapper; - { - char * method_name = mono_method_get_full_name (method); - g_print ("REPLACING %s toplevel collect\n", method_name); - g_free (method_name); - } - - } else if (!is_ok (error)) { - aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + MonoMethod *gen = replace_generated_method (acfg, method, error); + if (!is_ok (error)) { mono_error_cleanup (error); return FALSE; + } else if (gen != NULL) { + method = gen; } } From fbd3b89a7c258d6bc1cf02dfb2c618147eaea024 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 30 Apr 2024 16:05:41 -0400 Subject: [PATCH 03/30] checkpoint generic wrapper methods --- src/mono/mono/metadata/marshal.c | 57 +++++++++++++++++-- src/mono/mono/metadata/method-builder-ilgen.c | 12 ++++ src/mono/mono/mini/aot-compiler.c | 50 +++++++++++++--- src/mono/mono/mini/aot-runtime.c | 20 ++++++- src/mono/mono/mini/aot-runtime.h | 2 +- src/mono/sample/HelloWorld/Program.cs | 21 ++++++- 6 files changed, 147 insertions(+), 15 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index aad740a31d90e..d4ad029187838 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5226,31 +5226,80 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf MonoMethod *res; GHashTable *cache; MonoGenericContext *ctx = NULL; + MonoGenericContainer *container = NULL; MonoMethod *orig_method = NULL; WrapperInfo *info; + gboolean is_generic = FALSE; if (member_name == NULL && kind != MONO_UNSAFE_ACCESSOR_CTOR) member_name = accessor_method->name; + if (accessor_method->is_generic) { + /* got a generic method definition. need to make a generic method definition wrapper */ + g_assert (!accessor_method->is_inflated); + printf ("accessor method is generic: %s\n", mono_method_full_name (accessor_method, TRUE)); + is_generic = TRUE; + } + + /* FIXME: Support generic methods too */ + if (accessor_method->is_inflated && !mono_method_get_context (accessor_method)->method_inst) { + orig_method = accessor_method; + ctx = &((MonoMethodInflated*)accessor_method)->context; + accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; + container = mono_method_get_generic_container (accessor_method); + if (!container) + container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? + g_assert (container); + } else if (accessor_method->is_inflated && mono_method_get_context(accessor_method)->method_inst) { + // FIXME: ak: do we need a different case? it should work, right? + // N.B. both method_inst and type_inst might be set + orig_method = accessor_method; + ctx = &((MonoMethodInflated*)accessor_method)->context; + accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; + container = mono_method_get_generic_container (accessor_method); + if (!container) + container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? + g_assert (container); + } + /* * Check cache */ if (ctx) { - cache = NULL; - g_assert_not_reached (); + /* FIXME get the right cache */ + cache = NULL; /* get_cache (&((MonoMethodInflated*)orig_method)->owner->wrapper_caches.synchronized_cache , mono_aligned_addr_hash, NULL); */ + /* res = check_generic_wrapper_cache (cache, orig_method, orig_method, method);*/ + res = NULL; + if (res) + return res; } else { cache = get_cache (&mono_method_get_wrapper_cache (accessor_method)->unsafe_accessor_cache, mono_aligned_addr_hash, NULL); if ((res = mono_marshal_find_in_cache (cache, accessor_method))) return res; } - sig = mono_metadata_signature_dup_full (get_method_image (accessor_method), mono_method_signature_internal (accessor_method)); - sig->pinvoke = 0; mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); + if (is_generic) { + mb->method->is_generic = is_generic; + container = mono_class_try_get_generic_container (accessor_method->klass); + container = mono_metadata_load_generic_params (m_class_get_image (accessor_method->klass), accessor_method->token, container, /*owner:*/mb->method); + mono_method_set_generic_container (mb->method, container); + + MonoGenericContext ctx = {0,}; + ctx.method_inst = container->context.method_inst; + + ERROR_DECL (error); + sig = mono_inflate_generic_signature (mono_method_signature_internal (accessor_method), &ctx, error); + mono_error_assert_ok (error); // FIXME + } else { + sig = mono_metadata_signature_dup_full (get_method_image (accessor_method), mono_method_signature_internal (accessor_method)); + } + sig->pinvoke = 0; get_marshal_cb ()->mb_skip_visibility (mb); + // TODO: pass container, too get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, ctx, kind, member_name); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_UNSAFE_ACCESSOR); diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index 41bfc8fa6804c..287412110e4e5 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -214,6 +214,18 @@ create_method_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *signature, int mono_image_unlock (image); } + if (mb->method->is_generic) { + method->is_generic = TRUE; + MonoGenericContainer *container = mono_method_get_generic_container (mb->method); + mono_method_set_generic_container (method, container); + g_assert (!container->is_anonymous); + g_assert (container->is_method); + g_assert (container->owner.method == mb->method); + // HACK: reassign container owner from the method builder placeholder to the + // final created method + container->owner.method = method; + } + return method; } diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 731494855a30c..f3473d3fa9732 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -3874,6 +3874,15 @@ encode_method_ref (MonoAotCompile *acfg, MonoMethod *method, guint8 *buf, guint8 encode_value (len, p, &p); encode_string (info->d.unsafe_accessor.member_name, p, &p); } + if (method->is_inflated) { + encode_value(1, p, &p); + MonoMethodInflated *inflated = (MonoMethodInflated*)method; + MonoGenericContext *ctx = &inflated->context; + encode_generic_context (acfg, ctx, p, &p); + } else { + encode_value(0, p, &p); + } + } else if (info->subtype == WRAPPER_SUBTYPE_INTERP_IN) encode_signature (acfg, info->d.interp_in.sig, p, &p); @@ -4829,6 +4838,18 @@ mono_aot_can_enter_interp (MonoMethod *method) return FALSE; } +static MonoMethod* +instantiate_wrapper_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +{ + g_assert (extern_method_inst->is_inflated); + MonoMethodInflated *infl = (MonoMethodInflated*)extern_method_inst; + MonoMethod *extern_decl = infl->declaring; + MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); + MonoGenericContext *ctx = &infl->context; + MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); + return inflated_wrapper; +} + /** * Replaces some extern \c method by a wrapper. * @@ -4848,6 +4869,8 @@ replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *e { char * method_name = mono_method_get_full_name (method); g_print ("ADDING no header %s generic instances\n", method_name); + if (strstr (method_name, "!!0") != NULL) + g_print("anon\n"); g_free (method_name); } @@ -4857,15 +4880,20 @@ replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *e char *member_name = NULL; int accessor_kind = -1; if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - // FIXME: if `method` was inflated, get the uninflated wrapper and then re-inflate - // it. - MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); - { - char * method_name = mono_method_get_full_name (wrapper); - g_print ("REPLACED by %s in generic instances\n", method_name); - g_free (method_name); + MonoMethod *wrapper = NULL; + if (method->is_inflated) { + wrapper = instantiate_wrapper_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); + } else { + wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + } + if (is_ok (error)) { + { + char * method_name = mono_method_get_full_name (wrapper); + g_print ("REPLACED by %s in generic instances\n", method_name); + g_free (method_name); + } + return wrapper; } - return wrapper; } if (!is_ok (error)) { @@ -7925,6 +7953,12 @@ emit_exception_debug_info (MonoAotCompile *acfg, MonoCompile *cfg, gboolean stor */ buf2 = (guint8 *)g_malloc (4096); p2 = buf2; + if (cfg->method->wrapper_type && cfg->method->is_generic) { + printf ("barf\n"); + } + if (jinfo->d.method->wrapper_type && jinfo->d.method->is_generic) { + printf ("barf2\n"); + } encode_method_ref (acfg, jinfo->d.method, p2, &p2); len = GPTRDIFF_TO_INT (p2 - buf2); g_assert (len < 4096); diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index 95c4b8236bc3b..9857c1648fdca 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -940,6 +940,14 @@ typedef struct { gboolean no_aot_trampoline; } MethodRef; +static MonoMethod* +instantiate_wrapper_like_decl (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +{ + MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); + MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); + return inflated_wrapper; +} + /* * decode_method_ref_with_target: * @@ -1079,7 +1087,17 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod uint32_t name_len = decode_value (p, &p); const char *member_name = (const char*)p; p += name_len + 1; - ref->method = mono_marshal_get_unsafe_accessor_wrapper (m, kind, member_name); + uint8_t inflated = decode_value (p, &p); + if (inflated) { + MonoGenericContext ctx = {0,}; + decode_generic_context (module, &ctx, p, &p, error); + mono_error_assert_ok (error); + ref->method = instantiate_wrapper_like_decl (m, &ctx, kind, member_name, error); + if (!is_ok (error)) + return FALSE; + } else { + ref->method = mono_marshal_get_unsafe_accessor_wrapper (m, kind, member_name); + } } else if (subtype == WRAPPER_SUBTYPE_GSHAREDVT_IN) { ref->method = mono_marshal_get_gsharedvt_in_wrapper (); } else if (subtype == WRAPPER_SUBTYPE_GSHAREDVT_OUT) { diff --git a/src/mono/mono/mini/aot-runtime.h b/src/mono/mono/mini/aot-runtime.h index bd66659353627..e4f84523c45e2 100644 --- a/src/mono/mono/mini/aot-runtime.h +++ b/src/mono/mono/mini/aot-runtime.h @@ -11,7 +11,7 @@ #include "mini.h" /* Version number of the AOT file format */ -#define MONO_AOT_FILE_VERSION 185 +#define MONO_AOT_FILE_VERSION 186 #define MONO_AOT_TRAMP_PAGE_SIZE 16384 diff --git a/src/mono/sample/HelloWorld/Program.cs b/src/mono/sample/HelloWorld/Program.cs index 9f4dea608e8ba..d9f391e92ef30 100644 --- a/src/mono/sample/HelloWorld/Program.cs +++ b/src/mono/sample/HelloWorld/Program.cs @@ -16,7 +16,10 @@ public class Program public static void Main() { var box = new MyBox("xyz"); - RunIt(box, "abc"); + //Thinger(box); + //RunIt(box, "abc"); + //Console.WriteLine (box.Value); + RunItAgain(box, "hjk"); Console.WriteLine (box.Value); } @@ -27,6 +30,22 @@ public static void RunIt (MyBox dest, H input) boxWriter = input; } + [MethodImpl(MethodImplOptions.NoInlining)] + public static void RunItAgain (MyBox dest, S input) + { + ref S boxWriter = ref AccessHelper.AccessBox2(dest); + boxWriter = input; + } + [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] private static extern ref W AccessBox(MyBox x); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static ref W Thinger(MyBox x) => throw new InvalidOperationException("oops"); +} + +public class AccessHelper +{ + [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] + public static extern ref Q AccessBox2(MyBox q); } From 34381f7b0c7840cbc7ce1ae55cfa6d0250448043 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 30 Apr 2024 18:55:53 -0400 Subject: [PATCH 04/30] fix msvc build --- src/mono/mono/metadata/marshal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index d4ad029187838..0ec13b2045456 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5286,11 +5286,11 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf container = mono_metadata_load_generic_params (m_class_get_image (accessor_method->klass), accessor_method->token, container, /*owner:*/mb->method); mono_method_set_generic_container (mb->method, container); - MonoGenericContext ctx = {0,}; - ctx.method_inst = container->context.method_inst; + MonoGenericContext inst_ctx = {0,}; + inst_ctx.method_inst = container->context.method_inst; ERROR_DECL (error); - sig = mono_inflate_generic_signature (mono_method_signature_internal (accessor_method), &ctx, error); + sig = mono_inflate_generic_signature (mono_method_signature_internal (accessor_method), &inst_ctx, error); mono_error_assert_ok (error); // FIXME } else { sig = mono_metadata_signature_dup_full (get_method_image (accessor_method), mono_method_signature_internal (accessor_method)); From ecaff9e6f1b4c78ef36755ef4c7acd8466f51669 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 30 Apr 2024 19:53:20 -0400 Subject: [PATCH 05/30] hack --- src/mono/mono/mini/mini.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/mini/mini.c b/src/mono/mono/mini/mini.c index 7e53d8f583200..05121317431d4 100644 --- a/src/mono/mono/mini/mini.c +++ b/src/mono/mono/mini/mini.c @@ -3474,10 +3474,10 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts cfg->intvars = (guint16 *)mono_mempool_alloc0 (cfg->mempool, sizeof (guint16) * STACK_MAX * header->max_stack); if (cfg->verbose_level > 0) { - char *method_name; + char *v_method_name; - method_name = mono_method_get_full_name (method); - g_print ("converting %s%s%s%smethod %s\n", COMPILE_LLVM (cfg) ? "llvm " : "", cfg->gsharedvt ? "gsharedvt " : "", (cfg->gshared && !cfg->gsharedvt) ? "gshared " : "", cfg->interp_entry_only ? "interp only " : "", method_name); + v_method_name = mono_method_get_full_name (method); + g_print ("converting %s%s%s%smethod %s\n", COMPILE_LLVM (cfg) ? "llvm " : "", cfg->gsharedvt ? "gsharedvt " : "", (cfg->gshared && !cfg->gsharedvt) ? "gshared " : "", cfg->interp_entry_only ? "interp only " : "", v_method_name); /* if (COMPILE_LLVM (cfg)) g_print ("converting llvm method %s\n", method_name = mono_method_full_name (method, TRUE)); @@ -3488,7 +3488,7 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts else g_print ("converting method %s\n", method_name = mono_method_full_name (method, TRUE)); */ - g_free (method_name); + g_free (v_method_name); } if (cfg->opt & MONO_OPT_ABCREM) From 06be3b41325d03ffe40aedcf53dc66248a4b9544 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 30 Apr 2024 20:17:11 -0400 Subject: [PATCH 06/30] fix build --- src/mono/mono/mini/aot-runtime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index 9857c1648fdca..b3f4e0bfc8c61 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -1087,7 +1087,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod uint32_t name_len = decode_value (p, &p); const char *member_name = (const char*)p; p += name_len + 1; - uint8_t inflated = decode_value (p, &p); + int32_t inflated = decode_value (p, &p); if (inflated) { MonoGenericContext ctx = {0,}; decode_generic_context (module, &ctx, p, &p, error); From 430f822a39f9fa5a005b49e15ca3ed547770e6fe Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 1 May 2024 10:54:34 -0400 Subject: [PATCH 07/30] clean WIP printfs --- src/mono/mono/metadata/marshal.c | 1 - src/mono/mono/mini/aot-compiler.c | 32 ++++++------------------------- src/mono/mono/mini/mini.c | 24 +++++------------------ 3 files changed, 11 insertions(+), 46 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 0ec13b2045456..02b7754b0e8d6 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5237,7 +5237,6 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf if (accessor_method->is_generic) { /* got a generic method definition. need to make a generic method definition wrapper */ g_assert (!accessor_method->is_inflated); - printf ("accessor method is generic: %s\n", mono_method_full_name (accessor_method, TRUE)); is_generic = TRUE; } diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index f3473d3fa9732..229c9ea17f7b2 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -4325,14 +4325,6 @@ add_method_with_index (MonoAotCompile *acfg, MonoMethod *method, int index, gboo { g_assert (method); if (!g_hash_table_lookup (acfg->method_indexes, method)) { - { - char * method_name = mono_method_get_full_name (method); - g_print ("inserting %s into acfg\n", method_name); - if (strstr(method_name, "W_REF& Program:AccessBoxmethods, method); g_hash_table_insert (acfg->method_indexes, method, GUINT_TO_POINTER (index + 1)); acfg->nmethods = acfg->methods->len + 1; @@ -4866,14 +4858,6 @@ replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *e if (G_LIKELY (mono_method_metadata_has_header (method))) return NULL; - { - char * method_name = mono_method_get_full_name (method); - g_print ("ADDING no header %s generic instances\n", method_name); - if (strstr (method_name, "!!0") != NULL) - g_print("anon\n"); - g_free (method_name); - } - /* Unsafe accessors methods. Replace attempts to compile the accessor method by * its wrapper. */ @@ -4887,9 +4871,9 @@ replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *e wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); } if (is_ok (error)) { - { + if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_AOT)) { char * method_name = mono_method_get_full_name (wrapper); - g_print ("REPLACED by %s in generic instances\n", method_name); + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_AOT, "Replacing generated method by %s", method_name); g_free (method_name); } return wrapper; @@ -4897,7 +4881,9 @@ replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *e } if (!is_ok (error)) { - aot_printerrf (acfg, "Could not get unsafe accessor wrapper due to %s ", mono_error_get_message (error)); + char *method_name = mono_method_get_full_name (method); + aot_printerrf (acfg, "Could not get generated wrapper for %s due to %s", method_name, mono_error_get_message (error)); + g_free (method_name); } return NULL; @@ -7953,12 +7939,6 @@ emit_exception_debug_info (MonoAotCompile *acfg, MonoCompile *cfg, gboolean stor */ buf2 = (guint8 *)g_malloc (4096); p2 = buf2; - if (cfg->method->wrapper_type && cfg->method->is_generic) { - printf ("barf\n"); - } - if (jinfo->d.method->wrapper_type && jinfo->d.method->is_generic) { - printf ("barf2\n"); - } encode_method_ref (acfg, jinfo->d.method, p2, &p2); len = GPTRDIFF_TO_INT (p2 - buf2); g_assert (len < 4096); @@ -9552,7 +9532,7 @@ compile_method (MonoAotCompile *acfg, MonoMethod *method) if ((method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) || (method->iflags & METHOD_IMPL_ATTRIBUTE_RUNTIME) || (method->flags & METHOD_ATTRIBUTE_ABSTRACT)) { - printf ("Skip (impossible): %s\n", mono_method_full_name (method, TRUE)); + //printf ("Skip (impossible): %s\n", mono_method_full_name (method, TRUE)); return; } diff --git a/src/mono/mono/mini/mini.c b/src/mono/mono/mini/mini.c index 05121317431d4..b467c2d446620 100644 --- a/src/mono/mono/mini/mini.c +++ b/src/mono/mono/mini/mini.c @@ -3196,9 +3196,7 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts cfg->jit_mm = jit_mm_for_method (cfg->method); cfg->mem_manager = m_method_get_mem_manager (cfg->method); - char *method_name = mono_method_get_full_name (cfg->method); - - if (cfg->method->wrapper_type == MONO_WRAPPER_ALLOC || cfg->method->wrapper_type == MONO_WRAPPER_NATIVE_TO_MANAGED) { + if (cfg->method->wrapper_type == MONO_WRAPPER_ALLOC || cfg->method->wrapper_type == MONO_WRAPPER_NATIVE_TO_MANAGED) { /* We can't have seq points inside gc critical regions or native-to-managed wrapper */ cfg->gen_seq_points = FALSE; cfg->gen_sdb_seq_points = FALSE; @@ -3310,26 +3308,14 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts return cfg; } - { - g_print ("START converting %s%s%s%smethod %s\n", COMPILE_LLVM (cfg) ? "llvm " : "", cfg->gsharedvt ? "gsharedvt " : "", (cfg->gshared && !cfg->gsharedvt) ? "gshared " : "", cfg->interp_entry_only ? "interp only " : "", method_name); - if (strstr(method_name, "W& Program:AccessBox") != NULL) { - printf ("blaps\n"); - } - } - header = cfg->header = mono_method_get_header_checked (cfg->method, cfg->error); if (!header) { mono_cfg_set_exception (cfg, MONO_EXCEPTION_MONO_ERROR); if (MONO_METHOD_COMPILE_END_ENABLED ()) MONO_PROBE_METHOD_COMPILE_END (method, FALSE); - g_print ("NO HEADER, returning, %s\n", method_name); - if (strstr(method_name, "AccessBox") != 0) { - printf ("oops\n"); - } return cfg; } - if (cfg->llvm_only && cfg->interp && !cfg->interp_entry_only && header->num_clauses) { gboolean can_deopt = TRUE; /* @@ -3474,10 +3460,10 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts cfg->intvars = (guint16 *)mono_mempool_alloc0 (cfg->mempool, sizeof (guint16) * STACK_MAX * header->max_stack); if (cfg->verbose_level > 0) { - char *v_method_name; + char *method_name; - v_method_name = mono_method_get_full_name (method); - g_print ("converting %s%s%s%smethod %s\n", COMPILE_LLVM (cfg) ? "llvm " : "", cfg->gsharedvt ? "gsharedvt " : "", (cfg->gshared && !cfg->gsharedvt) ? "gshared " : "", cfg->interp_entry_only ? "interp only " : "", v_method_name); + method_name = mono_method_get_full_name (method); + g_print ("converting %s%s%s%smethod %s\n", COMPILE_LLVM (cfg) ? "llvm " : "", cfg->gsharedvt ? "gsharedvt " : "", (cfg->gshared && !cfg->gsharedvt) ? "gshared " : "", cfg->interp_entry_only ? "interp only " : "", method_name); /* if (COMPILE_LLVM (cfg)) g_print ("converting llvm method %s\n", method_name = mono_method_full_name (method, TRUE)); @@ -3488,7 +3474,7 @@ mini_method_compile (MonoMethod *method, guint32 opts, JitFlags flags, int parts else g_print ("converting method %s\n", method_name = mono_method_full_name (method, TRUE)); */ - g_free (v_method_name); + g_free (method_name); } if (cfg->opt & MONO_OPT_ABCREM) From 5789ab770f7278e24072e460bceaa1d61e77fbf0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 1 May 2024 11:52:34 -0400 Subject: [PATCH 08/30] use generic method owner caches --- src/mono/mono/metadata/marshal.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 02b7754b0e8d6..0221a4d9a6e55 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5235,23 +5235,16 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf member_name = accessor_method->name; if (accessor_method->is_generic) { + // FullAOT compilation with gshared or gsharedvt will get here. /* got a generic method definition. need to make a generic method definition wrapper */ g_assert (!accessor_method->is_inflated); is_generic = TRUE; } /* FIXME: Support generic methods too */ - if (accessor_method->is_inflated && !mono_method_get_context (accessor_method)->method_inst) { - orig_method = accessor_method; - ctx = &((MonoMethodInflated*)accessor_method)->context; - accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; - container = mono_method_get_generic_container (accessor_method); - if (!container) - container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? - g_assert (container); - } else if (accessor_method->is_inflated && mono_method_get_context(accessor_method)->method_inst) { - // FIXME: ak: do we need a different case? it should work, right? - // N.B. both method_inst and type_inst might be set + if (accessor_method->is_inflated) { + // non-full AOT may get here + g_assert (!is_generic); orig_method = accessor_method; ctx = &((MonoMethodInflated*)accessor_method)->context; accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; @@ -5266,9 +5259,8 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf */ if (ctx) { /* FIXME get the right cache */ - cache = NULL; /* get_cache (&((MonoMethodInflated*)orig_method)->owner->wrapper_caches.synchronized_cache , mono_aligned_addr_hash, NULL); */ - /* res = check_generic_wrapper_cache (cache, orig_method, orig_method, method);*/ - res = NULL; + cache = get_cache (&((MonoMethodInflated*)orig_method)->owner->wrapper_caches.unsafe_accessor_cache , mono_aligned_addr_hash, NULL); + res = check_generic_wrapper_cache (cache, orig_method, orig_method, accessor_method); if (res) return res; } else { From 374c1e745b84eeac129ba1bd45a98d6cb766a8e9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 1 May 2024 15:00:54 -0400 Subject: [PATCH 09/30] lookup unsafe accessor wrapper instances in aot-runtime if someone needs the unsafe accessor, lookup the wrapper instead. Needed when there's a call for extra instances --- src/mono/mono/mini/aot-runtime.c | 65 +++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index b3f4e0bfc8c61..6bc1aa74d3cbe 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -53,6 +53,7 @@ #include #include #include +#include #include #include #include @@ -941,7 +942,7 @@ typedef struct { } MethodRef; static MonoMethod* -instantiate_wrapper_like_decl (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +instantiate_unsafe_accessor_wrapper (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) { MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); @@ -1092,7 +1093,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod MonoGenericContext ctx = {0,}; decode_generic_context (module, &ctx, p, &p, error); mono_error_assert_ok (error); - ref->method = instantiate_wrapper_like_decl (m, &ctx, kind, member_name, error); + ref->method = instantiate_unsafe_accessor_wrapper (m, &ctx, kind, member_name, error); if (!is_ok (error)) return FALSE; } else { @@ -4605,6 +4606,50 @@ inst_is_private (MonoGenericInst *inst) return FALSE; } +static MonoMethod* +instantiate_wrapper_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +{ + g_assert (extern_method_inst->is_inflated); + MonoMethodInflated *infl = (MonoMethodInflated*)extern_method_inst; + MonoMethod *extern_decl = infl->declaring; + MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); + MonoGenericContext *ctx = &infl->context; + MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); + return inflated_wrapper; +} + +static MonoMethod* +replace_generated_method (MonoMethod *method, MonoError *error) +{ + if (G_LIKELY (mono_method_metadata_has_header (method))) + return NULL; + + /* Unsafe accessors methods. Replace attempts to compile the accessor method by + * its wrapper. + */ + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + MonoMethod *wrapper = NULL; + if (method->is_inflated) { + wrapper = instantiate_wrapper_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); + } else { + wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + } + if (is_ok (error)) { + if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_AOT)) { + char * method_name = mono_method_get_full_name (wrapper); + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_AOT, "Replacing generated method by %s", method_name); + g_free (method_name); + } + return wrapper; + } + } + + return NULL; +} + + gboolean mono_aot_can_dedup (MonoMethod *method) { @@ -4625,6 +4670,10 @@ mono_aot_can_dedup (MonoMethod *method) info->subtype == WRAPPER_SUBTYPE_INTERP_LMF || info->subtype == WRAPPER_SUBTYPE_AOT_INIT) return FALSE; + + // TODO: see if we can share these + if (info->subtype == WRAPPER_SUBTYPE_UNSAFE_ACCESSOR) + return FALSE; #if 0 // See is_linkonce_method () in mini-llvm.c if (info->subtype == WRAPPER_SUBTYPE_GSHAREDVT_IN_SIG || info->subtype == WRAPPER_SUBTYPE_GSHAREDVT_OUT_SIG) @@ -4947,6 +4996,7 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) gboolean dedupable = mono_aot_can_dedup (method); + // TODO: unsafe accessor methods should come here too? if (method->is_inflated && !method->wrapper_type && mono_method_is_generic_sharable_full (method, TRUE, FALSE, FALSE) && !dedupable) { MonoMethod *generic_orig_method = method; /* @@ -5085,6 +5135,17 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) } } + if (method_index == 0xffffff && !mono_method_metadata_has_header (method)) { + // replace lookups for unsafe accessor instances by lookups of the wrapper + MonoMethod *wrapper = replace_generated_method (method, error); + mono_error_assert_ok (error); + if (wrapper != NULL) { + method_index = find_aot_method (wrapper, &amodule); + if (method_index != 0xffffff) + method = wrapper; + } + } + if (method_index == 0xffffff) { if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_AOT)) { char *full_name; From db8cbade85ef4835124d28e6ca29819b0ef2e7b0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 1 May 2024 15:02:27 -0400 Subject: [PATCH 10/30] cleanup marshaling - dont' use ctx as a flag --- src/mono/mono/metadata/marshal-lightweight.c | 29 +++++++++------ src/mono/mono/metadata/marshal.c | 39 +++++++++++++++----- src/mono/mono/metadata/marshal.h | 2 +- 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 245b5dbe57250..476e5a32629aa 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2266,7 +2266,7 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo } static void -emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { // Field access requires a single argument for target type and a return type. g_assert (kind == MONO_UNSAFE_ACCESSOR_FIELD || kind == MONO_UNSAFE_ACCESSOR_STATIC_FIELD); @@ -2315,7 +2315,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ * of the expected member method (ie, with the first arg removed) */ static MonoMethodSignature * -method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMethodSignature *accessor_sig, MonoGenericContext *ctx) +method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMethodSignature *accessor_sig) { MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig); g_assert (ret->param_count > 0); @@ -2332,7 +2332,7 @@ method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMetho * of the expected constructor method (same args, but return type is void). */ static MonoMethodSignature * -ctor_sig_from_accessor_sig (MonoMethodBuilder *mb, MonoMethodSignature *accessor_sig, MonoGenericContext *ctx) +ctor_sig_from_accessor_sig (MonoMethodBuilder *mb, MonoMethodSignature *accessor_sig) { MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig); ret->hasthis = TRUE; /* ctors are considered instance methods */ @@ -2410,7 +2410,7 @@ inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_metho } static void -emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { g_assert (kind == MONO_UNSAFE_ACCESSOR_CTOR); // null or empty string member name is ok for a constructor @@ -2434,7 +2434,7 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m sig = update_signature(accessor_method); } - MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig, ctx); + MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig); MonoClass *in_class = mono_class_get_generic_type_definition (target_class); @@ -2459,7 +2459,7 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m } static void -emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { g_assert (kind == MONO_UNSAFE_ACCESSOR_METHOD || kind == MONO_UNSAFE_ACCESSOR_STATIC_METHOD); g_assert (member_name != NULL); @@ -2486,7 +2486,7 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor sig = update_signature(accessor_method); } - MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig, ctx); + MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig); MonoClass *in_class = mono_class_get_generic_type_definition (target_class); @@ -2520,9 +2520,14 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor } static void -emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, gboolean to_be_inflated, MonoUnsafeAccessorKind kind, const char *member_name) { - if (accessor_method->is_generic || ctx != NULL) { + // to_be_inflated means we're emitting a non-generic accessor method belonging to a gtd + // that will be inflated by marshal.c + if (to_be_inflated) + g_assert (!accessor_method->is_inflated && !accessor_method->is_generic); + + if (accessor_method->is_generic || to_be_inflated) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); return; } @@ -2535,14 +2540,14 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ switch (kind) { case MONO_UNSAFE_ACCESSOR_FIELD: case MONO_UNSAFE_ACCESSOR_STATIC_FIELD: - emit_unsafe_accessor_field_wrapper (mb, accessor_method, sig, ctx, kind, member_name); + emit_unsafe_accessor_field_wrapper (mb, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_CTOR: - emit_unsafe_accessor_ctor_wrapper (mb, accessor_method, sig, ctx, kind, member_name); + emit_unsafe_accessor_ctor_wrapper (mb, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: - emit_unsafe_accessor_method_wrapper (mb, accessor_method, sig, ctx, kind, member_name); + emit_unsafe_accessor_method_wrapper (mb, accessor_method, sig, kind, member_name); return; default: mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_InvalidKindValue"); diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 0221a4d9a6e55..755931c46661f 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5230,20 +5230,25 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf MonoMethod *orig_method = NULL; WrapperInfo *info; gboolean is_generic = FALSE; + gboolean is_inflated = FALSE; if (member_name == NULL && kind != MONO_UNSAFE_ACCESSOR_CTOR) member_name = accessor_method->name; + printf("CAME IN: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); + if (accessor_method->is_generic) { // FullAOT compilation with gshared or gsharedvt will get here. /* got a generic method definition. need to make a generic method definition wrapper */ g_assert (!accessor_method->is_inflated); is_generic = TRUE; - } + } else if (accessor_method->is_inflated) { + // FIXME: this always tries to compile a generic version of the accessor method and + // then inflate it. But maybe we dont' want to do that (particularly for field + // accessors). In particular if there is no method_inst, we're looking at an + // accessor method inside a generic class (alwayst a ginst? or sometimes a gtd?) + // In that case we might just want to compile the instance. - /* FIXME: Support generic methods too */ - if (accessor_method->is_inflated) { - // non-full AOT may get here g_assert (!is_generic); orig_method = accessor_method; ctx = &((MonoMethodInflated*)accessor_method)->context; @@ -5252,13 +5257,21 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf if (!container) container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? g_assert (container); + is_inflated = TRUE; } + printf("work on: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); + + /* + * the method is either a generic method definition, or it might be inflated, not both at + * the same time. + */ + g_assert (!(is_generic && is_inflated)); + /* * Check cache */ - if (ctx) { - /* FIXME get the right cache */ + if (is_inflated) { cache = get_cache (&((MonoMethodInflated*)orig_method)->owner->wrapper_caches.unsafe_accessor_cache , mono_aligned_addr_hash, NULL); res = check_generic_wrapper_cache (cache, orig_method, orig_method, accessor_method); if (res) @@ -5268,10 +5281,13 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf if ((res = mono_marshal_find_in_cache (cache, accessor_method))) return res; } - - + printf ("Cache miss\n"); + mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); if (is_generic) { + // If the accessor method was generic, make the wrapper generic, too. + + // Load a copy of the generic params of the accessor method mb->method->is_generic = is_generic; container = mono_class_try_get_generic_container (accessor_method->klass); container = mono_metadata_load_generic_params (m_class_get_image (accessor_method->klass), accessor_method->token, container, /*owner:*/mb->method); @@ -5281,6 +5297,8 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf inst_ctx.method_inst = container->context.method_inst; ERROR_DECL (error); + // make a copy of the accessor signature, but replace the params of the accessor + // method, by the params we just loaded sig = mono_inflate_generic_signature (mono_method_signature_internal (accessor_method), &inst_ctx, error); mono_error_assert_ok (error); // FIXME } else { @@ -5291,14 +5309,15 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf get_marshal_cb ()->mb_skip_visibility (mb); // TODO: pass container, too - get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, ctx, kind, member_name); + // TODO: passing the ctx is not super useful because accessor_method is always the generic version of the method, even if is_inflated == TRUE. pass `is_inflated` instead + get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, is_inflated, kind, member_name); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_UNSAFE_ACCESSOR); info->d.unsafe_accessor.method = accessor_method; info->d.unsafe_accessor.kind = kind; info->d.unsafe_accessor.member_name = member_name; - if (ctx) { + if (is_inflated) { MonoMethod *def; def = mono_mb_create_and_cache_full (cache, accessor_method, mb, sig, sig->param_count + 16, info, NULL); res = cache_generic_wrapper (cache, orig_method, def, ctx, orig_method); diff --git a/src/mono/mono/metadata/marshal.h b/src/mono/mono/metadata/marshal.h index 8d545fcc25dea..2cf169ba24128 100644 --- a/src/mono/mono/metadata/marshal.h +++ b/src/mono/mono/metadata/marshal.h @@ -342,7 +342,7 @@ typedef struct { void (*emit_synchronized_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method); void (*emit_unbox_wrapper) (MonoMethodBuilder *mb, MonoMethod *method); void (*emit_array_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *sig, MonoGenericContext *ctx); - void (*emit_unsafe_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name); + void (*emit_unsafe_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, gboolean to_be_inflated, MonoUnsafeAccessorKind kind, const char *member_name); void (*emit_generic_array_helper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig); void (*emit_thunk_invoke_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig); void (*emit_create_string_hack) (MonoMethodBuilder *mb, MonoMethodSignature *csig, MonoMethod *res); From f252a472ba754508a7c53e67127a9adc121dae11 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 2 May 2024 15:47:55 -0400 Subject: [PATCH 11/30] handle some generic field accessors As long as the target is just some type that mentions a generic field, we're ok - the regular gsharing ldflda works. It just can't be a type variable. --- src/mono/mono/metadata/marshal-lightweight.c | 75 +++++++++++++++++--- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 476e5a32629aa..a13dc8d5a97e5 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2266,13 +2266,65 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo } static void -emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflated, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { // Field access requires a single argument for target type and a return type. g_assert (kind == MONO_UNSAFE_ACCESSOR_FIELD || kind == MONO_UNSAFE_ACCESSOR_STATIC_FIELD); g_assert (member_name != NULL); MonoType *target_type = sig->params[0]; // params[0] is the field's parent + + // Try to do a static lookup. + // if the target type is G (not just !T or !!T), we can get a field token + if ((accessor_method->is_generic || to_be_inflated) && + (target_type->type == MONO_TYPE_VAR || target_type->type == MONO_TYPE_MVAR)) { + // this is the most dynamic version. + // + // it's not actually legal for fields (although I + // guess it might be legal with constraints) + // + // [UnsafeAccessor(Field, Name="_x")] + // ref SomeClass GetField(ref U target) where U : BaseClass; + // + // class BaseClass { + // private SomeClass _x; + // } + + // have to do an icall to get the field/method/ctor. + // and we need a new IL opcode mono_ldtypeof that takes a MonoClass + // and turns it into the runtime MonoClass that has gshared + // params replaced by the actual runtime instantiations. + // + // ldtypeof + // should turn into: + // mini_emit_get_rgctx_klass (cfg, context_used, klass, MONO_RGCTX_INFO_KLASS) + + // Then we can emit this: + + // ldtypeof params[0]> + // ldconst + // ldstr + // param_count; i++> + // ldtypeof params[i]> + // + // icall // may throw + // + // param_count; i++> + // ldarg + // + // calli // FIXME: method is top of stack? + // ret + // + // ret // address of fielde + // + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); + return; + }; + + // otherwise target_type is something we can do lookups on. + // We can at least find the generic MonoClassField and then + // ldflda will handle the generic sharing. + MonoType *ret_type = sig->ret; if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute."); @@ -2524,14 +2576,9 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ { // to_be_inflated means we're emitting a non-generic accessor method belonging to a gtd // that will be inflated by marshal.c - if (to_be_inflated) - g_assert (!accessor_method->is_inflated && !accessor_method->is_generic); + if (to_be_inflated && !accessor_method->is_generic) + g_assert (!accessor_method->is_inflated); - if (accessor_method->is_generic || to_be_inflated) { - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); - return; - } - if (!m_method_is_static (accessor_method)) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic"); return; @@ -2540,13 +2587,23 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ switch (kind) { case MONO_UNSAFE_ACCESSOR_FIELD: case MONO_UNSAFE_ACCESSOR_STATIC_FIELD: - emit_unsafe_accessor_field_wrapper (mb, accessor_method, sig, kind, member_name); + emit_unsafe_accessor_field_wrapper (mb, to_be_inflated, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_CTOR: + if (accessor_method->is_generic || to_be_inflated) { + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); + return; + } + emit_unsafe_accessor_ctor_wrapper (mb, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: + if (accessor_method->is_generic || to_be_inflated) { + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); + return; + } + emit_unsafe_accessor_method_wrapper (mb, accessor_method, sig, kind, member_name); return; default: From e424b641fdf72e7c6bae208d46cce0bc44fb89f9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 2 May 2024 15:48:49 -0400 Subject: [PATCH 12/30] issues.targets: enable some unsafe accessor AOT tests --- src/tests/issues.targets | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 8289fcefb924f..ee711ae4b2742 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2622,8 +2622,6 @@ - - @@ -3895,7 +3893,6 @@ - From 03c72f777489deba175b459724849361bd60dda9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 2 May 2024 16:34:40 -0400 Subject: [PATCH 13/30] WIP: looks like methods are working too? --- src/mono/mono/metadata/marshal-lightweight.c | 29 +++++--- src/mono/sample/HelloWorld/Program.cs | 72 ++++++++++++++++++-- 2 files changed, 85 insertions(+), 16 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index a13dc8d5a97e5..58bd37e5a29ae 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2511,7 +2511,7 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m } static void -emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflated, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { g_assert (kind == MONO_UNSAFE_ACCESSOR_METHOD || kind == MONO_UNSAFE_ACCESSOR_STATIC_METHOD); g_assert (member_name != NULL); @@ -2525,6 +2525,18 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor } MonoType *target_type = sig->params[0]; + + // Try to do a static lookup. if the target type is G (not + // just !T or !!T), we can get a method. TODO: if it's !T we + // can get some System.Object methods, too. Also if the type + // parameter is constrained we can call some of the methods + // from the constraint. + if ((accessor_method->is_generic || to_be_inflated) && + (target_type->type == MONO_TYPE_VAR || target_type->type == MONO_TYPE_MVAR)) { + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); + return; + }; + gboolean hasthis = kind == MONO_UNSAFE_ACCESSOR_METHOD; MonoClass *target_class = mono_class_from_mono_type_internal (target_type); @@ -2534,13 +2546,13 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor } ERROR_DECL(find_method_error); - if (accessor_method->is_inflated) { - sig = update_signature(accessor_method); - } + //if (accessor_method->is_inflated) { + // sig = update_signature(accessor_method); + //} MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig); - MonoClass *in_class = mono_class_get_generic_type_definition (target_class); + MonoClass *in_class = target_class; // mono_class_get_generic_type_definition (target_class); MonoMethod *target_method = NULL; if (!ctor_as_method) @@ -2599,12 +2611,7 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: - if (accessor_method->is_generic || to_be_inflated) { - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); - return; - } - - emit_unsafe_accessor_method_wrapper (mb, accessor_method, sig, kind, member_name); + emit_unsafe_accessor_method_wrapper (mb, to_be_inflated, accessor_method, sig, kind, member_name); return; default: mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_InvalidKindValue"); diff --git a/src/mono/sample/HelloWorld/Program.cs b/src/mono/sample/HelloWorld/Program.cs index d9f391e92ef30..e87371c54ac1f 100644 --- a/src/mono/sample/HelloWorld/Program.cs +++ b/src/mono/sample/HelloWorld/Program.cs @@ -4,6 +4,8 @@ using System; using System.Runtime.CompilerServices; +#if false + public class MyBox { private T _value; @@ -11,16 +13,28 @@ public class MyBox public MyBox(T inp) { Value = inp; } } +public class SimpleBox +{ + private decimal _value; + public decimal Value { get => _value; private set { _value = value;}} + public SimpleBox(decimal inp) { Value = inp;} +} + public class Program { public static void Main() { + var sbox = new SimpleBox((decimal)-5); + RunSimple(sbox, (decimal)20); + Console.WriteLine ("20 = {0}", sbox.Value); var box = new MyBox("xyz"); - //Thinger(box); - //RunIt(box, "abc"); - //Console.WriteLine (box.Value); RunItAgain(box, "hjk"); - Console.WriteLine (box.Value); + Console.WriteLine ("hjk = {0}", box.Value); + var box2 = new MyBox((decimal)-2); + RunItAgain(box2, (decimal)10.0); + Console.WriteLine (box2.Value); + RunIt(box, "abc"); + Console.WriteLine ("abc = {0}", box.Value); } [MethodImpl(MethodImplOptions.NoInlining)] @@ -41,11 +55,59 @@ public static void RunItAgain (MyBox dest, S input) private static extern ref W AccessBox(MyBox x); [MethodImpl(MethodImplOptions.NoInlining)] - private static ref W Thinger(MyBox x) => throw new InvalidOperationException("oops"); + private static void RunSimple(SimpleBox s, decimal input) + { + ref decimal boxWriter = ref SimpleHelper.AccessSimpleBox(s); + boxWriter = input; + } } public class AccessHelper { +#if false + [MethodImpl(MethodImplOptions.NoInlining)] + public static /*extern*/ ref Q AccessBox2(MyBox q) => throw new NotImplementedException("exn"); +#else [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] public static extern ref Q AccessBox2(MyBox q); +#endif +} + +public class SimpleHelper +{ + [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] + public static extern ref decimal AccessSimpleBox(SimpleBox b); +} + +#else + +public class MyService +{ + public MyService() {} + + private static string Explain(T arg) => arg.ToString() + " : " + typeof(T).ToString(); //$"{arg} : {typeof(T)}"; +} + +public class Program +{ + public static void Main() + { + new Program().Runner("abcd"); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public void Runner(Q x) + { + Console.WriteLine (AccessHelper.CallExplain(default, x)); + } + } + +public static class AccessHelper +{ + [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name="Explain")] + public static extern string CallExplain(MyService target, W arg); + +} + +#endif From 6fcefc69f5920d8e82514d7a74fc30837b5810ef Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 6 May 2024 14:58:59 -0400 Subject: [PATCH 14/30] [metadata] add ability to inflate wrapper data When we create generic wrappers (or wrappers in a generic class), if the wrapper data needs to refer to a field, method, or parameter type of the definition, that data might need to be inflated if the containing class is inflated (or the generic wrapper method is inflated). Add a new function to opt into inflation: ```c get_marshal_cb ()->mb_inflate_wrapper_data (mb); ``` Add a new function to be called after mono_mb_emit_op (or any other call that calls mono_mb_add_data): ```c mono_mb_emit_op (mb, CEE_LDFLDA, field); mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_FIELD); ``` Note: mono_mb_set_wrapper_data_kind asserts if you haven't called mb_inflate_wrapper_data. TODO: add more wrapper data kinds for MonoMethod* and MonoClass* etc --- src/mono/mono/metadata/class-internals.h | 1 + src/mono/mono/metadata/class.c | 12 ++ src/mono/mono/metadata/marshal-lightweight.c | 11 ++ src/mono/mono/metadata/marshal.h | 1 + .../metadata/method-builder-ilgen-internals.h | 19 ++++ src/mono/mono/metadata/method-builder-ilgen.c | 105 +++++++++++++++++- src/mono/mono/metadata/method-builder-ilgen.h | 7 ++ 7 files changed, 151 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index e7b56c72a38e2..b423350d29810 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -96,6 +96,7 @@ struct _MonoMethodWrapper { MonoMethodHeader *header; MonoMemoryManager *mem_manager; void *method_data; + unsigned int inflate_wrapper_data : 1; /* method_data[MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX] is an MonoMethodBuilderInflateWrapperData array */ }; struct _MonoDynamicMethod { diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 01888dfbabd57..313e8b822036e 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -1263,6 +1264,17 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k resw->method_data = (void **)g_malloc (sizeof (gpointer) * (len + 1)); memcpy (resw->method_data, mw->method_data, sizeof (gpointer) * (len + 1)); + if (mw->inflate_wrapper_data) { + mono_mb_inflate_generic_wrapper_data (context, (gpointer*)resw->method_data, error); + if (!is_ok (error)) { + g_free (resw->method_data); + goto fail; + } + // we can't set inflate_wrapper_data to 0 on the result, it's possible it + // will need to be inflated again (for example in the method_inst == + // generic_container->context.method_inst case, below) + resw->inflate_wrapper_data = 1; + } } if (iresult->context.method_inst) { diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 58bd37e5a29ae..158e8a3889e36 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2136,6 +2136,16 @@ mb_skip_visibility_ilgen (MonoMethodBuilder *mb) mb->skip_visibility = 1; } +static void +mb_inflate_wrapper_data_ilgen (MonoMethodBuilder *mb) +{ + g_assert (!mb->dynamic); // dynamic methods with inflated data not implemented yet - needs at least mono_free_method changes, probably more + mb->inflate_wrapper_data = TRUE; + int idx = mono_mb_add_data (mb, NULL); + // note: match index used in create_method_ilgen + g_assertf (idx == MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX, "mb_inflate_wrapper_data called after data already added"); +} + static void emit_synchronized_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method) { @@ -3467,6 +3477,7 @@ mono_marshal_lightweight_init (void) cb.emit_return = emit_return_ilgen; cb.emit_vtfixup_ftnptr = emit_vtfixup_ftnptr_ilgen; cb.mb_skip_visibility = mb_skip_visibility_ilgen; + cb.mb_inflate_wrapper_data = mb_inflate_wrapper_data_ilgen; cb.mb_emit_exception = mb_emit_exception_ilgen; cb.mb_emit_exception_for_error = mb_emit_exception_for_error_ilgen; cb.mb_emit_byte = mb_emit_byte_ilgen; diff --git a/src/mono/mono/metadata/marshal.h b/src/mono/mono/metadata/marshal.h index 2cf169ba24128..61b006506c493 100644 --- a/src/mono/mono/metadata/marshal.h +++ b/src/mono/mono/metadata/marshal.h @@ -351,6 +351,7 @@ typedef struct { void (*emit_return) (MonoMethodBuilder *mb); void (*emit_vtfixup_ftnptr) (MonoMethodBuilder *mb, MonoMethod *method, int param_count, guint16 type); void (*mb_skip_visibility) (MonoMethodBuilder *mb); + void (*mb_inflate_wrapper_data) (MonoMethodBuilder *mb); void (*mb_emit_exception) (MonoMethodBuilder *mb, const char *exc_nspace, const char *exc_name, const char *msg); void (*mb_emit_exception_for_error) (MonoMethodBuilder *mb, const MonoError *emitted_error); void (*mb_emit_byte) (MonoMethodBuilder *mb, guint8 op); diff --git a/src/mono/mono/metadata/method-builder-ilgen-internals.h b/src/mono/mono/metadata/method-builder-ilgen-internals.h index 1aa410beb34a5..0ca7d88ae2aa6 100644 --- a/src/mono/mono/metadata/method-builder-ilgen-internals.h +++ b/src/mono/mono/metadata/method-builder-ilgen-internals.h @@ -33,6 +33,25 @@ struct _MonoMethodBuilder { const gchar **param_names; MonoBitSet *volatile_args; MonoBitSet *volatile_locals; + gboolean inflate_wrapper_data; + GList *wrapper_data_inflate_info; }; +typedef struct MonoMethodBuilderInflateWrapperData +{ + uint16_t idx; + uint16_t kind; // MonoILGenWrapperDataKind +} MonoMethodBuilderInflateWrapperData; + +enum MonoILGenWrapperDataKind +{ + MONO_MB_ILGEN_WRAPPER_DATA_NONE = 0, + MONO_MB_ILGEN_WRAPPER_DATA_FIELD, +}; + +// index in MonoMethodWrapper:wrapper_data where the inflate info will be stored +// notes: +// - idx 1 is the wrapper info (see mono_method_get_wrapper_info) +#define MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX 2 + #endif diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index 287412110e4e5..43193c452f1b6 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -52,7 +52,7 @@ new_base_ilgen (MonoClass *klass, MonoWrapperType type, gboolean dynamic) mb->init_locals = TRUE; mb->dynamic = dynamic; - /* placeholder for the wrapper always at index 1 */ + /* placeholder for the wrapper always at index 1, see mono_marshal_set_wrapper_info */ mono_mb_add_data (mb, NULL); return mb; @@ -61,9 +61,15 @@ new_base_ilgen (MonoClass *klass, MonoWrapperType type, gboolean dynamic) static void free_ilgen (MonoMethodBuilder *mb) { - GList *l; + if (mb->wrapper_data_inflate_info) { + for (GList *p = mb->wrapper_data_inflate_info; p && p->data; p = p->next) { + g_free (p->data); + } + g_list_free (mb->wrapper_data_inflate_info); + mb->wrapper_data_inflate_info = NULL; + } - for (l = mb->locals_list; l; l = l->next) { + for (GList *l = mb->locals_list; l; l = l->next) { /* Allocated in mono_mb_add_local () */ g_free (l->data); } @@ -172,14 +178,44 @@ create_method_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *signature, int i = g_list_length ((GList *)mw->method_data); if (i) { + MonoMethodBuilderInflateWrapperData *inflate_data = NULL; + if (mb->inflate_wrapper_data) { + int count = g_list_length (mb->wrapper_data_inflate_info); + inflate_data = (MonoMethodBuilderInflateWrapperData *) mb_alloc0 (mb, sizeof(MonoMethodBuilderInflateWrapperData) * (count + 1)); + int j = 0; + for (GList *p = mb->wrapper_data_inflate_info; p && p->data; p = p->next) { + inflate_data[j++] = *(MonoMethodBuilderInflateWrapperData*)p->data; + } + // trailing idx 0 element to mark the end + inflate_data[j].idx = 0; + inflate_data[j].kind = MONO_MB_ILGEN_WRAPPER_DATA_NONE; + mw->inflate_wrapper_data = 1; + } GList *tmp; void **data; l = g_list_reverse ((GList *)mw->method_data); - data = (void **)mb_alloc0 (mb, sizeof (gpointer) * (i + 1)); + int data_count = i + (inflate_data ? 2 : 1); + data = (void **)mb_alloc0 (mb, sizeof (gpointer) * data_count); /* store the size in the first element */ data [0] = GUINT_TO_POINTER (i); i = 1; - for (tmp = l; tmp; tmp = tmp->next) { + + // manually peel off the first 1 or 2 iterations of the loop since they're special + tmp = l; + g_assert (tmp); + // wrapper info is in slot 1 + g_assert (tmp->data == NULL); + data [i++] = NULL; + tmp = tmp->next; + // inflate data is in slot 2 + if (inflate_data) { + g_assert (tmp); + g_assert (MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX == i); + g_assert (tmp->data == NULL); + data[i++] = inflate_data; + tmp = tmp->next; + } + for (;tmp; tmp = tmp->next) { data [i++] = tmp->data; } g_list_free (l); @@ -664,3 +700,62 @@ mono_mb_set_param_names (MonoMethodBuilder *mb, const char **param_names) { mb->param_names = param_names; } + +void +mono_mb_set_wrapper_data_kind (MonoMethodBuilder *mb, uint16_t wrapper_data_kind) +{ + g_assert (mb->inflate_wrapper_data); + MonoMethodWrapper *mw = (MonoMethodWrapper*)mb->method; + // index of the data added by most recent mono_mb_add_data + int idx = g_list_length((GList *)mw->method_data); + g_assert (idx > 0 && idx <= UINT16_MAX); + + MonoMethodBuilderInflateWrapperData *info = g_new (MonoMethodBuilderInflateWrapperData, 1); + info->idx = (uint16_t)idx; + info->kind = wrapper_data_kind; + mb->wrapper_data_inflate_info = g_list_prepend (mb->wrapper_data_inflate_info, info); +} + +gboolean +mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *method_data, MonoError *error) +{ + MonoMethodBuilderInflateWrapperData* inflate_info = (MonoMethodBuilderInflateWrapperData*)method_data[MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX]; + MonoMethodBuilderInflateWrapperData* p = inflate_info; + int count = 0; + while (p->idx != 0) {p++; count++;} + + for (int i = 0; i < count; i++) { + int idx = inflate_info[i].idx; + uint16_t kind = inflate_info[i].kind; + gpointer *pdata = &method_data[idx]; + switch (kind) { + case MONO_MB_ILGEN_WRAPPER_DATA_NONE: + continue; + case MONO_MB_ILGEN_WRAPPER_DATA_FIELD: { + MonoClassField *field = (MonoClassField*)*pdata; + MonoType *inflated_type = mono_class_inflate_generic_type_checked (m_class_get_byval_arg (m_field_get_parent (field)), context, error); + if (!is_ok (error)) + return FALSE; + + MonoClass *inflated_class = mono_class_from_mono_type_internal (inflated_type); + // TODO: EnC metadata-update + g_assert (!m_field_is_from_update (field)); + int i = GPTRDIFF_TO_INT (field - m_class_get_fields (m_field_get_parent (field))); + gpointer dummy = NULL; + + mono_metadata_free_type (inflated_type); + + mono_class_get_fields_internal (inflated_class, &dummy); + g_assert (m_class_get_fields (inflated_class)); + + MonoClassField *inflated_field = &m_class_get_fields (inflated_class) [i]; + + *pdata = inflated_field; + break; + } + default: + g_assert_not_reached(); + } + } + return TRUE; +} diff --git a/src/mono/mono/metadata/method-builder-ilgen.h b/src/mono/mono/metadata/method-builder-ilgen.h index 2a5e40dc70256..209565d30282c 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.h +++ b/src/mono/mono/metadata/method-builder-ilgen.h @@ -139,4 +139,11 @@ mono_mb_set_param_names (MonoMethodBuilder *mb, const char **param_names); char* mono_mb_strdup (MonoMethodBuilder *mb, const char *s); +void +mono_mb_set_wrapper_data_kind (MonoMethodBuilder *mb, uint16_t wrapper_data_kind); + +gboolean +mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *method_data, MonoError *error); + + #endif From ccbd0ba817f79b47d88d1e251ff1364d1cc1ef2d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 6 May 2024 15:04:28 -0400 Subject: [PATCH 15/30] [marshal] refactor unsafe accessor; opt into inflate_wrapper_data Try to separate the ideas of "the call to the UnsafeAccessor method was inflated, so we need to inflate the wrapper" and "the UnsafeAccessor method is a generic method definition, so the wrapper should be a generic method definition, too" --- src/mono/mono/metadata/marshal-lightweight.c | 2 + src/mono/mono/metadata/marshal.c | 90 +++++++++++++++----- 2 files changed, 71 insertions(+), 21 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 158e8a3889e36..d25cfebf19ac3 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2369,6 +2369,8 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflat if (kind == MONO_UNSAFE_ACCESSOR_FIELD) mono_mb_emit_ldarg (mb, 0); mono_mb_emit_op (mb, kind == MONO_UNSAFE_ACCESSOR_FIELD ? CEE_LDFLDA : CEE_LDSFLDA, target_field); + if ((accessor_method->is_generic || to_be_inflated)) + mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_FIELD); mono_mb_emit_byte (mb, CEE_RET); } diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 755931c46661f..66eae90d6a7fd 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5229,45 +5229,82 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf MonoGenericContainer *container = NULL; MonoMethod *orig_method = NULL; WrapperInfo *info; - gboolean is_generic = FALSE; + /* generic_wrapper == TRUE means we will create a generic wrapper method. */ + gboolean generic_wrapper = FALSE; + /* is_inflated == TRUE means we will inflate the wrapper method - the */ gboolean is_inflated = FALSE; + /* is_generic and is_inflated might be set independently, depending on how we're called. */ if (member_name == NULL && kind != MONO_UNSAFE_ACCESSOR_CTOR) member_name = accessor_method->name; printf("CAME IN: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); + /* + * the method is either a generic method definition, or it might be inflated, not both at + * the same time. + */ + g_assert (!(accessor_method->is_generic && accessor_method->is_inflated)); + + + if (accessor_method->is_inflated) { + MonoMethod *declaring = ((MonoMethodInflated*)accessor_method)->declaring; + if (declaring->is_generic) { + // JIT gets here sometimes. + generic_wrapper = TRUE; + } + is_inflated = TRUE; + } + if (accessor_method->is_generic) { // FullAOT compilation with gshared or gsharedvt will get here. /* got a generic method definition. need to make a generic method definition wrapper */ g_assert (!accessor_method->is_inflated); - is_generic = TRUE; - } else if (accessor_method->is_inflated) { + generic_wrapper = TRUE; + } + + if (is_inflated) { // FIXME: this always tries to compile a generic version of the accessor method and // then inflate it. But maybe we dont' want to do that (particularly for field // accessors). In particular if there is no method_inst, we're looking at an // accessor method inside a generic class (alwayst a ginst? or sometimes a gtd?) // In that case we might just want to compile the instance. - g_assert (!is_generic); - orig_method = accessor_method; - ctx = &((MonoMethodInflated*)accessor_method)->context; - accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; - container = mono_method_get_generic_container (accessor_method); - if (!container) - container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? - g_assert (container); - is_inflated = TRUE; + if (!generic_wrapper) { + orig_method = accessor_method; + ctx = &((MonoMethodInflated*)accessor_method)->context; + accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; + container = mono_method_get_generic_container (accessor_method); + if (!container) + container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? + g_assert (container); + } else { + // FIXME: + // in this case do we need to mess with the context and container? + // + // class C { + // public static extern void AccessorMethod(List t, List u); + // } + // + // when we need to make a wrapper + // + // public static extern void wrapper_AccessorMethod(List, List u); + // + // where we substitute the new gparams of the wrapper, but leave the + // gparams of C unchanged. + // + orig_method = accessor_method; + ctx = &((MonoMethodInflated*)accessor_method)->context; + accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; + container = mono_method_get_generic_container (accessor_method); + if (!container) + container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? + g_assert (container); + } } printf("work on: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); - /* - * the method is either a generic method definition, or it might be inflated, not both at - * the same time. - */ - g_assert (!(is_generic && is_inflated)); - /* * Check cache */ @@ -5284,16 +5321,17 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf printf ("Cache miss\n"); mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); - if (is_generic) { + if (generic_wrapper) { // If the accessor method was generic, make the wrapper generic, too. // Load a copy of the generic params of the accessor method - mb->method->is_generic = is_generic; + mb->method->is_generic = generic_wrapper; container = mono_class_try_get_generic_container (accessor_method->klass); container = mono_metadata_load_generic_params (m_class_get_image (accessor_method->klass), accessor_method->token, container, /*owner:*/mb->method); mono_method_set_generic_container (mb->method, container); MonoGenericContext inst_ctx = {0,}; + // FIXME: if is_inflated, do we need to mess with ctx? inst_ctx.method_inst = container->context.method_inst; ERROR_DECL (error); @@ -5308,8 +5346,12 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf get_marshal_cb ()->mb_skip_visibility (mb); + if (generic_wrapper || is_inflated) { + // wrapper data will mention MonoClassField* and MonoMethod* that need to be inflated + get_marshal_cb ()->mb_inflate_wrapper_data (mb); + } + // TODO: pass container, too - // TODO: passing the ctx is not super useful because accessor_method is always the generic version of the method, even if is_inflated == TRUE. pass `is_inflated` instead get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, is_inflated, kind, member_name); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_UNSAFE_ACCESSOR); @@ -5321,6 +5363,12 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf MonoMethod *def; def = mono_mb_create_and_cache_full (cache, accessor_method, mb, sig, sig->param_count + 16, info, NULL); res = cache_generic_wrapper (cache, orig_method, def, ctx, orig_method); + // FIXME: this right here is where things went bad when we inflated the generic + // wrapper. The reason is because all the MonoMethodWrapper:method_data is still + // refering to the generic class. But in the JIT case we don't have generic + // sharing, and so none of that stuff will get substituted, probably. + + // Why does this work for class context generic wrappers? } else { res = mono_mb_create_and_cache_full (cache, accessor_method, mb, sig, sig->param_count + 16, info, NULL); } From 56516b29d352de83c7810430de88383a771ff6d6 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 6 May 2024 15:36:09 -0400 Subject: [PATCH 16/30] comment out printfs --- src/mono/mono/metadata/marshal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 66eae90d6a7fd..ae38a98589bce 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5238,7 +5238,7 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf if (member_name == NULL && kind != MONO_UNSAFE_ACCESSOR_CTOR) member_name = accessor_method->name; - printf("CAME IN: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); + // printf("CAME IN: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); /* * the method is either a generic method definition, or it might be inflated, not both at @@ -5303,7 +5303,7 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf } } - printf("work on: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); + // printf("work on: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); /* * Check cache @@ -5318,7 +5318,7 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf if ((res = mono_marshal_find_in_cache (cache, accessor_method))) return res; } - printf ("Cache miss\n"); + // printf ("Cache miss\n"); mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); if (generic_wrapper) { From 769ae5554e5787124bdd8ef1b8099f03e314188f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 6 May 2024 15:54:03 -0400 Subject: [PATCH 17/30] inflate MonoMethod wrapper data; impl ctor generic unsafe accessors --- src/mono/mono/metadata/marshal-lightweight.c | 60 ++++++++----------- .../metadata/method-builder-ilgen-internals.h | 1 + src/mono/mono/metadata/method-builder-ilgen.c | 8 +++ 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index c0137c53f48a2..eb10da0647bb0 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2275,6 +2275,9 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo mono_mb_emit_byte (mb, CEE_RET); } +static gboolean +unsafe_accessor_target_type_forbidden (MonoType *target_type); + static void emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflated, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { @@ -2398,28 +2401,6 @@ emit_missing_method_error (MonoMethodBuilder *mb, MonoError *failure, const char } } -static MonoMethodSignature * -update_signature (MonoMethod *accessor_method) -{ - MonoClass *accessor_method_class_instance = accessor_method->klass; - MonoClass *accessor_method_class = mono_class_get_generic_type_definition (accessor_method_class_instance); - - const char *accessor_method_name = accessor_method->name; - - gpointer iter = NULL; - MonoMethod *m = NULL; - while ((m = mono_class_get_methods (accessor_method_class, &iter))) { - if (!m) - continue; - - if (strcmp (m->name, accessor_method_name)) - continue; - - return mono_metadata_signature_dup_full (get_method_image (m), mono_method_signature_internal (m)); - } - g_assert_not_reached (); -} - static MonoMethod * inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_method, MonoError *error) { @@ -2437,7 +2418,7 @@ inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_metho } static void -emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflated, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { g_assert (kind == MONO_UNSAFE_ACCESSOR_CTOR); // null or empty string member name is ok for a constructor @@ -2455,17 +2436,23 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m return; } + gboolean inflate_generic_data = FALSE; + if ((accessor_method->is_generic || to_be_inflated)) { + // We want to do a static lookup: target type must be G, not just !T or !!T + g_assert (target_type->type != MONO_TYPE_VAR && target_type->type != MONO_TYPE_MVAR); + // If the target is generic, we'll find a generic MonoMethod* and then mark it to be + // inflated in order to handle instances, or gshared/gsharedvt instances. + + inflate_generic_data = TRUE; + }; + MonoClass *target_class = mono_class_from_mono_type_internal (target_type); ERROR_DECL(find_method_error); - if (accessor_method->is_inflated) { - sig = update_signature(accessor_method); - target_type = sig->ret; - } MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig); - MonoClass *in_class = mono_class_get_generic_type_definition (target_class); + MonoClass *in_class = target_class; // mono_class_get_generic_type_definition (target_class); MonoMethod *target_method = mono_unsafe_accessor_find_ctor (in_class, member_sig, target_class, find_method_error); if (!is_ok (find_method_error) || target_method == NULL) { @@ -2484,6 +2471,8 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m emit_unsafe_accessor_ldargs (mb, sig, 0); mono_mb_emit_op (mb, CEE_NEWOBJ, target_method); + if (inflate_generic_data) + mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_METHOD); mono_mb_emit_byte (mb, CEE_RET); } @@ -2498,16 +2487,18 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_infla - MonoType *target_type = sig->param_count > 1 ? sig->params[0] : NULL; + MonoType *target_type = sig->param_count >= 1 ? sig->params[0] : NULL; if (sig->param_count < 1 || target_type == NULL || unsafe_accessor_target_type_forbidden (target_type)) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute."); return; } + gboolean inflate_generic_data = FALSE; if ((accessor_method->is_generic || to_be_inflated)) { // We want to do a static lookup: target type must be G, not just !T or !!T - g_assert (target_type->type != MONO_TYPE_VAR && target_type->type != MONO_TYPE_MVAR)); + g_assert (target_type->type != MONO_TYPE_VAR && target_type->type != MONO_TYPE_MVAR); + inflate_generic_data = TRUE; }; gboolean hasthis = kind == MONO_UNSAFE_ACCESSOR_METHOD; @@ -2550,6 +2541,8 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_infla emit_unsafe_accessor_ldargs (mb, sig, !hasthis ? 1 : 0); mono_mb_emit_op (mb, hasthis ? CEE_CALLVIRT : CEE_CALL, target_method); + if (inflate_generic_data) + mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_METHOD); mono_mb_emit_byte (mb, CEE_RET); } @@ -2572,12 +2565,7 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ emit_unsafe_accessor_field_wrapper (mb, to_be_inflated, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_CTOR: - if (accessor_method->is_generic || to_be_inflated) { - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics"); - return; - } - - emit_unsafe_accessor_ctor_wrapper (mb, accessor_method, sig, kind, member_name); + emit_unsafe_accessor_ctor_wrapper (mb, to_be_inflated, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: diff --git a/src/mono/mono/metadata/method-builder-ilgen-internals.h b/src/mono/mono/metadata/method-builder-ilgen-internals.h index 0ca7d88ae2aa6..c66ace69b8cf9 100644 --- a/src/mono/mono/metadata/method-builder-ilgen-internals.h +++ b/src/mono/mono/metadata/method-builder-ilgen-internals.h @@ -47,6 +47,7 @@ enum MonoILGenWrapperDataKind { MONO_MB_ILGEN_WRAPPER_DATA_NONE = 0, MONO_MB_ILGEN_WRAPPER_DATA_FIELD, + MONO_MB_ILGEN_WRAPPER_DATA_METHOD, }; // index in MonoMethodWrapper:wrapper_data where the inflate info will be stored diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index 43193c452f1b6..427ba7e4543ae 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -753,6 +753,14 @@ mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *met *pdata = inflated_field; break; } + case MONO_MB_ILGEN_WRAPPER_DATA_METHOD: { + MonoMethod *method = (MonoMethod*)*pdata; + MonoMethod *inflated_method = mono_class_inflate_generic_method_checked (method, context, error); + if (!is_ok (error)) + return FALSE; + *pdata = inflated_method; + break; + } default: g_assert_not_reached(); } From 9688f2b3f4b47a874c6ddbf81d4d3ac622bd949d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 6 May 2024 19:20:52 -0400 Subject: [PATCH 18/30] fix windows build --- src/mono/mono/metadata/method-builder-ilgen.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index 427ba7e4543ae..1e0b1060396e9 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -724,9 +724,9 @@ mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *met int count = 0; while (p->idx != 0) {p++; count++;} - for (int i = 0; i < count; i++) { - int idx = inflate_info[i].idx; - uint16_t kind = inflate_info[i].kind; + for (int x = 0; x < count; x++) { + int idx = inflate_info[x].idx; + uint16_t kind = inflate_info[x].kind; gpointer *pdata = &method_data[idx]; switch (kind) { case MONO_MB_ILGEN_WRAPPER_DATA_NONE: From 70aaa2e675d9531ed7c011a2dc13404e990cf251 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 7 May 2024 13:00:59 -0400 Subject: [PATCH 19/30] [aot] handle case of partial generic sharing for unsafe accessor In partial generic sharing, a call to an instance like `Foo` is replaced by `Foo` where T is constrained to `int` and enums that use `int` as a base type. In that case the AOT compiler will emit the unsafe accessor wrapper instantiated with `T_INT`. So in the AOT lookup we have to find calls to `UnsafeAccessor` and replace them with calls to `(wrapper) UnsafeAccessor` to match what the AOT compiler is doing --- src/mono/mono/mini/aot-runtime.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index 6bc1aa74d3cbe..1b3212cf40501 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -5117,6 +5117,19 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) method_index = find_aot_method (shared, &amodule); if (method_index != 0xffffff) method = shared; + if (method_index == 0xffffff && !mono_method_metadata_has_header (method)) { + // replace lookups for unsafe accessor instances by lookups of the wrapper + MonoMethod *wrapper = replace_generated_method (method, error); + mono_error_assert_ok (error); + if (wrapper != NULL) { + shared = mini_get_shared_method_full (wrapper, SHARE_MODE_NONE, error); + return_val_if_nok (error, NULL); + + method_index = find_aot_method (shared, &amodule); + if (method_index != 0xffffff) + method = shared; + } + } } if (method_index == 0xffffff && method->is_inflated && mono_method_is_generic_sharable_full (method, FALSE, FALSE, TRUE)) { From e814387dd093b44cbc6e09379281c0d16cd729f2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 7 May 2024 15:42:11 -0400 Subject: [PATCH 20/30] [aot] for unsafe accessor wrappers with no name, record a length 0 This is needed because for inflated unsafe accessors we write the inflated bit after the name. So if we're accessing a constructor and we didn't record a name in the AOT image, we would erroneously read the inflated bit as the name length. --- src/mono/mono/mini/aot-compiler.c | 2 ++ src/mono/mono/mini/aot-runtime.c | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 229c9ea17f7b2..09f7422ffe72e 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -3873,6 +3873,8 @@ encode_method_ref (MonoAotCompile *acfg, MonoMethod *method, guint8 *buf, guint8 uint32_t len = (uint32_t) strlen (info->d.unsafe_accessor.member_name); encode_value (len, p, &p); encode_string (info->d.unsafe_accessor.member_name, p, &p); + } else { + encode_value (0, p, &p); } if (method->is_inflated) { encode_value(1, p, &p); diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index 1b3212cf40501..17bd8581bf74c 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -1086,8 +1086,11 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod return FALSE; MonoUnsafeAccessorKind kind = (MonoUnsafeAccessorKind) decode_value (p, &p); uint32_t name_len = decode_value (p, &p); - const char *member_name = (const char*)p; - p += name_len + 1; + const char *member_name = NULL; + if (name_len > 0) { + member_name = (const char*)p; + p += name_len + 1; + } int32_t inflated = decode_value (p, &p); if (inflated) { MonoGenericContext ctx = {0,}; From 03208db64a456969236cb36f39d8d40420e22fcd Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 8 May 2024 16:19:50 -0400 Subject: [PATCH 21/30] [aot-runtime] try to fix gsharedvt lookup --- src/mono/mono/mini/aot-runtime.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index 17bd8581bf74c..c9743a7c71f78 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -5140,11 +5140,26 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) /* gsharedvt */ /* Use the all-vt shared method since this is what was AOTed */ shared = mini_get_shared_method_full (method, SHARE_MODE_GSHAREDVT, error); + if (shared && !mono_method_metadata_has_header (shared)) { + MonoMethod *wrapper = replace_generated_method (shared, error); + mono_error_assert_ok (error); + if (wrapper != NULL) { + shared = mini_get_shared_method_full (wrapper, SHARE_MODE_GSHAREDVT, error); + return_val_if_nok (error, NULL); + + if (shared) { + method = wrapper; + } + } + } + if (!shared) return NULL; method_index = find_aot_method (shared, &amodule); if (method_index != 0xffffff) { + // XXX AK: I don't understand why we call mini_get_shared_method_full twice + // in the gshared case, above, we just say method = shared, which seems right. method = mini_get_shared_method_full (method, SHARE_MODE_GSHAREDVT, error); if (!method) return NULL; From c244dd2bffb78cd799d5b8256f364481ebe1f068 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 9 May 2024 14:19:04 -0400 Subject: [PATCH 22/30] better comments; remove fied FIXMEs --- src/mono/mono/metadata/marshal.c | 65 +++++++------------ src/mono/mono/metadata/method-builder-ilgen.c | 5 +- 2 files changed, 28 insertions(+), 42 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index ae38a98589bce..8a8ab19295f1a 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5231,9 +5231,9 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf WrapperInfo *info; /* generic_wrapper == TRUE means we will create a generic wrapper method. */ gboolean generic_wrapper = FALSE; - /* is_inflated == TRUE means we will inflate the wrapper method - the */ + /* is_inflated == TRUE means we will inflate a wrapper method before returning. */ gboolean is_inflated = FALSE; - /* is_generic and is_inflated might be set independently, depending on how we're called. */ + /* one or both of generic_wrapper or is_inflated might be set, depending on how we're called. */ if (member_name == NULL && kind != MONO_UNSAFE_ACCESSOR_CTOR) member_name = accessor_method->name; @@ -5264,43 +5264,33 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf } if (is_inflated) { - // FIXME: this always tries to compile a generic version of the accessor method and + // TODO: this always tries to compile a generic version of the accessor method and // then inflate it. But maybe we dont' want to do that (particularly for field // accessors). In particular if there is no method_inst, we're looking at an // accessor method inside a generic class (alwayst a ginst? or sometimes a gtd?) // In that case we might just want to compile the instance. - if (!generic_wrapper) { - orig_method = accessor_method; - ctx = &((MonoMethodInflated*)accessor_method)->context; - accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; - container = mono_method_get_generic_container (accessor_method); - if (!container) - container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? - g_assert (container); - } else { - // FIXME: - // in this case do we need to mess with the context and container? - // - // class C { - // public static extern void AccessorMethod(List t, List u); - // } - // - // when we need to make a wrapper - // - // public static extern void wrapper_AccessorMethod(List, List u); - // - // where we substitute the new gparams of the wrapper, but leave the - // gparams of C unchanged. - // - orig_method = accessor_method; - ctx = &((MonoMethodInflated*)accessor_method)->context; - accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; - container = mono_method_get_generic_container (accessor_method); - if (!container) - container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try? - g_assert (container); - } + orig_method = accessor_method; + ctx = &((MonoMethodInflated*)accessor_method)->context; + accessor_method = ((MonoMethodInflated*)accessor_method)->declaring; + container = mono_method_get_generic_container (accessor_method); + if (!container) + container = mono_class_try_get_generic_container (accessor_method->klass); + g_assert (container); + // TODO: + // in the example below, do we need to mess with the context and container? + // + // class C { + // public static extern void AccessorMethod(List t, List u); + // } + // + // when we make a wrapper + // + // public static extern void wrapper_AccessorMethod(List, List u); + // + // do we need to substitute the new gparams of the wrapper, but leave the + // gparams of C unchanged? + // } // printf("work on: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0); @@ -5351,7 +5341,6 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf get_marshal_cb ()->mb_inflate_wrapper_data (mb); } - // TODO: pass container, too get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, is_inflated, kind, member_name); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_UNSAFE_ACCESSOR); @@ -5363,12 +5352,6 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf MonoMethod *def; def = mono_mb_create_and_cache_full (cache, accessor_method, mb, sig, sig->param_count + 16, info, NULL); res = cache_generic_wrapper (cache, orig_method, def, ctx, orig_method); - // FIXME: this right here is where things went bad when we inflated the generic - // wrapper. The reason is because all the MonoMethodWrapper:method_data is still - // refering to the generic class. But in the JIT case we don't have generic - // sharing, and so none of that stuff will get substituted, probably. - - // Why does this work for class context generic wrappers? } else { res = mono_mb_create_and_cache_full (cache, accessor_method, mb, sig, sig->param_count + 16, info, NULL); } diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index 1e0b1060396e9..292baf1de7082 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -738,7 +738,10 @@ mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *met return FALSE; MonoClass *inflated_class = mono_class_from_mono_type_internal (inflated_type); - // TODO: EnC metadata-update + // TODO: EnC metadata-update. But note: + // error ENC0025: Adding an extern method requires restarting the application. + // So for UnsafeAccessor methods we don't need to handle this. But if we + // have other kinds of generic wrappers, it might become an issue. g_assert (!m_field_is_from_update (field)); int i = GPTRDIFF_TO_INT (field - m_class_get_fields (m_field_get_parent (field))); gpointer dummy = NULL; From 2ff358bab287e20186666cb03f93633537841467 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 9 May 2024 15:37:43 -0400 Subject: [PATCH 23/30] update HelloWorld sample to support either normal AOT or FullAOT --- src/mono/sample/HelloWorld/HelloWorld.csproj | 10 +++++++--- src/mono/sample/HelloWorld/Makefile | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/mono/sample/HelloWorld/HelloWorld.csproj b/src/mono/sample/HelloWorld/HelloWorld.csproj index 07bf3acd09ee7..4f897f0fc5627 100644 --- a/src/mono/sample/HelloWorld/HelloWorld.csproj +++ b/src/mono/sample/HelloWorld/HelloWorld.csproj @@ -5,7 +5,10 @@ $(NetCoreAppCurrent) true - true + true + true + normal + full + LLVMPath="$(MonoAotCrossDir)" + Mode="$(SampleAOTMode)" + > diff --git a/src/mono/sample/HelloWorld/Makefile b/src/mono/sample/HelloWorld/Makefile index 3a40630813a65..441c0ee488c25 100644 --- a/src/mono/sample/HelloWorld/Makefile +++ b/src/mono/sample/HelloWorld/Makefile @@ -5,7 +5,9 @@ DOTNET_Q_ARGS=--nologo -v:q -consoleloggerparameters:NoSummary MONO_CONFIG?=Debug MONO_ARCH?=$(shell . $(TOP)eng/common/native/init-os-and-arch.sh && echo $${arch}) TARGET_OS?=$(shell . $(TOP)eng/common/native/init-os-and-arch.sh && echo $${os}) -AOT?=true +AOT?=false +FULL_AOT?=false +TRIM?=false USE_LLVM?=false StripILCode?=false TrimmingEligibleMethodsOutputDirectory?= # @@ -13,7 +15,7 @@ TrimmingEligibleMethodsOutputDirectory?= # #MIBC_PROFILE_PATH= MONO_ENV_OPTIONS ?= -ifeq ($(AOT),true) +ifeq ($(FULL_AOT),true) MONO_ENV_OPTIONS += --full-aot endif @@ -21,7 +23,9 @@ publish: $(DOTNET) publish \ -c $(MONO_CONFIG) \ -r $(TARGET_OS)-$(MONO_ARCH) \ - /p:RunAOTCompilation=$(AOT) \ + /p:SampleUseAOT=$(AOT) \ + /p:SampleUseFullAOT=$(FULL_AOT) \ + /p:SampleTrim=$(TRIM) \ /p:MonoEnableLLVM=$(USE_LLVM) \ /p:StripILCode=$(StripILCode) \ /p:TrimmingEligibleMethodsOutputDirectory=$(TrimmingEligibleMethodsOutputDirectory) \ From 25f9e99297288f77b43a57d95cb3dc8c4fcd9c0e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 9 May 2024 15:39:13 -0400 Subject: [PATCH 24/30] revert HelloWorld sample code changes --- src/mono/sample/HelloWorld/Program.cs | 112 +++----------------------- 1 file changed, 9 insertions(+), 103 deletions(-) diff --git a/src/mono/sample/HelloWorld/Program.cs b/src/mono/sample/HelloWorld/Program.cs index e87371c54ac1f..0a65da4203a6d 100644 --- a/src/mono/sample/HelloWorld/Program.cs +++ b/src/mono/sample/HelloWorld/Program.cs @@ -2,112 +2,18 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Runtime.CompilerServices; -#if false - -public class MyBox -{ - private T _value; - public T Value { get => _value; private set { _value = value; }} - public MyBox(T inp) { Value = inp; } -} - -public class SimpleBox +namespace HelloWorld { - private decimal _value; - public decimal Value { get => _value; private set { _value = value;}} - public SimpleBox(decimal inp) { Value = inp;} -} - -public class Program -{ - public static void Main() + internal class Program { - var sbox = new SimpleBox((decimal)-5); - RunSimple(sbox, (decimal)20); - Console.WriteLine ("20 = {0}", sbox.Value); - var box = new MyBox("xyz"); - RunItAgain(box, "hjk"); - Console.WriteLine ("hjk = {0}", box.Value); - var box2 = new MyBox((decimal)-2); - RunItAgain(box2, (decimal)10.0); - Console.WriteLine (box2.Value); - RunIt(box, "abc"); - Console.WriteLine ("abc = {0}", box.Value); - } - - [MethodImpl(MethodImplOptions.NoInlining)] - public static void RunIt (MyBox dest, H input) - { - ref H boxWriter = ref AccessBox(dest); - boxWriter = input; - } - - [MethodImpl(MethodImplOptions.NoInlining)] - public static void RunItAgain (MyBox dest, S input) - { - ref S boxWriter = ref AccessHelper.AccessBox2(dest); - boxWriter = input; - } - - [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] - private static extern ref W AccessBox(MyBox x); - - [MethodImpl(MethodImplOptions.NoInlining)] - private static void RunSimple(SimpleBox s, decimal input) - { - ref decimal boxWriter = ref SimpleHelper.AccessSimpleBox(s); - boxWriter = input; - } -} - -public class AccessHelper -{ -#if false - [MethodImpl(MethodImplOptions.NoInlining)] - public static /*extern*/ ref Q AccessBox2(MyBox q) => throw new NotImplementedException("exn"); -#else - [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] - public static extern ref Q AccessBox2(MyBox q); -#endif -} - -public class SimpleHelper -{ - [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_value")] - public static extern ref decimal AccessSimpleBox(SimpleBox b); -} - -#else - -public class MyService -{ - public MyService() {} - - private static string Explain(T arg) => arg.ToString() + " : " + typeof(T).ToString(); //$"{arg} : {typeof(T)}"; -} - -public class Program -{ - public static void Main() + private static void Main(string[] args) { - new Program().Runner("abcd"); + bool isMono = typeof(object).Assembly.GetType("Mono.RuntimeStructs") != null; + Console.WriteLine($"Hello World {(isMono ? "from Mono!" : "from CoreCLR!")}"); + Console.WriteLine(typeof(object).Assembly.FullName); + Console.WriteLine(System.Reflection.Assembly.GetEntryAssembly ()); + Console.WriteLine(System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription); } - - [MethodImpl(MethodImplOptions.NoInlining)] - public void Runner(Q x) - { - Console.WriteLine (AccessHelper.CallExplain(default, x)); - } - -} - -public static class AccessHelper -{ - [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name="Explain")] - public static extern string CallExplain(MyService target, W arg); - + } } - -#endif From 0374f2bfd94dc3f348034b489c8db419972355f0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 9 May 2024 16:04:10 -0400 Subject: [PATCH 25/30] rename helper methods --- src/mono/mono/mini/aot-compiler.c | 4 ++-- src/mono/mono/mini/aot-runtime.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 09f7422ffe72e..ae4ba45c4372e 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -4833,7 +4833,7 @@ mono_aot_can_enter_interp (MonoMethod *method) } static MonoMethod* -instantiate_wrapper_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +inflate_unsafe_accessor_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) { g_assert (extern_method_inst->is_inflated); MonoMethodInflated *infl = (MonoMethodInflated*)extern_method_inst; @@ -4868,7 +4868,7 @@ replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *e if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { MonoMethod *wrapper = NULL; if (method->is_inflated) { - wrapper = instantiate_wrapper_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); + wrapper = inflate_unsafe_accessor_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); } else { wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); } diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index c9743a7c71f78..544750f8e6c0a 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -942,7 +942,7 @@ typedef struct { } MethodRef; static MonoMethod* -instantiate_unsafe_accessor_wrapper (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +inflate_unsafe_accessor_wrapper (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) { MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); @@ -1096,7 +1096,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod MonoGenericContext ctx = {0,}; decode_generic_context (module, &ctx, p, &p, error); mono_error_assert_ok (error); - ref->method = instantiate_unsafe_accessor_wrapper (m, &ctx, kind, member_name, error); + ref->method = inflate_unsafe_accessor_wrapper (m, &ctx, kind, member_name, error); if (!is_ok (error)) return FALSE; } else { @@ -4610,7 +4610,7 @@ inst_is_private (MonoGenericInst *inst) } static MonoMethod* -instantiate_wrapper_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +inflate_unsafe_accessor_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) { g_assert (extern_method_inst->is_inflated); MonoMethodInflated *infl = (MonoMethodInflated*)extern_method_inst; @@ -4635,7 +4635,7 @@ replace_generated_method (MonoMethod *method, MonoError *error) if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { MonoMethod *wrapper = NULL; if (method->is_inflated) { - wrapper = instantiate_wrapper_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); + wrapper = inflate_unsafe_accessor_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); } else { wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); } From 9786200a99ce4f2079a7912b0185a401a9684de2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 10 May 2024 17:28:34 -0400 Subject: [PATCH 26/30] apply suggestions from code review --- src/mono/mono/metadata/marshal-lightweight.c | 12 ++---------- src/mono/mono/metadata/marshal.c | 3 --- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index eb10da0647bb0..45d0966be68f6 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2295,9 +2295,6 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflat gboolean inflate_generic_data = FALSE; if ((accessor_method->is_generic || to_be_inflated)) { - // We want to do a static lookup: the target type must be G not just !T or !!T, - // so that we can get a MonoClassField at compile time - g_assert (target_type->type != MONO_TYPE_VAR && target_type->type != MONO_TYPE_MVAR); // If the target is generic, we'll find a generic MonoClassField and then mark it to be // inflated in order to handle instances, or gshared/gsharedvt instances. inflate_generic_data = TRUE; @@ -2438,11 +2435,8 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflate gboolean inflate_generic_data = FALSE; if ((accessor_method->is_generic || to_be_inflated)) { - // We want to do a static lookup: target type must be G, not just !T or !!T - g_assert (target_type->type != MONO_TYPE_VAR && target_type->type != MONO_TYPE_MVAR); // If the target is generic, we'll find a generic MonoMethod* and then mark it to be // inflated in order to handle instances, or gshared/gsharedvt instances. - inflate_generic_data = TRUE; }; @@ -2452,7 +2446,7 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflate MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig); - MonoClass *in_class = target_class; // mono_class_get_generic_type_definition (target_class); + MonoClass *in_class = target_class; MonoMethod *target_method = mono_unsafe_accessor_find_ctor (in_class, member_sig, target_class, find_method_error); if (!is_ok (find_method_error) || target_method == NULL) { @@ -2496,8 +2490,6 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_infla gboolean inflate_generic_data = FALSE; if ((accessor_method->is_generic || to_be_inflated)) { - // We want to do a static lookup: target type must be G, not just !T or !!T - g_assert (target_type->type != MONO_TYPE_VAR && target_type->type != MONO_TYPE_MVAR); inflate_generic_data = TRUE; }; @@ -2513,7 +2505,7 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_infla MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig); - MonoClass *in_class = target_class; // mono_class_get_generic_type_definition (target_class); + MonoClass *in_class = target_class; MonoMethod *target_method = NULL; if (!ctor_as_method) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 8a8ab19295f1a..03657e11d58bb 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5257,9 +5257,6 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf } if (accessor_method->is_generic) { - // FullAOT compilation with gshared or gsharedvt will get here. - /* got a generic method definition. need to make a generic method definition wrapper */ - g_assert (!accessor_method->is_inflated); generic_wrapper = TRUE; } From ceb534578edb6f2d3a2d3f59cceaeb96bbef8fc1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 13 May 2024 09:43:49 -0400 Subject: [PATCH 27/30] DRY. compute inflate_generic_data in one place --- src/mono/mono/metadata/marshal-lightweight.c | 40 ++++---------------- src/mono/mono/metadata/marshal.c | 5 ++- src/mono/mono/metadata/marshal.h | 2 +- 3 files changed, 12 insertions(+), 35 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 45d0966be68f6..24aaeebc33ba4 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2279,7 +2279,7 @@ static gboolean unsafe_accessor_target_type_forbidden (MonoType *target_type); static void -emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflated, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { // Field access requires a single argument for target type and a return type. g_assert (kind == MONO_UNSAFE_ACCESSOR_FIELD || kind == MONO_UNSAFE_ACCESSOR_STATIC_FIELD); @@ -2293,13 +2293,6 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflat return; } - gboolean inflate_generic_data = FALSE; - if ((accessor_method->is_generic || to_be_inflated)) { - // If the target is generic, we'll find a generic MonoClassField and then mark it to be - // inflated in order to handle instances, or gshared/gsharedvt instances. - inflate_generic_data = TRUE; - }; - MonoType *ret_type = sig->ret; MonoClass *target_class = mono_class_from_mono_type_internal (target_type); @@ -2415,7 +2408,7 @@ inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_metho } static void -emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflated, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { g_assert (kind == MONO_UNSAFE_ACCESSOR_CTOR); // null or empty string member name is ok for a constructor @@ -2433,13 +2426,6 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflate return; } - gboolean inflate_generic_data = FALSE; - if ((accessor_method->is_generic || to_be_inflated)) { - // If the target is generic, we'll find a generic MonoMethod* and then mark it to be - // inflated in order to handle instances, or gshared/gsharedvt instances. - inflate_generic_data = TRUE; - }; - MonoClass *target_class = mono_class_from_mono_type_internal (target_type); ERROR_DECL(find_method_error); @@ -2471,7 +2457,7 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflate } static void -emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_inflated, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { g_assert (kind == MONO_UNSAFE_ACCESSOR_METHOD || kind == MONO_UNSAFE_ACCESSOR_STATIC_METHOD); g_assert (member_name != NULL); @@ -2479,8 +2465,6 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_infla // We explicitly allow calling a constructor as if it was an instance method, but we need some hacks in a couple of places gboolean ctor_as_method = !strcmp (member_name, ".ctor"); - - MonoType *target_type = sig->param_count >= 1 ? sig->params[0] : NULL; if (sig->param_count < 1 || target_type == NULL || unsafe_accessor_target_type_forbidden (target_type)) { @@ -2488,11 +2472,6 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_infla return; } - gboolean inflate_generic_data = FALSE; - if ((accessor_method->is_generic || to_be_inflated)) { - inflate_generic_data = TRUE; - }; - gboolean hasthis = kind == MONO_UNSAFE_ACCESSOR_METHOD; MonoClass *target_class = mono_class_from_mono_type_internal (target_type); @@ -2539,13 +2518,8 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean to_be_infla } static void -emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, gboolean to_be_inflated, MonoUnsafeAccessorKind kind, const char *member_name) +emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { - // to_be_inflated means we're emitting a non-generic accessor method belonging to a gtd - // that will be inflated by marshal.c - if (to_be_inflated && !accessor_method->is_generic) - g_assert (!accessor_method->is_inflated); - if (!m_method_is_static (accessor_method)) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic"); return; @@ -2554,14 +2528,14 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ switch (kind) { case MONO_UNSAFE_ACCESSOR_FIELD: case MONO_UNSAFE_ACCESSOR_STATIC_FIELD: - emit_unsafe_accessor_field_wrapper (mb, to_be_inflated, accessor_method, sig, kind, member_name); + emit_unsafe_accessor_field_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_CTOR: - emit_unsafe_accessor_ctor_wrapper (mb, to_be_inflated, accessor_method, sig, kind, member_name); + emit_unsafe_accessor_ctor_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name); return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: - emit_unsafe_accessor_method_wrapper (mb, to_be_inflated, accessor_method, sig, kind, member_name); + emit_unsafe_accessor_method_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name); return; default: mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_InvalidKindValue"); diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 03657e11d58bb..f82f8cd69fe25 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5338,7 +5338,10 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf get_marshal_cb ()->mb_inflate_wrapper_data (mb); } - get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, is_inflated, kind, member_name); + // if the wrapper will be inflated (either now by us, or when it's called, because it is + // generic), mark the wrapper data to be inflated, too + gboolean inflate_generic_data = accessor_method->is_generic || is_inflated; + get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_UNSAFE_ACCESSOR); info->d.unsafe_accessor.method = accessor_method; diff --git a/src/mono/mono/metadata/marshal.h b/src/mono/mono/metadata/marshal.h index 61b006506c493..d55fd6757d18b 100644 --- a/src/mono/mono/metadata/marshal.h +++ b/src/mono/mono/metadata/marshal.h @@ -342,7 +342,7 @@ typedef struct { void (*emit_synchronized_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method); void (*emit_unbox_wrapper) (MonoMethodBuilder *mb, MonoMethod *method); void (*emit_array_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *sig, MonoGenericContext *ctx); - void (*emit_unsafe_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, gboolean to_be_inflated, MonoUnsafeAccessorKind kind, const char *member_name); + void (*emit_unsafe_accessor_wrapper) (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name); void (*emit_generic_array_helper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig); void (*emit_thunk_invoke_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig); void (*emit_create_string_hack) (MonoMethodBuilder *mb, MonoMethodSignature *csig, MonoMethod *res); From 342fe1d58625941e352724942102e23d7e6fd515 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 13 May 2024 10:10:52 -0400 Subject: [PATCH 28/30] Just do one loop for inflating generic wrapper data --- src/mono/mono/metadata/method-builder-ilgen.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index 292baf1de7082..6b387696435bc 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -721,12 +721,10 @@ mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *met { MonoMethodBuilderInflateWrapperData* inflate_info = (MonoMethodBuilderInflateWrapperData*)method_data[MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX]; MonoMethodBuilderInflateWrapperData* p = inflate_info; - int count = 0; - while (p->idx != 0) {p++; count++;} - for (int x = 0; x < count; x++) { - int idx = inflate_info[x].idx; - uint16_t kind = inflate_info[x].kind; + while (p && p->idx != 0) { + int idx = p->idx; + uint16_t kind = p->kind; gpointer *pdata = &method_data[idx]; switch (kind) { case MONO_MB_ILGEN_WRAPPER_DATA_NONE: @@ -740,8 +738,11 @@ mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *met MonoClass *inflated_class = mono_class_from_mono_type_internal (inflated_type); // TODO: EnC metadata-update. But note: // error ENC0025: Adding an extern method requires restarting the application. - // So for UnsafeAccessor methods we don't need to handle this. But if we - // have other kinds of generic wrappers, it might become an issue. + // + // So for UnsafeAccessor methods we don't need to handle this + // until https://github.com/dotnet/runtime/issues/102080 + // + // But if we have other kinds of generic wrappers, we may need to do it sooner. g_assert (!m_field_is_from_update (field)); int i = GPTRDIFF_TO_INT (field - m_class_get_fields (m_field_get_parent (field))); gpointer dummy = NULL; @@ -767,6 +768,7 @@ mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *met default: g_assert_not_reached(); } + p++; } return TRUE; } From a667f53a35bc08e52ca5e0a7ebcfcabc4d2967d1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 13 May 2024 11:13:14 -0400 Subject: [PATCH 29/30] better comments --- src/mono/mono/metadata/method-builder-ilgen-internals.h | 5 ++--- src/mono/mono/metadata/method-builder-ilgen.c | 8 ++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/metadata/method-builder-ilgen-internals.h b/src/mono/mono/metadata/method-builder-ilgen-internals.h index c66ace69b8cf9..74538a7d5a1f2 100644 --- a/src/mono/mono/metadata/method-builder-ilgen-internals.h +++ b/src/mono/mono/metadata/method-builder-ilgen-internals.h @@ -50,9 +50,8 @@ enum MonoILGenWrapperDataKind MONO_MB_ILGEN_WRAPPER_DATA_METHOD, }; -// index in MonoMethodWrapper:wrapper_data where the inflate info will be stored -// notes: -// - idx 1 is the wrapper info (see mono_method_get_wrapper_info) +// index in MonoMethodWrapper:wrapper_data where the inflate info will be stored. +// note: idx 1 is the wrapper info (see mono_marshal_get_wrapper_info) #define MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX 2 #endif diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index 6b387696435bc..47f8a6d6cb54f 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -257,8 +257,10 @@ create_method_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *signature, int g_assert (!container->is_anonymous); g_assert (container->is_method); g_assert (container->owner.method == mb->method); - // HACK: reassign container owner from the method builder placeholder to the - // final created method + // NOTE: reassigning container owner from the method builder placeholder to the + // final created method. The "proper" way to do this would be a deep copy or + // calling mono_metadata_load_generic_params again and inflating everything. But + // method builder is already one-shot, so this is ok, too. container->owner.method = method; } @@ -749,6 +751,8 @@ mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *met mono_metadata_free_type (inflated_type); + // note: inflated class might not have been used for much yet. Ensure + // fields are initialized. mono_class_get_fields_internal (inflated_class, &dummy); g_assert (m_class_get_fields (inflated_class)); From 0bf0f6163957b3707e70d34fbaffc2a257aab948 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 13 May 2024 15:13:57 -0400 Subject: [PATCH 30/30] DRY. move common AOT code to mini.c --- src/mono/mono/mini/aot-compiler.c | 50 ++------------------------ src/mono/mono/mini/aot-runtime.c | 60 +++---------------------------- src/mono/mono/mini/mini.c | 59 ++++++++++++++++++++++++++++++ src/mono/mono/mini/mini.h | 6 ++++ 4 files changed, 71 insertions(+), 104 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index ae4ba45c4372e..2908bf14a88fd 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -4832,63 +4832,17 @@ mono_aot_can_enter_interp (MonoMethod *method) return FALSE; } -static MonoMethod* -inflate_unsafe_accessor_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) -{ - g_assert (extern_method_inst->is_inflated); - MonoMethodInflated *infl = (MonoMethodInflated*)extern_method_inst; - MonoMethod *extern_decl = infl->declaring; - MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); - MonoGenericContext *ctx = &infl->context; - MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); - return inflated_wrapper; -} - -/** - * Replaces some extern \c method by a wrapper. - * - * Unsafe accessor methods are static extern methods with no header. Calls to - * them are replaced by calls to a wrapper. So during AOT compilation when we - * collect methods to AOT, we replace these methods by the wrappers, too. - * - * Returns the wrapper method, or \c NULL if it doesn't need to be replaced. - * On error returns NULL and sets \c error. - */ static MonoMethod* replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *error) { - if (G_LIKELY (mono_method_metadata_has_header (method))) - return NULL; - - /* Unsafe accessors methods. Replace attempts to compile the accessor method by - * its wrapper. - */ - char *member_name = NULL; - int accessor_kind = -1; - if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - MonoMethod *wrapper = NULL; - if (method->is_inflated) { - wrapper = inflate_unsafe_accessor_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); - } else { - wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); - } - if (is_ok (error)) { - if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_AOT)) { - char * method_name = mono_method_get_full_name (wrapper); - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_AOT, "Replacing generated method by %s", method_name); - g_free (method_name); - } - return wrapper; - } - } - + MonoMethod *wrapper = mini_replace_generated_method (method, error); if (!is_ok (error)) { char *method_name = mono_method_get_full_name (method); aot_printerrf (acfg, "Could not get generated wrapper for %s due to %s", method_name, mono_error_get_message (error)); g_free (method_name); } - return NULL; + return wrapper; } static void diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index 544750f8e6c0a..527247809c7d5 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -941,14 +941,6 @@ typedef struct { gboolean no_aot_trampoline; } MethodRef; -static MonoMethod* -inflate_unsafe_accessor_wrapper (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) -{ - MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); - MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); - return inflated_wrapper; -} - /* * decode_method_ref_with_target: * @@ -1096,7 +1088,7 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod MonoGenericContext ctx = {0,}; decode_generic_context (module, &ctx, p, &p, error); mono_error_assert_ok (error); - ref->method = inflate_unsafe_accessor_wrapper (m, &ctx, kind, member_name, error); + ref->method = mini_inflate_unsafe_accessor_wrapper (m, &ctx, kind, member_name, error); if (!is_ok (error)) return FALSE; } else { @@ -4609,50 +4601,6 @@ inst_is_private (MonoGenericInst *inst) return FALSE; } -static MonoMethod* -inflate_unsafe_accessor_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) -{ - g_assert (extern_method_inst->is_inflated); - MonoMethodInflated *infl = (MonoMethodInflated*)extern_method_inst; - MonoMethod *extern_decl = infl->declaring; - MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); - MonoGenericContext *ctx = &infl->context; - MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); - return inflated_wrapper; -} - -static MonoMethod* -replace_generated_method (MonoMethod *method, MonoError *error) -{ - if (G_LIKELY (mono_method_metadata_has_header (method))) - return NULL; - - /* Unsafe accessors methods. Replace attempts to compile the accessor method by - * its wrapper. - */ - char *member_name = NULL; - int accessor_kind = -1; - if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - MonoMethod *wrapper = NULL; - if (method->is_inflated) { - wrapper = inflate_unsafe_accessor_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); - } else { - wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); - } - if (is_ok (error)) { - if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_AOT)) { - char * method_name = mono_method_get_full_name (wrapper); - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_AOT, "Replacing generated method by %s", method_name); - g_free (method_name); - } - return wrapper; - } - } - - return NULL; -} - - gboolean mono_aot_can_dedup (MonoMethod *method) { @@ -5122,7 +5070,7 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) method = shared; if (method_index == 0xffffff && !mono_method_metadata_has_header (method)) { // replace lookups for unsafe accessor instances by lookups of the wrapper - MonoMethod *wrapper = replace_generated_method (method, error); + MonoMethod *wrapper = mini_replace_generated_method (method, error); mono_error_assert_ok (error); if (wrapper != NULL) { shared = mini_get_shared_method_full (wrapper, SHARE_MODE_NONE, error); @@ -5141,7 +5089,7 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) /* Use the all-vt shared method since this is what was AOTed */ shared = mini_get_shared_method_full (method, SHARE_MODE_GSHAREDVT, error); if (shared && !mono_method_metadata_has_header (shared)) { - MonoMethod *wrapper = replace_generated_method (shared, error); + MonoMethod *wrapper = mini_replace_generated_method (shared, error); mono_error_assert_ok (error); if (wrapper != NULL) { shared = mini_get_shared_method_full (wrapper, SHARE_MODE_GSHAREDVT, error); @@ -5168,7 +5116,7 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) if (method_index == 0xffffff && !mono_method_metadata_has_header (method)) { // replace lookups for unsafe accessor instances by lookups of the wrapper - MonoMethod *wrapper = replace_generated_method (method, error); + MonoMethod *wrapper = mini_replace_generated_method (method, error); mono_error_assert_ok (error); if (wrapper != NULL) { method_index = find_aot_method (wrapper, &amodule); diff --git a/src/mono/mono/mini/mini.c b/src/mono/mono/mini/mini.c index b467c2d446620..395a869ab22be 100644 --- a/src/mono/mono/mini/mini.c +++ b/src/mono/mono/mini/mini.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -4613,3 +4614,61 @@ mini_get_simd_type_info (MonoClass *klass, guint32 *nelems) } } +MonoMethod* +mini_inflate_unsafe_accessor_wrapper (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +{ + MonoMethod *generic_wrapper = mono_marshal_get_unsafe_accessor_wrapper (extern_decl, accessor_kind, member_name); + MonoMethod *inflated_wrapper = mono_class_inflate_generic_method_checked (generic_wrapper, ctx, error); + return inflated_wrapper; +} + + +static MonoMethod* +inflate_unsafe_accessor_like_decl (MonoMethod *extern_method_inst, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error) +{ + g_assert (extern_method_inst->is_inflated); + MonoMethodInflated *infl = (MonoMethodInflated*)extern_method_inst; + MonoMethod *extern_decl = infl->declaring; + MonoGenericContext *ctx = &infl->context; + return mini_inflate_unsafe_accessor_wrapper (extern_decl, ctx, accessor_kind, member_name, error); +} + +/** + * Replaces some extern \c method by a wrapper. + * + * Unsafe accessor methods are static extern methods with no header. Calls to + * them are replaced by calls to a wrapper. So during AOT compilation when we + * collect methods to AOT, we replace these methods by the wrappers, too. + * + * Returns the wrapper method, or \c NULL if it doesn't need to be replaced. + * On error returns NULL and sets \c error. + */ +MonoMethod* +mini_replace_generated_method (MonoMethod *method, MonoError *error) +{ + if (G_LIKELY (mono_method_metadata_has_header (method))) + return NULL; + + /* Unsafe accessors methods. Replace attempts to compile the accessor method by + * its wrapper. + */ + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + MonoMethod *wrapper = NULL; + if (method->is_inflated) { + wrapper = inflate_unsafe_accessor_like_decl (method, (MonoUnsafeAccessorKind)accessor_kind, member_name, error); + } else { + wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + } + if (is_ok (error)) { + if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_AOT)) { + char * method_name = mono_method_get_full_name (wrapper); + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_AOT, "Replacing generated method by %s", method_name); + g_free (method_name); + } + return wrapper; + } + } + return NULL; +} diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index 8a8c3dde3c360..215abfee0fb5f 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -3033,4 +3033,10 @@ MonoMemoryManager* mini_get_default_mem_manager (void); MONO_COMPONENT_API int mono_wasm_get_debug_level (void); +MonoMethod* +mini_inflate_unsafe_accessor_wrapper (MonoMethod *extern_decl, MonoGenericContext *ctx, MonoUnsafeAccessorKind accessor_kind, const char *member_name, MonoError *error); + +MonoMethod * +mini_replace_generated_method (MonoMethod *method, MonoError *error); + #endif /* __MONO_MINI_H__ */