From 2385d08f74c1e2763ab9937334ab97b66b5069dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Mon, 13 May 2024 17:49:22 -0400 Subject: [PATCH] [mono] generic wrapper methods for unsafe accessors (#101732) 1. Adds support for compiling generic wrapper methods to Mono. In some situations we need to emit a wrapper method that is generic. This makes the MethodBuilder infrastructure support that. 2. Adds support for inflating generic wrapper data. Wrappers have pointer data associated with them that is used by the code generator (For example instead of emitting field tokens, we record the `MonoClassField*` directly and then emit a fake token that is just the index of the `MonoClassField*` in the `MonoMethodWrapper:method_data` array). The pointer data associated with a wrapper is normally just consumed verbatim. But if the wrapper is part of a generic class, or if the wrapper is a generic method, the wrapper data might have generic parameters (for example it might be a MonoClassField for `MyList` instead of `MyList`). Add support for tagging the data with its kind and inflating it if the wrapper method is inflated. 3. Applies (1) and (2) to unsafe accessor methods - the unsafe accesor wrapper generation now always tries to get the generic definition and to generate a wrapper for that generic definition and then inflate it. 4. Some AOT changes so that FullAOT substitutes lookups for an unsafe accessor by lookups for the wrapper. Including if the unsafe accessor or wrapper is generic. This also enabled gshared and gsharedvt for unsafe accessor wrappers. This also fixes https://github.com/dotnet/runtime/issues/92883 Contributes to https://github.com/dotnet/runtime/issues/99830, https://github.com/dotnet/runtime/issues/89439 **NOT DONE** - We don't check constraints on the generic target types yet --- * always AOT wrappers, even for generics, not the actual accessor * add generic wrapper methods * use generic method owner caches * 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 * cleanup marshaling - dont' use ctx as a flag * 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. * issues.targets: enable some unsafe accessor AOT tests * [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 * [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" * inflate MonoMethod wrapper data; impl ctor generic unsafe accessors * fix windows build * [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 * [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. * [aot-runtime] try to fix gsharedvt lookup * better comments; remove fied FIXMEs * update HelloWorld sample to support either normal AOT or FullAOT * rename helper methods * apply suggestions from code review * DRY. compute inflate_generic_data in one place * Just do one loop for inflating generic wrapper data * better comments * DRY. move common AOT code to mini.c --- src/mono/mono/metadata/class-internals.h | 1 + src/mono/mono/metadata/class.c | 12 ++ src/mono/mono/metadata/marshal-lightweight.c | 110 +++++++------- src/mono/mono/metadata/marshal.c | 106 ++++++++++++-- src/mono/mono/metadata/marshal.h | 3 +- .../metadata/method-builder-ilgen-internals.h | 19 +++ src/mono/mono/metadata/method-builder-ilgen.c | 134 +++++++++++++++++- src/mono/mono/metadata/method-builder-ilgen.h | 7 + src/mono/mono/mini/aot-compiler.c | 91 +++++++++--- src/mono/mono/mini/aot-runtime.c | 64 ++++++++- src/mono/mono/mini/aot-runtime.h | 2 +- src/mono/mono/mini/mini.c | 59 ++++++++ src/mono/mono/mini/mini.h | 6 + src/mono/sample/HelloWorld/HelloWorld.csproj | 9 +- src/mono/sample/HelloWorld/Makefile | 11 +- src/tests/issues.targets | 3 - 16 files changed, 533 insertions(+), 104 deletions(-) diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index e7b56c72a38e2b..b423350d298107 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 2209f0f88a1e6e..ec0ddbe2f85aa6 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 4094d8fbcb1e83..24aaeebc33ba47 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) { @@ -2265,20 +2275,26 @@ 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, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, 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); g_assert (member_name != NULL); - MonoType *target_type = sig->params[0]; // params[0] is the field's parent - MonoType *ret_type = sig->ret; - if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID) { + + MonoType *target_type = sig->param_count == 1 ? sig->params[0] : NULL; // params[0] is the field's parent + + if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID || unsafe_accessor_target_type_forbidden (target_type)) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute."); return; } + MonoType *ret_type = sig->ret; + MonoClass *target_class = mono_class_from_mono_type_internal (target_type); gboolean target_byref = m_type_is_byref (target_type); gboolean target_valuetype = m_class_is_valuetype (target_class); @@ -2307,6 +2323,8 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ 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 (inflate_generic_data) + mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_FIELD); mono_mb_emit_byte (mb, CEE_RET); } @@ -2315,7 +2333,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 +2350,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 */ @@ -2373,28 +2391,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) { @@ -2412,7 +2408,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, 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 @@ -2424,22 +2420,19 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m } MonoType *target_type = sig->ret; // for constructors the return type is the target type - 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; - } if (target_type == NULL || m_type_is_byref (target_type) || unsafe_accessor_target_type_forbidden (target_type)) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute."); return; } + + MonoClass *target_class = mono_class_from_mono_type_internal (target_type); + + ERROR_DECL(find_method_error); - 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); + 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) { @@ -2458,11 +2451,13 @@ 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); } 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, 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); @@ -2470,9 +2465,13 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor // 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)) { + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute."); + return; + } - MonoType *target_type = sig->params[0]; gboolean hasthis = kind == MONO_UNSAFE_ACCESSOR_METHOD; MonoClass *target_class = mono_class_from_mono_type_internal (target_type); @@ -2482,19 +2481,10 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor } ERROR_DECL(find_method_error); - if (accessor_method->is_inflated) { - sig = update_signature(accessor_method); - target_type = sig->params[0]; - } - - 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; - } - 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); + MonoClass *in_class = target_class; MonoMethod *target_method = NULL; if (!ctor_as_method) @@ -2522,17 +2512,14 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor 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); } 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, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name) { - if (accessor_method->is_generic || ctx != NULL) { - 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; @@ -2541,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, accessor_method, sig, ctx, 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, accessor_method, sig, ctx, 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, accessor_method, sig, ctx, 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"); @@ -3404,6 +3391,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.c b/src/mono/mono/metadata/marshal.c index aad740a31d90e5..f82f8cd69fe255 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5226,39 +5226,129 @@ 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; + /* generic_wrapper == TRUE means we will create a generic wrapper method. */ + gboolean generic_wrapper = FALSE; + /* is_inflated == TRUE means we will inflate a wrapper method before returning. */ + gboolean is_inflated = FALSE; + /* 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; + // 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) { + generic_wrapper = TRUE; + } + + if (is_inflated) { + // 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. + + 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); + /* * Check cache */ - if (ctx) { - cache = NULL; - g_assert_not_reached (); + 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) + 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; } + // printf ("Cache miss\n"); + + mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); + if (generic_wrapper) { + // If the accessor method was generic, make the wrapper generic, too. - sig = mono_metadata_signature_dup_full (get_method_image (accessor_method), mono_method_signature_internal (accessor_method)); - sig->pinvoke = 0; + // Load a copy of the generic params of the accessor method + 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); - mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); + 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); + // 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 { + 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); - get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, ctx, kind, member_name); + 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); + } + + // 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; 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 8d545fcc25dea6..d55fd6757d18ba 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, 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); @@ -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 1aa410beb34a50..74538a7d5a1f22 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, + MONO_MB_ILGEN_WRAPPER_DATA_METHOD, +}; + +// 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 41bfc8fa6804c3..47f8a6d6cb54fa 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); @@ -214,6 +250,20 @@ 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); + // 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; + } + return method; } @@ -652,3 +702,77 @@ 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; + + 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: + 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. But note: + // error ENC0025: Adding an extern method requires restarting the application. + // + // 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; + + 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)); + + MonoClassField *inflated_field = &m_class_get_fields (inflated_class) [i]; + + *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(); + } + p++; + } + return TRUE; +} diff --git a/src/mono/mono/metadata/method-builder-ilgen.h b/src/mono/mono/metadata/method-builder-ilgen.h index 2a5e40dc70256d..209565d30282ca 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 diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index cb525d12e20d70..57edfe8b0873a6 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -3873,7 +3873,18 @@ 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); + 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); @@ -4821,6 +4832,19 @@ mono_aot_can_enter_interp (MonoMethod *method) return FALSE; } +static MonoMethod* +replace_generated_method (MonoAotCompile *acfg, MonoMethod *method, MonoError *error) +{ + 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 wrapper; +} + static void add_full_aot_wrappers (MonoAotCompile *acfg) { @@ -5311,25 +5335,6 @@ add_full_aot_wrappers (MonoAotCompile *acfg) add_method (acfg, mono_marshal_get_ptr_to_struct (klass)); } } - - /* 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_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)); - } - } - } static void @@ -6024,6 +6029,16 @@ add_generic_instances (MonoAotCompile *acfg) continue; } + 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; + } + } + if (m_class_get_image (method->klass) != acfg->image) continue; @@ -6119,6 +6134,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); @@ -9355,6 +9372,17 @@ 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)) { + ERROR_DECL(error); + MonoMethod *gen = replace_generated_method (acfg, m, error); + if (!is_ok (error)) { + mono_error_cleanup (error); + return; + } else if (gen != NULL) { + m = gen; + } + } + add_extra_method_with_depth (acfg, m, depth + 1); add_types_from_method_header (acfg, m); } @@ -12993,6 +13021,17 @@ collect_methods (MonoAotCompile *acfg) method = wrapper; } + 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. + MonoMethod *gen = replace_generated_method (acfg, method, error); + if (!is_ok (error)) { + mono_error_cleanup (error); + return FALSE; + } else if (gen != NULL) { + method = gen; + } + } + /* FIXME: Some mscorlib methods don't have debug info */ /* if (acfg->aot_opts.soft_debug && !method->wrapper_type) { @@ -13037,6 +13076,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/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index a9709afb81ad65..902c049c50f25d 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 @@ -1077,9 +1078,22 @@ 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; - ref->method = mono_marshal_get_unsafe_accessor_wrapper (m, kind, member_name); + 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,}; + decode_generic_context (module, &ctx, p, &p, error); + mono_error_assert_ok (error); + ref->method = mini_inflate_unsafe_accessor_wrapper (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) { @@ -4609,6 +4623,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) @@ -4931,6 +4949,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; /* @@ -5051,6 +5070,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 = 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); + 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)) { @@ -5058,17 +5090,43 @@ 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 = 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); + 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; } } + if (method_index == 0xffffff && !mono_method_metadata_has_header (method)) { + // replace lookups for unsafe accessor instances by lookups of the wrapper + MonoMethod *wrapper = mini_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; diff --git a/src/mono/mono/mini/aot-runtime.h b/src/mono/mono/mini/aot-runtime.h index bd666593536276..e4f84523c45e22 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/mono/mini/mini.c b/src/mono/mono/mini/mini.c index b467c2d4466205..395a869ab22bec 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 8a8c3dde3c360c..215abfee0fb5fa 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__ */ diff --git a/src/mono/sample/HelloWorld/HelloWorld.csproj b/src/mono/sample/HelloWorld/HelloWorld.csproj index cc052494b398fa..4f897f0fc5627d 100644 --- a/src/mono/sample/HelloWorld/HelloWorld.csproj +++ b/src/mono/sample/HelloWorld/HelloWorld.csproj @@ -1,9 +1,14 @@ + Exe $(NetCoreAppCurrent) true + true + true + normal + full + LLVMPath="$(MonoAotCrossDir)" + Mode="$(SampleAOTMode)" + > diff --git a/src/mono/sample/HelloWorld/Makefile b/src/mono/sample/HelloWorld/Makefile index afdfa16f3512c7..441c0ee488c25a 100644 --- a/src/mono/sample/HelloWorld/Makefile +++ b/src/mono/sample/HelloWorld/Makefile @@ -6,19 +6,26 @@ 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 +FULL_AOT?=false +TRIM?=false USE_LLVM?=false StripILCode?=false TrimmingEligibleMethodsOutputDirectory?= # #MIBC_PROFILE_PATH= -MONO_ENV_OPTIONS ?= +MONO_ENV_OPTIONS ?= +ifeq ($(FULL_AOT),true) +MONO_ENV_OPTIONS += --full-aot +endif 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) \ diff --git a/src/tests/issues.targets b/src/tests/issues.targets index f035f47f112a68..a27fbb6ac4ef8b 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2576,8 +2576,6 @@ - - @@ -3813,7 +3811,6 @@ -