From bc9d6ed76888cfdcf504fed0cc15dfa09ac99a1c Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Wed, 5 Jul 2023 16:01:32 -0400 Subject: [PATCH 01/34] Detect an UnsafeAccessorAttribute for method with interpreter --- src/mono/mono/metadata/custom-attrs.c | 42 +++++++++++++++++++ src/mono/mono/metadata/reflection-internals.h | 2 + src/mono/mono/mini/interp/transform.c | 7 ++++ 3 files changed, 51 insertions(+) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 31a0be630fa05..25195aed3ec4d 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -50,6 +50,7 @@ static gboolean type_is_reference (MonoType *type); static GENERATE_GET_CLASS_WITH_CACHE (custom_attribute_typed_argument, "System.Reflection", "CustomAttributeTypedArgument"); static GENERATE_GET_CLASS_WITH_CACHE (custom_attribute_named_argument, "System.Reflection", "CustomAttributeNamedArgument"); static GENERATE_TRY_GET_CLASS_WITH_CACHE (customattribute_data, "System.Reflection", "RuntimeCustomAttributeData"); +static GENERATE_TRY_GET_CLASS_WITH_CACHE (unsafe_accessor_attribute, "System.Runtime.CompilerServices", "UnsafeAccessorAttribute"); static MonoCustomAttrInfo* mono_custom_attrs_from_builders_handle (MonoImage *alloc_img, MonoImage *image, MonoArrayHandle cattrs, gboolean respect_cattr_visibility); @@ -2056,6 +2057,47 @@ mono_custom_attrs_from_method_checked (MonoMethod *method, MonoError *error) return mono_custom_attrs_from_index_checked (m_class_get_image (method->klass), idx, FALSE, error); } +gboolean +mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kind, char **member_name, MonoError *error) +{ + MonoCustomAttrInfo *cinfo = mono_custom_attrs_from_method_checked (method, error); + + if (!cinfo) + return FALSE; + + MonoClass *unsafeAccessor = mono_class_try_get_unsafe_accessor_attribute_class (); + MonoCustomAttrEntry *attr = NULL; + + for (int idx = 0; idx < cinfo->num_attrs; ++idx) { + MonoClass *ctor_class = cinfo->attrs [idx].ctor->klass; + if (ctor_class == unsafeAccessor) { + attr = &cinfo->attrs [idx]; + break; + } + } + + if (!attr) + return FALSE; + + MonoDecodeCustomAttr *decoded_args = mono_reflection_create_custom_attr_data_args_noalloc (m_class_get_image (attr->ctor->klass), attr->ctor, attr->data, attr->data_size, error); + + if (!is_ok (error)) { + mono_error_cleanup (error); + return FALSE; + } + + for (int i = 0; i < decoded_args->named_args_num; ++i) { + if (decoded_args->named_args_info [i].field && !strcmp (decoded_args->named_args_info [i].field->name, "Kind")) { + *accessor_kind = *(int*)decoded_args->named_args [i]->value.primitive; + }else if (decoded_args->named_args_info [i].field && !strcmp (decoded_args->named_args_info [i].field->name, "Kind")) { + *member_name = *(char**)decoded_args->named_args [i]->value.primitive; + } + } + + mono_reflection_free_custom_attr_data_args_noalloc (decoded_args); + return TRUE; +} + /** * mono_custom_attrs_from_class: */ diff --git a/src/mono/mono/metadata/reflection-internals.h b/src/mono/mono/metadata/reflection-internals.h index cb8d87a85038c..01e3d136e0aab 100644 --- a/src/mono/mono/metadata/reflection-internals.h +++ b/src/mono/mono/metadata/reflection-internals.h @@ -65,6 +65,8 @@ MonoCustomAttrInfo* mono_custom_attrs_from_index_checked (MonoImage *image, uint32_t idx, gboolean ignore_missing, MonoError *error); MONO_COMPONENT_API MonoCustomAttrInfo* mono_custom_attrs_from_method_checked (MonoMethod *method, MonoError *error); +gboolean +mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kind, char **member_name, MonoError *error); MONO_COMPONENT_API MonoCustomAttrInfo* mono_custom_attrs_from_class_checked (MonoClass *klass, MonoError *error); MONO_COMPONENT_API MonoCustomAttrInfo* diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 9089733a47675..c949ab2519699 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -11351,6 +11351,13 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon return_if_nok (error); } + int accessor_kind = -1; + char *member_name = NULL; + if (!header && mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + g_warning ("Method %s is an unsafe accessor with kind %d and target %s", method->name, accessor_kind, member_name); + g_assert_not_reached(); + } + if (!header) { header = mono_method_get_header_checked (method, error); return_if_nok (error); From 1643710c5f1861117c70d9884ab70eabaf2260b7 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 6 Jul 2023 13:34:13 -0400 Subject: [PATCH 02/34] Change field to property --- src/mono/mono/metadata/custom-attrs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 25195aed3ec4d..e3fcd9c919804 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2087,9 +2087,9 @@ mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kin } for (int i = 0; i < decoded_args->named_args_num; ++i) { - if (decoded_args->named_args_info [i].field && !strcmp (decoded_args->named_args_info [i].field->name, "Kind")) { + if (decoded_args->named_args_info [i].prop && !strcmp (decoded_args->named_args_info [i].prop->name, "Kind")) { *accessor_kind = *(int*)decoded_args->named_args [i]->value.primitive; - }else if (decoded_args->named_args_info [i].field && !strcmp (decoded_args->named_args_info [i].field->name, "Kind")) { + }else if (decoded_args->named_args_info [i].prop && !strcmp (decoded_args->named_args_info [i].prop->name, "Name")) { *member_name = *(char**)decoded_args->named_args [i]->value.primitive; } } From b0def6cb26d505573866d6df7a5f78d69240ef04 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 6 Jul 2023 14:25:45 -0400 Subject: [PATCH 03/34] Get Kind from typed_args --- src/mono/mono/metadata/custom-attrs.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index e3fcd9c919804..64cebc5554520 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2086,11 +2086,12 @@ mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kin return FALSE; } + g_assert (decoded_args->typed_args_num == 1); + *accessor_kind = *(int*)decoded_args->typed_args [0]->value.primitive; + for (int i = 0; i < decoded_args->named_args_num; ++i) { - if (decoded_args->named_args_info [i].prop && !strcmp (decoded_args->named_args_info [i].prop->name, "Kind")) { - *accessor_kind = *(int*)decoded_args->named_args [i]->value.primitive; - }else if (decoded_args->named_args_info [i].prop && !strcmp (decoded_args->named_args_info [i].prop->name, "Name")) { - *member_name = *(char**)decoded_args->named_args [i]->value.primitive; + if (decoded_args->named_args_info [i].prop && !strcmp (decoded_args->named_args_info [i].prop->name, "Name")) { + *member_name = (char*)decoded_args->named_args [i]->value.primitive; } } From 0c51e8a058864068d809436ebe44f5b90797f0b5 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 6 Jul 2023 14:03:27 -0400 Subject: [PATCH 04/34] define MonoUnsafeAccessorKind enum --- src/mono/mono/metadata/CMakeLists.txt | 2 ++ src/mono/mono/metadata/unsafe-accessor.c | 10 ++++++++++ src/mono/mono/metadata/unsafe-accessor.h | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 src/mono/mono/metadata/unsafe-accessor.c create mode 100644 src/mono/mono/metadata/unsafe-accessor.h diff --git a/src/mono/mono/metadata/CMakeLists.txt b/src/mono/mono/metadata/CMakeLists.txt index dac92188d4f68..39ec34205c680 100644 --- a/src/mono/mono/metadata/CMakeLists.txt +++ b/src/mono/mono/metadata/CMakeLists.txt @@ -171,6 +171,8 @@ set(metadata_common_sources abi-details.h abi.c memory-manager.c + unsafe-accessor.h + unsafe-accessor.c icall-table.h ${icall_table_sources}) diff --git a/src/mono/mono/metadata/unsafe-accessor.c b/src/mono/mono/metadata/unsafe-accessor.c new file mode 100644 index 0000000000000..070b05d93d570 --- /dev/null +++ b/src/mono/mono/metadata/unsafe-accessor.c @@ -0,0 +1,10 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// + +#include "config.h" +#include +#include "mono/metadata/unsafe-accessor.h" +#include "mono/utils/mono-compiler.h" + +MONO_EMPTY_SOURCE_FILE (unsafe_accessor) diff --git a/src/mono/mono/metadata/unsafe-accessor.h b/src/mono/mono/metadata/unsafe-accessor.h new file mode 100644 index 0000000000000..22f7a9909ac93 --- /dev/null +++ b/src/mono/mono/metadata/unsafe-accessor.h @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// + +#ifndef __MONO_METADATA_UNSAFE_ACCESSOR_H__ +#define __MONO_METADATA_UNSAFE_ACCESSOR_H__ + +/* keep in sync with System.Runtime.CompilerServices.UnsafeAccessorKind + * https://github.com/dotnet/runtime/blob/a2c19cd005a1130ba7f921e0264287cfbfa8513c/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/UnsafeAccessorAttribute.cs#L9-L35 + */ +enum MonoUnsafeAccessorKind { + MONO_UNSAFE_ACCESSOR_CTOR, + MONO_UNSAFE_ACCESSOR_METHOD, + MONO_UNSAFE_ACCESSOR_STATIC_METHOD, + MONO_UNSAFE_ACCESSOR_FIELD, + MONO_UNSAFE_ACCESSOR_STATIC_FIELD, +}; + +#endif /* __MONO_METADATA_UNSAFE_ACCESSOR_H__ */ From 94ce7d212dbabf1ada08f45b866f226954f1f14c Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 6 Jul 2023 17:41:26 -0400 Subject: [PATCH 05/34] Add the frontend for JIT --- src/mono/mono/metadata/class-internals.h | 3 ++ src/mono/mono/metadata/loader.c | 46 ++++++++++++++++++++++-- src/mono/mono/mini/mini-runtime.c | 10 ++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 68f83b16ab2fc..3863d3213fa40 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1421,6 +1421,9 @@ mono_method_has_no_body (MonoMethod *method); MONO_COMPONENT_API MonoMethodHeader* mono_method_get_header_internal (MonoMethod *method, MonoError *error); +gboolean +mono_method_metadata_has_header (MonoMethod *method); + MONO_COMPONENT_API void mono_method_get_param_names_internal (MonoMethod *method, const char **names); diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 81209134e6444..a142378644eb0 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -2076,8 +2076,8 @@ mono_method_get_header_internal (MonoMethod *method, MonoError *error) g_assert (mono_metadata_token_table (method->token) == MONO_TABLE_METHOD); idx = mono_metadata_token_index (method->token); - if (G_UNLIKELY (img->has_updates)) - loc = mono_metadata_update_get_updated_method_rva (img, idx); + if (G_UNLIKELY (img->has_updates)) + loc = mono_metadata_update_get_updated_method_rva (img, idx); if (!loc) { rva = mono_metadata_decode_row_col (&img->tables [MONO_TABLE_METHOD], idx - 1, MONO_METHOD_RVA); @@ -2100,6 +2100,48 @@ mono_method_get_header_internal (MonoMethod *method, MonoError *error) return mono_metadata_parse_mh_full (img, container, (const char *)loc, error); } +gboolean +mono_method_metadata_has_header (MonoMethod *method) +{ + int idx; + guint32 rva; + MonoImage* img; + gpointer loc = NULL; + MonoGenericContainer *container; + + img = m_class_get_image (method->klass); + + if (mono_method_has_no_body (method)) { + return FALSE; + } + + if (method->is_inflated) { + MonoMethodInflated *imethod = (MonoMethodInflated *) method; + MonoMethodHeader *header, *iheader; + + return mono_method_metadata_has_header (imethod->declaring); + } + + if (method->wrapper_type != MONO_WRAPPER_NONE || method->sre_method) { + MonoMethodWrapper *mw = (MonoMethodWrapper *)method; + return mw->header != NULL; + } + + g_assert (mono_metadata_token_table (method->token) == MONO_TABLE_METHOD); + idx = mono_metadata_token_index (method->token); + + if (G_UNLIKELY (img->has_updates)) + loc = mono_metadata_update_get_updated_method_rva (img, idx); + + if (!loc) { + rva = mono_metadata_decode_row_col (&img->tables [MONO_TABLE_METHOD], idx - 1, MONO_METHOD_RVA); + + loc = mono_image_rva_map (img, rva); + } + + return loc != NULL; +} + MonoMethodHeader* mono_method_get_header_checked (MonoMethod *method, MonoError *error) // Public function that must initialize MonoError for compatibility. diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 933b6f8b48db6..58deaf7e21283 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -2649,6 +2649,16 @@ compile_special (MonoMethod *method, MonoError *error) } } + gboolean has_header = mono_method_metadata_has_header (method); + if (G_UNLIKELY (!has_header)) { + char *member_name = NULL; + int accessor_kind = -1; + if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { + g_warning ("Method %s is an unsafe accessor with kind %d and target %s", method->name, accessor_kind, member_name); + g_assert_not_reached (); + } + } + return NULL; } From a49a8e04733c852727eee85cbb26659d474693ea Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jul 2023 11:05:49 -0400 Subject: [PATCH 06/34] Add mono_marshal_get_unsafe_accessor_wrapper and WRAPPER_SUBTYPE_UNSAFE_ACCESSOR And the associated AOT compiler/runtime and marshaling caching boilerplate. --- src/mono/mono/metadata/loader-internals.h | 1 + src/mono/mono/metadata/marshal-lightweight.c | 9 ++ src/mono/mono/metadata/marshal.c | 93 ++++++++++++++++++++ src/mono/mono/metadata/marshal.h | 16 +++- src/mono/mono/metadata/unsafe-accessor.h | 4 +- src/mono/mono/mini/aot-compiler.c | 13 +++ src/mono/mono/mini/aot-runtime.c | 9 ++ 7 files changed, 142 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/loader-internals.h b/src/mono/mono/metadata/loader-internals.h index af05fa36fa5f4..0e91afbbc1342 100644 --- a/src/mono/mono/metadata/loader-internals.h +++ b/src/mono/mono/metadata/loader-internals.h @@ -88,6 +88,7 @@ typedef struct { GHashTable *cominterop_invoke_cache; GHashTable *cominterop_wrapper_cache; /* LOCKING: marshal lock */ GHashTable *thunk_invoke_cache; + GHashTable *unsafe_accessor_cache; } MonoWrapperCaches; /* Lock-free allocator */ diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 8871b2f4eee4e..e22a065be841b 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -39,6 +39,7 @@ #include "mono/metadata/handle.h" #include "mono/metadata/custom-attrs-internals.h" #include "mono/metadata/icall-internals.h" +#include "mono/metadata/unsafe-accessor.h" #include "mono/utils/mono-tls.h" #include "mono/utils/mono-memory-model.h" #include "mono/utils/atomic.h" @@ -2319,6 +2320,13 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo 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) +{ + // TODO: implement me + mono_mb_emit_byte (mb, CEE_RET); +} + static void emit_generic_array_helper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig) { @@ -3154,6 +3162,7 @@ mono_marshal_lightweight_init (void) cb.emit_synchronized_wrapper = emit_synchronized_wrapper_ilgen; cb.emit_unbox_wrapper = emit_unbox_wrapper_ilgen; cb.emit_array_accessor_wrapper = emit_array_accessor_wrapper_ilgen; + cb.emit_unsafe_accessor_wrapper = emit_unsafe_accessor_wrapper_ilgen; cb.emit_generic_array_helper = emit_generic_array_helper_ilgen; cb.emit_thunk_invoke_wrapper = emit_thunk_invoke_wrapper_ilgen; cb.emit_create_string_hack = emit_create_string_hack_ilgen; diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 0307dc16a515a..fe645e97d4597 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5094,6 +5094,98 @@ mono_marshal_get_array_accessor_wrapper (MonoMethod *method) return res; } +/* + * mono_marshal_get_unsafe_accessor_wrapper: + * + * Return a wrapper for an extern [UnsafeAccessor] method that accesses a member of some class. + * + * member_name can be NULL in which case the name of the member is the same as the name of the accessor method + * + * If the kind is Field or StaticField the accessor_method must have a signature like: + * ref FieldType AccessorMethod (TargetClassOrStruct @this); + * or + * ref FieldType AccessorMethod (ref TargetStruct @this); + * + * If the kind is Method or StaticMethod, the accessor_method must have a signature like: + * ReturnType AccessorMethod (TargetClassOrStruct @this[, FirstArg arg1[, SecondArg arg2[, ...]]]) + * + * where the member method is + * + * class TargetClassOrStruct { + * ReturnType MemberName ([FirstArg arg1[, SecondArg arg2[, ...]]]); + * } + * + * + * or + * ReturnType AccessorMethod (ref TargetStruct @this[, FirstArg arg1[, SecondArg arg2[, ...]]]) + * + * where the member method is + * + * struct TargetStruct { + * ReturnType MemberName ([FirstArg arg1[, SecondArg arg2[, ...]]]); + * } + * + * + * If the kind is Constructor, the accessor_method must have a signature like: + * TargetClass AccessorMethod ([FirstArg arg1[, SecondArg arg2[, ...]]]); + */ +MonoMethod * +mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsafeAccessorKind kind, const char *member_name) +{ + MonoMethodSignature *sig; + MonoMethodBuilder *mb; + MonoMethod *res; + GHashTable *cache; + MonoGenericContext *ctx = NULL; + MonoMethod *orig_method = NULL; + WrapperInfo *info; + MonoClass *accessor_klass = accessor_method->klass; + + g_assert (!accessor_method->is_generic); // we don't support generic accessors to generic definitions right now + g_assert (!m_class_is_ginst (accessor_klass) && ! m_class_is_gtd (accessor_klass)); // TODO: don't assert - generate an accessor that throws BadImageFormatException when called. + + if (member_name == NULL) + member_name = accessor_method->name; + + /* + * Check cache + */ + if (ctx) { + cache = NULL; + g_assert_not_reached (); + } 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); + + 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); + info->d.unsafe_accessor.method = accessor_method; + info->d.unsafe_accessor.kind = kind; + info->d.unsafe_accessor.member_name = member_name; + + if (ctx) { + 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); + } else { + res = mono_mb_create_and_cache_full (cache, accessor_method, mb, sig, sig->param_count + 16, info, NULL); + } + mono_mb_free (mb); + + // TODO: remove before merging + mono_method_print_code (res); + + return res; +} + #ifdef HOST_WIN32 static void* @@ -6398,4 +6490,5 @@ mono_wrapper_caches_free (MonoWrapperCaches *cache) free_hash (cache->cominterop_invoke_cache); free_hash (cache->cominterop_wrapper_cache); free_hash (cache->thunk_invoke_cache); + free_hash (cache->unsafe_accessor_cache); } diff --git a/src/mono/mono/metadata/marshal.h b/src/mono/mono/metadata/marshal.h index 9f261cf01a7fc..f78962313783c 100644 --- a/src/mono/mono/metadata/marshal.h +++ b/src/mono/mono/metadata/marshal.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -135,7 +136,8 @@ typedef enum { WRAPPER_SUBTYPE_INTERP_IN, WRAPPER_SUBTYPE_INTERP_LMF, WRAPPER_SUBTYPE_AOT_INIT, - WRAPPER_SUBTYPE_LLVM_FUNC + WRAPPER_SUBTYPE_LLVM_FUNC, + WRAPPER_SUBTYPE_UNSAFE_ACCESSOR, } WrapperSubtype; typedef struct { @@ -239,6 +241,12 @@ typedef struct { MonoMethodSignature *sig; } NativeFuncWrapperInfo; +typedef struct { + MonoMethod *method; + MonoUnsafeAccessorKind kind; + const char *member_name; /* the member we're accessing */ +} UnsafeAccessorWrapperInfo; + /* * This structure contains additional information to uniquely identify a given wrapper * method. It can be retrieved by mono_marshal_get_wrapper_info () for certain types @@ -285,6 +293,8 @@ typedef struct { LLVMFuncWrapperInfo llvm_func; /* NATIVE_FUNC_INDIRECT */ NativeFuncWrapperInfo native_func; + /* UNSAFE_ACCESSOR */ + UnsafeAccessorWrapperInfo unsafe_accessor; } d; } WrapperInfo; @@ -332,6 +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_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); @@ -588,6 +599,9 @@ mono_marshal_get_gsharedvt_in_wrapper (void); MonoMethod* mono_marshal_get_gsharedvt_out_wrapper (void); +MonoMethod* +mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsafeAccessorKind kind, const char *member_name); + void mono_marshal_free_dynamic_wrappers (MonoMethod *method); diff --git a/src/mono/mono/metadata/unsafe-accessor.h b/src/mono/mono/metadata/unsafe-accessor.h index 22f7a9909ac93..913ac51425f09 100644 --- a/src/mono/mono/metadata/unsafe-accessor.h +++ b/src/mono/mono/metadata/unsafe-accessor.h @@ -8,12 +8,12 @@ /* keep in sync with System.Runtime.CompilerServices.UnsafeAccessorKind * https://github.com/dotnet/runtime/blob/a2c19cd005a1130ba7f921e0264287cfbfa8513c/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/UnsafeAccessorAttribute.cs#L9-L35 */ -enum MonoUnsafeAccessorKind { +typedef enum MonoUnsafeAccessorKind { MONO_UNSAFE_ACCESSOR_CTOR, MONO_UNSAFE_ACCESSOR_METHOD, MONO_UNSAFE_ACCESSOR_STATIC_METHOD, MONO_UNSAFE_ACCESSOR_FIELD, MONO_UNSAFE_ACCESSOR_STATIC_FIELD, -}; +} MonoUnsafeAccessorKind; #endif /* __MONO_METADATA_UNSAFE_ACCESSOR_H__ */ diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 35d73ccccfa05..236a03e5f9dd5 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -3840,6 +3840,14 @@ encode_method_ref (MonoAotCompile *acfg, MonoMethod *method, guint8 *buf, guint8 encode_method_ref (acfg, info->d.synchronized_inner.method, p, &p); else if (info->subtype == WRAPPER_SUBTYPE_ARRAY_ACCESSOR) encode_method_ref (acfg, info->d.array_accessor.method, p, &p); + else if (info->subtype == WRAPPER_SUBTYPE_UNSAFE_ACCESSOR) { + encode_method_ref (acfg, info->d.unsafe_accessor.method, p, &p); + encode_value (info->d.unsafe_accessor.kind, p, &p); + /* WISH: is there some kind of string heap token we could use here? */ + 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 if (info->subtype == WRAPPER_SUBTYPE_INTERP_IN) encode_signature (acfg, info->d.interp_in.sig, p, &p); else if (info->subtype == WRAPPER_SUBTYPE_GSHAREDVT_IN_SIG) @@ -10151,6 +10159,9 @@ append_mangled_wrapper_subtype (GString *s, WrapperSubtype subtype) case WRAPPER_SUBTYPE_ARRAY_ACCESSOR: label = "array_acc"; break; + case WRAPPER_SUBTYPE_UNSAFE_ACCESSOR: + label = "unsafe_acc"; + break; case WRAPPER_SUBTYPE_GENERIC_ARRAY_HELPER: label = "generic_arry_help"; break; @@ -10317,6 +10328,8 @@ append_mangled_wrapper (GString *s, MonoMethod *method) success = success && append_mangled_method (s, info->d.synchronized_inner.method); else if (info->subtype == WRAPPER_SUBTYPE_ARRAY_ACCESSOR) success = success && append_mangled_method (s, info->d.array_accessor.method); + else if (info->subtype == WRAPPER_SUBTYPE_UNSAFE_ACCESSOR) + success = success && append_mangled_method (s, info->d.unsafe_accessor.method); else if (info->subtype == WRAPPER_SUBTYPE_INTERP_IN) append_mangled_signature (s, info->d.interp_in.sig); else if (info->subtype == WRAPPER_SUBTYPE_GSHAREDVT_IN_SIG) { diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index e6c0904e65b79..a1d1e1939021c 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -1066,6 +1066,15 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod if (!m) return FALSE; ref->method = mono_marshal_get_array_accessor_wrapper (m); + } else if (subtype == WRAPPER_SUBTYPE_UNSAFE_ACCESSOR) { + MonoMethod *m = decode_resolve_method_ref (module, p, &p, error); + 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; + if (!m) + return FALSE; + 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) { From 4701689d01b37740da72171ceb0525fced6c2aea Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jul 2023 11:13:52 -0400 Subject: [PATCH 07/34] [interp] get the unsafe accessor wrapper --- src/mono/mono/mini/interp/transform.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index c949ab2519699..c573def8dd84b 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -11355,6 +11355,10 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon char *member_name = NULL; if (!header && mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { g_warning ("Method %s is an unsafe accessor with kind %d and target %s", method->name, accessor_kind, member_name); + // get the wrapper instead + method = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + g_assert (method); + // TODO: remove this assert_not_reached when the accessor generation is done g_assert_not_reached(); } From 495ce381001863aac351d0fda0fb77fb6a3c3bb9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jul 2023 13:12:20 -0400 Subject: [PATCH 08/34] fix: skip visibility in unsafe accessor wrappers that is the whole point of them --- src/mono/mono/metadata/marshal.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index fe645e97d4597..58ee42d90c0bc 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5164,6 +5164,8 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); + get_marshal_cb ()->mb_skip_visibility (mb); + 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); From 35c4d45d028ab94d995b78044693a671b0904305 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jul 2023 13:12:54 -0400 Subject: [PATCH 09/34] fix: decode the length and copy the name from UnsafeAccessorAttribute The name has a length as a prefix and doesn not have a null terminator --- src/mono/mono/metadata/custom-attrs.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 64cebc5554520..99a45b53bc142 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2091,7 +2091,12 @@ mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kin for (int i = 0; i < decoded_args->named_args_num; ++i) { if (decoded_args->named_args_info [i].prop && !strcmp (decoded_args->named_args_info [i].prop->name, "Name")) { - *member_name = (char*)decoded_args->named_args [i]->value.primitive; + const char *ptr = (const char*)decoded_args->named_args [i]->value.primitive; + uint32_t len = mono_metadata_decode_value (ptr, &ptr); + char *name = m_method_alloc0 (method, len + 1); + memcpy (name, ptr, len); + name[len] = 0; + *member_name = (char*)name; } } From 0c8f4da0983bb70dddd4c1b7bc3a907f1edce4e7 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 10 Jul 2023 13:12:02 -0400 Subject: [PATCH 10/34] [mini] compile wrapper --- src/mono/mono/mini/mini-runtime.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 58deaf7e21283..84b3ec4df6827 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -2655,8 +2655,15 @@ compile_special (MonoMethod *method, MonoError *error) int accessor_kind = -1; if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { g_warning ("Method %s is an unsafe accessor with kind %d and target %s", method->name, accessor_kind, member_name); - g_assert_not_reached (); - } + MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); + gpointer compiled_wrapper = mono_jit_compile_method_jit_only (wrapper, error); + return_val_if_nok (error, NULL); + code = mono_get_addr_from_ftnptr (compiled_wrapper); + jinfo = mini_jit_info_table_find (code); + if (jinfo) + MONO_PROFILER_RAISE (jit_done, (method, jinfo)); + return code; + } } return NULL; From 8d039e68028ceb3687ab8fe53648e474da6b0a6f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 10 Jul 2023 13:41:39 -0400 Subject: [PATCH 11/34] [aot] Emit unsafe accessor wrappers to the AOT image --- src/mono/mono/mini/aot-compiler.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 236a03e5f9dd5..bb3dac29d1c31 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -5243,6 +5243,26 @@ 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 From 080559aab366d25693a7b30d6d85a5b9e101b8cc Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 10 Jul 2023 15:49:55 -0400 Subject: [PATCH 12/34] Add the method to emit wrapper for field --- src/mono/mono/metadata/marshal-lightweight.c | 48 +++++++++++++++++++- src/mono/mono/mini/aot-compiler.c | 1 - 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index e22a065be841b..9ca400f1c7c26 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2321,12 +2321,56 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo } 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_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) { - // TODO: implement me + // 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 accessor wrapper's parent + 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", "UnsafeAccessor"); + + 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); + gboolean ret_byref = m_type_is_byref (ret_type); + if (!ret_byref || (kind = MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor"); + + mono_mb_emit_ldarg (mb, 0); + + MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL); + if (target_field == NULL || target_field->type->type != ret_type->type) + mono_mb_emit_exception_full (mb, "System", "MissingFieldException", "UnsafeAccessor"); + + mono_mb_emit_op (mb, CEE_LDFLDA, target_field); 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) +{ + 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); + return; + case MONO_UNSAFE_ACCESSOR_CTOR: + // TODO + mono_mb_emit_byte (mb, CEE_RET); + return; + case MONO_UNSAFE_ACCESSOR_METHOD: + case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: + // TODO + mono_mb_emit_byte (mb, CEE_RET); + return; + default: + g_assert_not_reached(); // some unknown wrapper kind + } +} + static void emit_generic_array_helper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig) { diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index bb3dac29d1c31..0d0caacf984c2 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -5260,7 +5260,6 @@ add_full_aot_wrappers (MonoAotCompile *acfg) 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)); } - } } From 2ec266bc43678c8bd73c28489b6975cc1e5ab67b Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 10 Jul 2023 15:53:36 -0400 Subject: [PATCH 13/34] Fix typo --- src/mono/mono/metadata/marshal-lightweight.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 9ca400f1c7c26..6c4a678df7e08 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2336,7 +2336,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ gboolean target_byref = m_type_is_byref (target_type); gboolean target_valuetype = m_class_is_valuetype (target_class); gboolean ret_byref = m_type_is_byref (ret_type); - if (!ret_byref || (kind = MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) + if (!ret_byref || (kind == MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor"); mono_mb_emit_ldarg (mb, 0); From 8571969a4b6f83264b91c1172040ebdfd1a2a0af Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 10 Jul 2023 16:00:45 -0400 Subject: [PATCH 14/34] Remove assertion for interpreter --- src/mono/mono/mini/interp/transform.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index c573def8dd84b..5edf4ec6222e2 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -11354,12 +11354,8 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon int accessor_kind = -1; char *member_name = NULL; if (!header && mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - g_warning ("Method %s is an unsafe accessor with kind %d and target %s", method->name, accessor_kind, member_name); - // get the wrapper instead method = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); g_assert (method); - // TODO: remove this assert_not_reached when the accessor generation is done - g_assert_not_reached(); } if (!header) { From b7e35c6d3637f882230efc0a709a885bfd3cdfa0 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 10 Jul 2023 16:31:19 -0400 Subject: [PATCH 15/34] Fix format and replace assertion with proper exception --- src/mono/mono/metadata/loader.c | 53 ++++++++++---------- src/mono/mono/metadata/marshal-lightweight.c | 4 +- src/mono/mono/metadata/marshal.c | 4 +- src/mono/mono/metadata/unsafe-accessor.h | 2 +- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index a142378644eb0..522578554f950 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -2104,42 +2104,41 @@ gboolean mono_method_metadata_has_header (MonoMethod *method) { int idx; - guint32 rva; - MonoImage* img; - gpointer loc = NULL; - MonoGenericContainer *container; - - img = m_class_get_image (method->klass); + guint32 rva; + MonoImage* img; + gpointer loc = NULL; + MonoGenericContainer *container; - if (mono_method_has_no_body (method)) { - return FALSE; - } + img = m_class_get_image (method->klass); - if (method->is_inflated) { - MonoMethodInflated *imethod = (MonoMethodInflated *) method; - MonoMethodHeader *header, *iheader; + if (mono_method_has_no_body (method)) { + return FALSE; + } - return mono_method_metadata_has_header (imethod->declaring); - } + if (method->is_inflated) { + MonoMethodInflated *imethod = (MonoMethodInflated *) method; + MonoMethodHeader *header, *iheader; - if (method->wrapper_type != MONO_WRAPPER_NONE || method->sre_method) { - MonoMethodWrapper *mw = (MonoMethodWrapper *)method; - return mw->header != NULL; - } + return mono_method_metadata_has_header (imethod->declaring); + } - g_assert (mono_metadata_token_table (method->token) == MONO_TABLE_METHOD); - idx = mono_metadata_token_index (method->token); + if (method->wrapper_type != MONO_WRAPPER_NONE || method->sre_method) { + MonoMethodWrapper *mw = (MonoMethodWrapper *)method; + return mw->header != NULL; + } - if (G_UNLIKELY (img->has_updates)) - loc = mono_metadata_update_get_updated_method_rva (img, idx); + g_assert (mono_metadata_token_table (method->token) == MONO_TABLE_METHOD); + idx = mono_metadata_token_index (method->token); - if (!loc) { - rva = mono_metadata_decode_row_col (&img->tables [MONO_TABLE_METHOD], idx - 1, MONO_METHOD_RVA); + if (G_UNLIKELY (img->has_updates)) + loc = mono_metadata_update_get_updated_method_rva (img, idx); - loc = mono_image_rva_map (img, rva); - } + if (!loc) { + rva = mono_metadata_decode_row_col (&img->tables [MONO_TABLE_METHOD], idx - 1, MONO_METHOD_RVA); + loc = mono_image_rva_map (img, rva); + } - return loc != NULL; + return loc != NULL; } MonoMethodHeader* diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 6c4a678df7e08..767d024b65095 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2330,14 +2330,14 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ MonoType *target_type = sig->params[0]; // params[0] is the accessor wrapper's parent 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", "UnsafeAccessor"); + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedArgCheck"); 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); gboolean ret_byref = m_type_is_byref (ret_type); if (!ret_byref || (kind == MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor"); + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedRefCheck"); mono_mb_emit_ldarg (mb, 0); diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 58ee42d90c0bc..4f04e660406c0 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5141,8 +5141,8 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf WrapperInfo *info; MonoClass *accessor_klass = accessor_method->klass; - g_assert (!accessor_method->is_generic); // we don't support generic accessors to generic definitions right now - g_assert (!m_class_is_ginst (accessor_klass) && ! m_class_is_gtd (accessor_klass)); // TODO: don't assert - generate an accessor that throws BadImageFormatException when called. + if (accessor_method->is_generic) + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); if (member_name == NULL) member_name = accessor_method->name; diff --git a/src/mono/mono/metadata/unsafe-accessor.h b/src/mono/mono/metadata/unsafe-accessor.h index 913ac51425f09..dd50cc6ca97ab 100644 --- a/src/mono/mono/metadata/unsafe-accessor.h +++ b/src/mono/mono/metadata/unsafe-accessor.h @@ -8,7 +8,7 @@ /* keep in sync with System.Runtime.CompilerServices.UnsafeAccessorKind * https://github.com/dotnet/runtime/blob/a2c19cd005a1130ba7f921e0264287cfbfa8513c/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/UnsafeAccessorAttribute.cs#L9-L35 */ -typedef enum MonoUnsafeAccessorKind { +typedef enum { MONO_UNSAFE_ACCESSOR_CTOR, MONO_UNSAFE_ACCESSOR_METHOD, MONO_UNSAFE_ACCESSOR_STATIC_METHOD, From 449aa6f2e73344c965f7e8853af3121a1340d8a3 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 10 Jul 2023 16:42:21 -0400 Subject: [PATCH 16/34] Free the memory and throw proper NotImplementedException --- src/mono/mono/metadata/custom-attrs.c | 4 ++++ src/mono/mono/metadata/marshal-lightweight.c | 4 ++-- src/mono/mono/mini/mini-runtime.c | 1 - 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 99a45b53bc142..247e8765e8016 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2076,6 +2076,9 @@ mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kin } } + if (!cinfo->cached) + mono_custom_attrs_free(cinfo); + if (!attr) return FALSE; @@ -2083,6 +2086,7 @@ mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kin if (!is_ok (error)) { mono_error_cleanup (error); + mono_reflection_free_custom_attr_data_args_noalloc (decoded_args); return FALSE; } diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 767d024b65095..a63b801fe5c29 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2359,12 +2359,12 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ return; case MONO_UNSAFE_ACCESSOR_CTOR: // TODO - mono_mb_emit_byte (mb, CEE_RET); + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor"); return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: // TODO - mono_mb_emit_byte (mb, CEE_RET); + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor"); return; default: g_assert_not_reached(); // some unknown wrapper kind diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 84b3ec4df6827..1d01a77fb3c14 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -2654,7 +2654,6 @@ compile_special (MonoMethod *method, MonoError *error) char *member_name = NULL; int accessor_kind = -1; if (mono_method_get_unsafe_accessor_attr_data (method, &accessor_kind, &member_name, error)) { - g_warning ("Method %s is an unsafe accessor with kind %d and target %s", method->name, accessor_kind, member_name); MonoMethod *wrapper = mono_marshal_get_unsafe_accessor_wrapper (method, (MonoUnsafeAccessorKind)accessor_kind, member_name); gpointer compiled_wrapper = mono_jit_compile_method_jit_only (wrapper, error); return_val_if_nok (error, NULL); From da84576fd11151834fc0eaa9c1d92336420877ba Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 10 Jul 2023 16:48:57 -0400 Subject: [PATCH 17/34] Enable StaticField and Field tests --- .../compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 982b2e39fc4b5..24da8e2c8e83a 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -155,7 +155,6 @@ public static void Verify_CallCtorAsMethodValue() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_AccessStaticFieldClass() { Console.WriteLine($"Running {nameof(Verify_AccessStaticFieldClass)}"); @@ -167,7 +166,6 @@ public static void Verify_AccessStaticFieldClass() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_AccessFieldClass() { Console.WriteLine($"Running {nameof(Verify_AccessFieldClass)}"); @@ -180,7 +178,6 @@ public static void Verify_AccessFieldClass() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_AccessStaticFieldValue() { Console.WriteLine($"Running {nameof(Verify_AccessStaticFieldValue)}"); @@ -192,7 +189,6 @@ public static void Verify_AccessStaticFieldValue() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_AccessFieldValue() { Console.WriteLine($"Running {nameof(Verify_AccessFieldValue)}"); From 23a4d9a684b1dfcaf9657936d7d463df958783cc Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Tue, 11 Jul 2023 10:39:22 -0400 Subject: [PATCH 18/34] Remove unused variables --- src/mono/mono/metadata/loader.c | 3 --- src/mono/mono/metadata/marshal.c | 7 +++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 522578554f950..31b5d751bed7f 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -2107,7 +2107,6 @@ mono_method_metadata_has_header (MonoMethod *method) guint32 rva; MonoImage* img; gpointer loc = NULL; - MonoGenericContainer *container; img = m_class_get_image (method->klass); @@ -2117,8 +2116,6 @@ mono_method_metadata_has_header (MonoMethod *method) if (method->is_inflated) { MonoMethodInflated *imethod = (MonoMethodInflated *) method; - MonoMethodHeader *header, *iheader; - return mono_method_metadata_has_header (imethod->declaring); } diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 4f04e660406c0..1e755df0a84fb 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5139,10 +5139,6 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf MonoGenericContext *ctx = NULL; MonoMethod *orig_method = NULL; WrapperInfo *info; - MonoClass *accessor_klass = accessor_method->klass; - - if (accessor_method->is_generic) - mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); if (member_name == NULL) member_name = accessor_method->name; @@ -5164,6 +5160,9 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); + if (accessor_method->is_generic) + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); + get_marshal_cb ()->mb_skip_visibility (mb); get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, ctx, kind, member_name); From 342c8d4607d84c3113a81fb53213437f0c475c0b Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Tue, 11 Jul 2023 10:56:41 -0400 Subject: [PATCH 19/34] Only allow static method --- src/mono/mono/metadata/marshal.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 1e755df0a84fb..cd7249d160ad2 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5163,6 +5163,9 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf if (accessor_method->is_generic) mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); + if (!m_method_is_static (accessor_method)) + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic"); + get_marshal_cb ()->mb_skip_visibility (mb); get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, ctx, kind, member_name); From 375d460c1c5063c6ba7f31af777c4b1599c618bf Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Tue, 11 Jul 2023 11:12:45 -0400 Subject: [PATCH 20/34] Return immediately once hit an exception --- src/mono/mono/metadata/marshal-lightweight.c | 22 +++++++++++++++++--- src/mono/mono/metadata/marshal.c | 6 ------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index a63b801fe5c29..4812282c9b851 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2329,21 +2329,27 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ MonoType *target_type = sig->params[0]; // params[0] is the accessor wrapper's parent MonoType *ret_type = sig->ret; - if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID) + if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedArgCheck"); + return; + } 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); gboolean ret_byref = m_type_is_byref (ret_type); - if (!ret_byref || (kind == MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) + if (!ret_byref || (kind == MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) { mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedRefCheck"); + return; + } mono_mb_emit_ldarg (mb, 0); MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL); - if (target_field == NULL || target_field->type->type != ret_type->type) + if (target_field == NULL || target_field->type->type != ret_type->type) { mono_mb_emit_exception_full (mb, "System", "MissingFieldException", "UnsafeAccessor"); + return; + } mono_mb_emit_op (mb, CEE_LDFLDA, target_field); mono_mb_emit_byte (mb, CEE_RET); @@ -2352,6 +2358,16 @@ emit_unsafe_accessor_field_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) { + if (accessor_method->is_generic) { + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); + return; + } + + if (!m_method_is_static (accessor_method)) { + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic"); + return; + } + switch (kind) { case MONO_UNSAFE_ACCESSOR_FIELD: case MONO_UNSAFE_ACCESSOR_STATIC_FIELD: diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index cd7249d160ad2..0e48c3d024664 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5160,12 +5160,6 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER); - if (accessor_method->is_generic) - mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); - - if (!m_method_is_static (accessor_method)) - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic"); - get_marshal_cb ()->mb_skip_visibility (mb); get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, ctx, kind, member_name); From 9222fd218a98ad73c4751b772e88befe84d2a111 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Tue, 11 Jul 2023 12:08:04 -0400 Subject: [PATCH 21/34] Free cinfo at the right location --- src/mono/mono/metadata/custom-attrs.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 247e8765e8016..8fc9f47daf0af 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2076,17 +2076,19 @@ mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kin } } - if (!cinfo->cached) - mono_custom_attrs_free(cinfo); - - if (!attr) + if (!attr){ + if (!cinfo->cached) + mono_custom_attrs_free(cinfo); return FALSE; + } MonoDecodeCustomAttr *decoded_args = mono_reflection_create_custom_attr_data_args_noalloc (m_class_get_image (attr->ctor->klass), attr->ctor, attr->data, attr->data_size, error); if (!is_ok (error)) { mono_error_cleanup (error); mono_reflection_free_custom_attr_data_args_noalloc (decoded_args); + if (!cinfo->cached) + mono_custom_attrs_free(cinfo); return FALSE; } @@ -2105,6 +2107,9 @@ mono_method_get_unsafe_accessor_attr_data (MonoMethod *method, int *accessor_kin } mono_reflection_free_custom_attr_data_args_noalloc (decoded_args); + if (!cinfo->cached) + mono_custom_attrs_free(cinfo); + return TRUE; } From ab2e8c5aad1a5b63830b4c2d40039e219e0ab149 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Tue, 11 Jul 2023 14:13:39 -0400 Subject: [PATCH 22/34] Disable test, cause Constructor kind is not supported yet. --- .../compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 24da8e2c8e83a..cef583e59dbc8 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -166,6 +166,7 @@ public static void Verify_AccessStaticFieldClass() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_AccessFieldClass() { Console.WriteLine($"Running {nameof(Verify_AccessFieldClass)}"); From 2f8031a0533c8f1b89957d7c8d98033cd701d94d Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Tue, 11 Jul 2023 16:21:31 -0400 Subject: [PATCH 23/34] Stop inlining UnsafeAccessor wrapper for interpreter --- src/mono/mono/metadata/marshal-lightweight.c | 21 +++++++++++++------- src/mono/mono/mini/interp/transform.c | 1 + 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 4812282c9b851..c0312af50544e 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2320,6 +2320,13 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo mono_mb_emit_byte (mb, CEE_RET); } +static void +emit_unsafe_accessor_exception (MonoMethodBuilder *mb, const char *id, const char *msg) +{ + mono_mb_emit_exception_full (mb, "System", id, msg); + mb->method->iflags |= METHOD_IMPL_ATTRIBUTE_NOINLINING; +} + static void emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) { @@ -2330,7 +2337,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ MonoType *target_type = sig->params[0]; // params[0] is the accessor wrapper's parent 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", "UnsafeAccessor_FailedArgCheck"); + emit_unsafe_accessor_exception (mb, "BadImageFormatException", "UnsafeAccessor_FailedArgCheck"); return; } @@ -2339,7 +2346,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ gboolean target_valuetype = m_class_is_valuetype (target_class); gboolean ret_byref = m_type_is_byref (ret_type); if (!ret_byref || (kind == MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) { - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedRefCheck"); + emit_unsafe_accessor_exception (mb, "BadImageFormatException", "UnsafeAccessor_FailedRefCheck"); return; } @@ -2347,7 +2354,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL); if (target_field == NULL || target_field->type->type != ret_type->type) { - mono_mb_emit_exception_full (mb, "System", "MissingFieldException", "UnsafeAccessor"); + emit_unsafe_accessor_exception (mb, "MissingFieldException", "UnsafeAccessor"); return; } @@ -2359,12 +2366,12 @@ static void emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) { if (accessor_method->is_generic) { - mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); + emit_unsafe_accessor_exception (mb, "NotImplementedException", "UnsafeAccessor_Generics"); return; } if (!m_method_is_static (accessor_method)) { - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic"); + emit_unsafe_accessor_exception (mb, "BadImageFormatException", "UnsafeAccessor_NonStatic"); return; } @@ -2375,12 +2382,12 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ return; case MONO_UNSAFE_ACCESSOR_CTOR: // TODO - mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor"); + emit_unsafe_accessor_exception (mb, "NotImplementedException", "UnsafeAccessor"); return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: // TODO - mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor"); + emit_unsafe_accessor_exception (mb, "NotImplementedException", "UnsafeAccessor"); return; default: g_assert_not_reached(); // some unknown wrapper kind diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 5edf4ec6222e2..881dab7b47285 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -11386,6 +11386,7 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon memcpy ((char*)imethod + start_offset, (char*)&tmp_imethod + start_offset, sizeof (InterpMethod) - start_offset); mono_memory_barrier (); imethod->transformed = TRUE; + imethod->method = method; mono_interp_stats.methods_transformed++; mono_atomic_fetch_add_i32 (&mono_jit_stats.methods_with_interp, 1); From 4d874c309dc2dd4eb3ecb82ad65f0c296b790834 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Wed, 12 Jul 2023 15:31:27 -0400 Subject: [PATCH 24/34] Revert inlining change --- src/mono/mono/metadata/marshal-lightweight.c | 21 +++++++------------- src/mono/mono/mini/interp/transform.c | 1 - 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index c0312af50544e..4812282c9b851 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2320,13 +2320,6 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo mono_mb_emit_byte (mb, CEE_RET); } -static void -emit_unsafe_accessor_exception (MonoMethodBuilder *mb, const char *id, const char *msg) -{ - mono_mb_emit_exception_full (mb, "System", id, msg); - mb->method->iflags |= METHOD_IMPL_ATTRIBUTE_NOINLINING; -} - static void emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) { @@ -2337,7 +2330,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ MonoType *target_type = sig->params[0]; // params[0] is the accessor wrapper's parent MonoType *ret_type = sig->ret; if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID) { - emit_unsafe_accessor_exception (mb, "BadImageFormatException", "UnsafeAccessor_FailedArgCheck"); + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedArgCheck"); return; } @@ -2346,7 +2339,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ gboolean target_valuetype = m_class_is_valuetype (target_class); gboolean ret_byref = m_type_is_byref (ret_type); if (!ret_byref || (kind == MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) { - emit_unsafe_accessor_exception (mb, "BadImageFormatException", "UnsafeAccessor_FailedRefCheck"); + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedRefCheck"); return; } @@ -2354,7 +2347,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL); if (target_field == NULL || target_field->type->type != ret_type->type) { - emit_unsafe_accessor_exception (mb, "MissingFieldException", "UnsafeAccessor"); + mono_mb_emit_exception_full (mb, "System", "MissingFieldException", "UnsafeAccessor"); return; } @@ -2366,12 +2359,12 @@ static void emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name) { if (accessor_method->is_generic) { - emit_unsafe_accessor_exception (mb, "NotImplementedException", "UnsafeAccessor_Generics"); + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics"); return; } if (!m_method_is_static (accessor_method)) { - emit_unsafe_accessor_exception (mb, "BadImageFormatException", "UnsafeAccessor_NonStatic"); + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic"); return; } @@ -2382,12 +2375,12 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_ return; case MONO_UNSAFE_ACCESSOR_CTOR: // TODO - emit_unsafe_accessor_exception (mb, "NotImplementedException", "UnsafeAccessor"); + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor"); return; case MONO_UNSAFE_ACCESSOR_METHOD: case MONO_UNSAFE_ACCESSOR_STATIC_METHOD: // TODO - emit_unsafe_accessor_exception (mb, "NotImplementedException", "UnsafeAccessor"); + mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor"); return; default: g_assert_not_reached(); // some unknown wrapper kind diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index dba00dff2ec2c..bc52e37f2dd21 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -11454,7 +11454,6 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon memcpy ((char*)imethod + start_offset, (char*)&tmp_imethod + start_offset, sizeof (InterpMethod) - start_offset); mono_memory_barrier (); imethod->transformed = TRUE; - imethod->method = method; mono_interp_stats.methods_transformed++; mono_atomic_fetch_add_i32 (&mono_jit_stats.methods_with_interp, 1); From ade7c7856f4f70db8b21a4fd682671f23a9954e2 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Wed, 12 Jul 2023 15:46:53 -0400 Subject: [PATCH 25/34] Update exception message --- src/mono/mono/metadata/marshal-lightweight.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 4812282c9b851..060fd8d7790e4 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2327,10 +2327,10 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ 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 accessor wrapper's parent + 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) { - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedArgCheck"); + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute."); return; } @@ -2339,7 +2339,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ gboolean target_valuetype = m_class_is_valuetype (target_class); gboolean ret_byref = m_type_is_byref (ret_type); if (!ret_byref || (kind == MONO_UNSAFE_ACCESSOR_FIELD && target_valuetype && !target_byref)) { - mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_FailedRefCheck"); + mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute."); return; } @@ -2347,7 +2347,8 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL); if (target_field == NULL || target_field->type->type != ret_type->type) { - mono_mb_emit_exception_full (mb, "System", "MissingFieldException", "UnsafeAccessor"); + mono_mb_emit_exception_full (mb, "System", "MissingFieldException", + g_strdup_printf("No '%s' in '%s'. Or the type of '%s' doesn't match", member_name, m_class_get_name (target_class), member_name)); return; } From 58822ca75211064380136d6defb4b52be0185467 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 13 Jul 2023 12:05:33 -0400 Subject: [PATCH 26/34] Fix loading static field and add static status check and a test --- src/mono/mono/metadata/marshal-lightweight.c | 13 +++++++++---- .../UnsafeAccessors/UnsafeAccessorsTests.cs | 6 ++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 060fd8d7790e4..44a271d8a8f81 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2343,16 +2343,21 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ return; } - mono_mb_emit_ldarg (mb, 0); - MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL); - if (target_field == NULL || target_field->type->type != ret_type->type) { + if (target_field == NULL || !mono_metadata_type_equal_full (target_field->type, m_class_get_byval_arg (mono_class_from_mono_type_internal (ret_type)), TRUE)) { mono_mb_emit_exception_full (mb, "System", "MissingFieldException", g_strdup_printf("No '%s' in '%s'. Or the type of '%s' doesn't match", member_name, m_class_get_name (target_class), member_name)); return; } + gboolean is_field_static = !!(target_field->type->attrs & FIELD_ATTRIBUTE_STATIC); + if ((kind == MONO_UNSAFE_ACCESSOR_FIELD && is_field_static) || (kind == MONO_UNSAFE_ACCESSOR_STATIC_FIELD && !is_field_static)) { + mono_mb_emit_exception_full (mb, "System", "MissingFieldException", g_strdup_printf("The status of static of field '%s' in '%s' doesn't match", member_name, m_class_get_name (target_class))); + return; + } - mono_mb_emit_op (mb, CEE_LDFLDA, target_field); + 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); mono_mb_emit_byte (mb, CEE_RET); } diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index cef583e59dbc8..d6827a7fed874 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -386,6 +386,9 @@ public static void Verify_InvalidTargetUnsafeAccessor() AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : DoesNotExist, () => FieldNotFound(null)); + AssertExtensions.ThrowsMissingMemberException( + isNativeAot ? null : DoesNotExist, + () => FieldNotFoundStaticStatus(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : DoesNotExist, () => StaticFieldNotFound(null)); @@ -406,6 +409,9 @@ public static void Verify_InvalidTargetUnsafeAccessor() [UnsafeAccessor(UnsafeAccessorKind.Field, Name=DoesNotExist)] extern static ref string FieldNotFound(UserDataClass d); + [UnsafeAccessor(UnsafeAccessorKind.Field, Name=UserDataClass.StaticFieldName)] + extern static ref string FieldNotFoundStaticStatus(UserDataClass d); + [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name=DoesNotExist)] extern static ref string StaticFieldNotFound(UserDataClass d); From 36865904e4efb65c2b33454221a914298946d5c3 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 13 Jul 2023 14:44:46 -0400 Subject: [PATCH 27/34] Add another test and address feedback --- src/mono/mono/metadata/marshal-lightweight.c | 2 +- .../UnsafeAccessors/UnsafeAccessorsTests.cs | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 44a271d8a8f81..0b4e787f5f2bd 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2351,7 +2351,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_ } gboolean is_field_static = !!(target_field->type->attrs & FIELD_ATTRIBUTE_STATIC); if ((kind == MONO_UNSAFE_ACCESSOR_FIELD && is_field_static) || (kind == MONO_UNSAFE_ACCESSOR_STATIC_FIELD && !is_field_static)) { - mono_mb_emit_exception_full (mb, "System", "MissingFieldException", g_strdup_printf("The status of static of field '%s' in '%s' doesn't match", member_name, m_class_get_name (target_class))); + mono_mb_emit_exception_full (mb, "System", "MissingFieldException", g_strdup_printf("UnsafeAccessorKind does not match expected static modifier on field '%s' in '%s'", member_name, m_class_get_name (target_class))); return; } diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index d6827a7fed874..9dd1811f5d534 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -370,6 +370,7 @@ public static void Verify_ManagedUnmanagedFunctionPointersDontMatch() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/88858", TestRuntimes.CoreCLR)] public static void Verify_InvalidTargetUnsafeAccessor() { Console.WriteLine($"Running {nameof(Verify_InvalidTargetUnsafeAccessor)}"); @@ -388,7 +389,10 @@ public static void Verify_InvalidTargetUnsafeAccessor() () => FieldNotFound(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : DoesNotExist, - () => FieldNotFoundStaticStatus(null)); + () => FieldNotFoundStaticMismatch1(null)); + AssertExtensions.ThrowsMissingMemberException( + isNativeAot ? null : DoesNotExist, + () => FieldNotFoundStaticMismatch2(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : DoesNotExist, () => StaticFieldNotFound(null)); @@ -409,8 +413,11 @@ public static void Verify_InvalidTargetUnsafeAccessor() [UnsafeAccessor(UnsafeAccessorKind.Field, Name=DoesNotExist)] extern static ref string FieldNotFound(UserDataClass d); - [UnsafeAccessor(UnsafeAccessorKind.Field, Name=UserDataClass.StaticFieldName)] - extern static ref string FieldNotFoundStaticStatus(UserDataClass d); + [UnsafeAccessor(UnsafeAccessorKind.Field, Name=UserDataValue.StaticFieldName)] + extern static ref string FieldNotFoundStaticMismatch1(UserDataValue d); + + [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name=UserDataValue.FieldName)] + extern static ref string FieldNotFoundStaticMismatch2(UserDataValue d); [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name=DoesNotExist)] extern static ref string StaticFieldNotFound(UserDataClass d); From 8afb00a8130c9aeb3ed19b287ad420acfa894cc7 Mon Sep 17 00:00:00 2001 From: Fan Yang <52458914+fanyang-mono@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:30:39 -0400 Subject: [PATCH 28/34] Update src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs Co-authored-by: Aaron Robinson --- .../compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 9dd1811f5d534..b68b233b86ec4 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -388,7 +388,7 @@ public static void Verify_InvalidTargetUnsafeAccessor() isNativeAot ? null : DoesNotExist, () => FieldNotFound(null)); AssertExtensions.ThrowsMissingMemberException( - isNativeAot ? null : DoesNotExist, + isNativeAot ? null : UserDataClass.StaticFieldName, () => FieldNotFoundStaticMismatch1(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : DoesNotExist, From a3f9e7700e123acdc1c35b5896c4483bbb426cb1 Mon Sep 17 00:00:00 2001 From: Fan Yang <52458914+fanyang-mono@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:30:47 -0400 Subject: [PATCH 29/34] Update src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs Co-authored-by: Aaron Robinson --- .../compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index b68b233b86ec4..7d9a12163e20e 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -391,7 +391,7 @@ public static void Verify_InvalidTargetUnsafeAccessor() isNativeAot ? null : UserDataClass.StaticFieldName, () => FieldNotFoundStaticMismatch1(null)); AssertExtensions.ThrowsMissingMemberException( - isNativeAot ? null : DoesNotExist, + isNativeAot ? null : UserDataValue.FieldName, () => FieldNotFoundStaticMismatch2(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : DoesNotExist, From 9f6b5328591dfe5125fbff85e0f29b2e1b3087ba Mon Sep 17 00:00:00 2001 From: Fan Yang <52458914+fanyang-mono@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:30:53 -0400 Subject: [PATCH 30/34] Update src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs Co-authored-by: Aaron Robinson --- .../compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 7d9a12163e20e..0569156cc9cd4 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -392,7 +392,7 @@ public static void Verify_InvalidTargetUnsafeAccessor() () => FieldNotFoundStaticMismatch1(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : UserDataValue.FieldName, - () => FieldNotFoundStaticMismatch2(null)); + () => FieldNotFoundStaticMismatch2(default)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : DoesNotExist, () => StaticFieldNotFound(null)); From 0c15ccd5f8db55825f200581ed70330775ef62c2 Mon Sep 17 00:00:00 2001 From: Fan Yang <52458914+fanyang-mono@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:31:00 -0400 Subject: [PATCH 31/34] Update src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs Co-authored-by: Aaron Robinson --- .../compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 0569156cc9cd4..f8565c17a92e0 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -389,7 +389,7 @@ public static void Verify_InvalidTargetUnsafeAccessor() () => FieldNotFound(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : UserDataClass.StaticFieldName, - () => FieldNotFoundStaticMismatch1(null)); + () => FieldNotFoundStaticMismatch1(default)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : UserDataValue.FieldName, () => FieldNotFoundStaticMismatch2(default)); From f21b78e970bf6001ad755bf5359bbddecd9fd7e8 Mon Sep 17 00:00:00 2001 From: Fan Yang <52458914+fanyang-mono@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:31:07 -0400 Subject: [PATCH 32/34] Update src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs Co-authored-by: Aaron Robinson --- .../compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index f8565c17a92e0..27c3230d8e2de 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -370,7 +370,6 @@ public static void Verify_ManagedUnmanagedFunctionPointersDontMatch() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/88858", TestRuntimes.CoreCLR)] public static void Verify_InvalidTargetUnsafeAccessor() { Console.WriteLine($"Running {nameof(Verify_InvalidTargetUnsafeAccessor)}"); From 781f512ce428370b3257c05455f8cee12d4c28da Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 13 Jul 2023 15:42:35 -0400 Subject: [PATCH 33/34] Address more review feedback --- src/mono/mono/mini/aot-runtime.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index a1d1e1939021c..86c70dba38862 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -1068,12 +1068,12 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod ref->method = mono_marshal_get_array_accessor_wrapper (m); } else if (subtype == WRAPPER_SUBTYPE_UNSAFE_ACCESSOR) { MonoMethod *m = decode_resolve_method_ref (module, p, &p, error); + if (!m) + 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; - if (!m) - return FALSE; 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 (); From 17febef36c727cd7418b48082a1543c1d7c80e3d Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 13 Jul 2023 17:38:28 -0400 Subject: [PATCH 34/34] Fix function call to field --- .../UnsafeAccessors/UnsafeAccessorsTests.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 27c3230d8e2de..b155c0a445e07 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -388,7 +388,11 @@ public static void Verify_InvalidTargetUnsafeAccessor() () => FieldNotFound(null)); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : UserDataClass.StaticFieldName, - () => FieldNotFoundStaticMismatch1(default)); + () => + { + UserDataValue value = default; + FieldNotFoundStaticMismatch1(ref value); + }); AssertExtensions.ThrowsMissingMemberException( isNativeAot ? null : UserDataValue.FieldName, () => FieldNotFoundStaticMismatch2(default)); @@ -413,7 +417,7 @@ public static void Verify_InvalidTargetUnsafeAccessor() extern static ref string FieldNotFound(UserDataClass d); [UnsafeAccessor(UnsafeAccessorKind.Field, Name=UserDataValue.StaticFieldName)] - extern static ref string FieldNotFoundStaticMismatch1(UserDataValue d); + extern static ref string FieldNotFoundStaticMismatch1(ref UserDataValue d); [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name=UserDataValue.FieldName)] extern static ref string FieldNotFoundStaticMismatch2(UserDataValue d);