From 25cd4ec24b455f4d119d3a5d668c6ceb1b61dc19 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Wed, 19 Jul 2023 17:23:56 -0400 Subject: [PATCH 1/9] Support Ambiguous Match --- src/mono/mono/metadata/loader.c | 241 +++++++++++++++--- src/mono/mono/metadata/metadata-internals.h | 9 + src/mono/mono/metadata/metadata.c | 84 ++++-- .../UnsafeAccessors/UnsafeAccessorsTests.cs | 2 + 4 files changed, 286 insertions(+), 50 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index b3b7bbd1086087..dd2253ad0928b5 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -403,31 +403,6 @@ mono_field_from_token_checked (MonoImage *image, guint32 token, MonoClass **retk return field; } -static gboolean -mono_metadata_signature_vararg_match (MonoMethodSignature *sig1, MonoMethodSignature *sig2) -{ - int i; - - if (sig1->hasthis != sig2->hasthis || - sig1->sentinelpos != sig2->sentinelpos) - return FALSE; - - for (i = 0; i < sig1->sentinelpos; i++) { - MonoType *p1 = sig1->params[i]; - MonoType *p2 = sig2->params[i]; - - /*if (p1->attrs != p2->attrs) - return FALSE; - */ - if (!mono_metadata_type_equal (p1, p2)) - return FALSE; - } - - if (!mono_metadata_type_equal (sig1->ret, sig2->ret)) - return FALSE; - return TRUE; -} - static MonoMethod * find_method_in_class (MonoClass *klass, const char *name, const char *qname, const char *fqname, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) @@ -441,7 +416,7 @@ find_method_in_class (MonoClass *klass, const char *name, const char *qname, con MonoImage *klass_image = m_class_get_image (klass); /* FIXME: !mono_class_is_ginst (from_class) condition causes test failures. */ - if (m_class_get_type_token (klass) && !image_is_dynamic (klass_image) && !m_class_get_methods (klass) && !m_class_get_rank (klass) && klass == from_class && !mono_class_is_ginst (from_class)) { + if (m_class_get_type_token (klass) && !image_is_dynamic (klass_image) && !m_class_get_methods (klass) && !m_class_get_rank (klass) && klass == from_class && !mono_class_is_ginst (from_class) && (sig->call_convention != MONO_CALL_VARARG)) { int first_idx = mono_class_get_first_method_idx (klass); int mcount = mono_class_get_method_count (klass); for (i = 0; i < mcount; ++i) { @@ -466,7 +441,7 @@ find_method_in_class (MonoClass *klass, const char *name, const char *qname, con other_sig = mono_method_signature_checked (method, error); if (!is_ok (error)) //bail out if we hit a loader error return NULL; - if (other_sig && (sig->call_convention != MONO_CALL_VARARG) && mono_metadata_signature_equal (sig, other_sig)) + if (other_sig && mono_metadata_signature_equal (sig, other_sig)) return method; } } @@ -514,7 +489,7 @@ find_method_in_class (MonoClass *klass, const char *name, const char *qname, con continue; if (sig->call_convention == MONO_CALL_VARARG) { - if (mono_metadata_signature_vararg_match (sig, msig)) { + if (mono_metadata_signature_equal_vararg (sig, msig)) { matched = TRUE; break; } @@ -630,6 +605,210 @@ find_method (MonoClass *in_class, MonoClass *ic, const char* name, MonoMethodSig return result; } +static MonoMethod * +find_method_simple (MonoClass *klass, const char *name, const char *qname, const char *fqname, + MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) +{ + MonoMethod *method_maybe = NULL; + + /* FIXME: method refs from metadata-upate probably end up here */ + + /* Search directly in the metadata to avoid calling setup_methods () */ + error_init (error); + + MonoImage *klass_image = m_class_get_image (klass); + /* FIXME: !mono_class_is_ginst (from_class) condition causes test failures. */ + if (m_class_get_type_token (klass) && !image_is_dynamic (klass_image) && !m_class_get_methods (klass) && !m_class_get_rank (klass) && klass == from_class && !mono_class_is_ginst (from_class)) { + int first_idx = mono_class_get_first_method_idx (klass); + int mcount = mono_class_get_method_count (klass); + for (int i = 0; i < mcount; ++i) { + guint32 cols [MONO_METHOD_SIZE]; + MonoMethod *method; + const char *m_name; + MonoMethodSignature *other_sig; + + mono_metadata_decode_table_row (klass_image, MONO_TABLE_METHOD, first_idx + i, cols, MONO_METHOD_SIZE); + + m_name = mono_metadata_string_heap (klass_image, cols [MONO_METHOD_NAME]); + + if (!((fqname && !strcmp (m_name, fqname)) || + (qname && !strcmp (m_name, qname)) || + (name && !strcmp (m_name, name)))) + continue; + + method = mono_get_method_checked (klass_image, MONO_TOKEN_METHOD_DEF | (first_idx + i + 1), klass, NULL, error); + if (!is_ok (error)) //bail out if we hit a loader error + return NULL; + + // Check method signature + if (method) { + other_sig = mono_method_signature_checked (method, error); + if (!is_ok (error)) //bail out if we hit a loader error + return NULL; + if (other_sig) { + gboolean found = ignore_cmods ? mono_metadata_signature_equal_ignore_custom_modifier (sig, other_sig) : mono_metadata_signature_equal (sig, other_sig); + if (found) { + if (method_maybe != NULL) { + if (ignore_cmods) + return find_method_simple (klass, name, qname, fqname, sig, from_class, FALSE, error); + // Throw Arg_AmbiguousMatchException_UnsafeAccessor exception + } + method_maybe = method; + } + } + } + } + return method_maybe; + } + + return NULL; +} + +typedef struct MethodLookupResultInfo { + int i; + MonoMethod *m; + gboolean matched; +} MethodLookupResultInfo; + +static MethodLookupResultInfo * +find_method_slow (MonoClass *klass, const char *name, const char *qname, const char *fqname, + MonoMethodSignature *sig, gboolean ignore_cmods, MonoError *error) +{ + gpointer iter = NULL; + MethodLookupResultInfo *result = (MethodLookupResultInfo *)g_malloc0 (sizeof (MethodLookupResultInfo)); + int i = -1; + MonoMethod *m = NULL; + gboolean matched = FALSE; + result->i = i; + result->m = m; + result->matched = matched; + + /* FIXME: metadata-update iterating using + * mono_class_get_methods will break if `m` is NULL. Need to + * reconcile with the `if (!m)` "we must cope" comment below. + */ + while ((m = mono_class_get_methods (klass, &iter))) { + i++; + MonoMethodSignature *msig; + + /* We must cope with failing to load some of the types. */ + if (!m) + continue; + + if (!((fqname && !strcmp (m->name, fqname)) || + (qname && !strcmp (m->name, qname)) || + (name && !strcmp (m->name, name)))) + continue; + msig = mono_method_signature_checked (m, error); + if (!is_ok (error)) //bail out if we hit a loader error + return NULL; + + if (!msig) + continue; + + gboolean found = FALSE; + if (ignore_cmods) + found = sig->call_convention == MONO_CALL_VARARG ? mono_metadata_signature_equal_vararg_ignore_custom_modifier (sig, msig) : mono_metadata_signature_equal_ignore_custom_modifier (sig, msig); + else + found = sig->call_convention == MONO_CALL_VARARG ? mono_metadata_signature_equal_vararg (sig, msig) : mono_metadata_signature_equal (sig, msig); + + if (found) { + if (matched) { + if (ignore_cmods) + return find_method_slow (klass, name, qname, fqname, sig, FALSE, error); + // Throw Arg_AmbiguousMatchException_UnsafeAccessor exception + } + matched = TRUE; + result->i = i; + result->m = m; + result->matched = matched; + } + } + + return result; +} + +static MonoMethod * +find_method_in_class_unsafe_accessor (MonoClass *klass, const char *name, const char *qname, const char *fqname, + MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) +{ + MonoMethod *method = NULL; + if (sig->call_convention != MONO_CALL_VARARG) + method = find_method_simple (klass, name, qname, fqname, sig, from_class, ignore_cmods, error); + if (method) + return method; + + mono_class_setup_methods (klass); /* FIXME don't swallow the error here. */ + /* + We can't fail lookup of methods otherwise the runtime will fail with MissingMethodException instead of TypeLoadException. + See mono/tests/generic-type-load-exception.2.il + FIXME we should better report this error to the caller + */ + if (!m_class_get_methods (klass) || mono_class_has_failure (klass)) { + ERROR_DECL (cause_error); + mono_error_set_for_class_failure (cause_error, klass); + mono_error_set_type_load_class (error, klass, "Could not find method '%s' due to a type load error: %s", name, mono_error_get_message (cause_error)); + mono_error_cleanup (cause_error); + return NULL; + } + + MethodLookupResultInfo *result = find_method_slow (klass, name, qname, fqname, sig, ignore_cmods, error); + int mcount = mono_class_get_method_count (klass); + + if (result->matched) { + if (result->i < mcount) + return mono_class_get_method_by_index (from_class, result->i); + else if (result->m != NULL) { + // FIXME: metadata-update: hack + // it's from a metadata-update, probably + MonoMethod * m = mono_class_inflate_generic_method_full_checked ( + result->m, from_class, mono_class_get_context (from_class), error); + mono_error_assert_ok (error); + g_assert (m != NULL); + g_assert (m->klass == from_class); + g_assert (m->is_inflated); + return m; + } + } + + g_free (result); + return NULL; +} + +static MonoMethod * +find_method_unsafe_accessor (MonoClass *in_class, const char* name, MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) +{ + char *qname, *fqname; + MonoMethod *result_maybe = NULL; + + error_init (error); + qname = fqname = NULL; + + while (in_class) { + g_assert (from_class); + MonoMethod * result = find_method_in_class_unsafe_accessor (in_class, name, qname, fqname, sig, from_class, ignore_cmods, error); + if (!is_ok (error)) + return NULL; + + if (result) { + if (result_maybe != NULL) { + if (ignore_cmods) + return find_method_unsafe_accessor (in_class, name, sig, from_class, FALSE, error); + //Throw Arg_AmbiguousMatchException_UnsafeAccessor exception + } + result_maybe = result; + } + + // FIXME: don't support failing to lazily load the interfaces of one of the types. See find_method for more info + + in_class = m_class_get_parent (in_class); + from_class = m_class_get_parent (from_class); + } + g_assert (!in_class == !from_class); + + return result_maybe; +} + static MonoMethodSignature* inflate_generic_signature_checked (MonoImage *image, MonoMethodSignature *sig, MonoGenericContext *context, MonoError *error) { @@ -975,16 +1154,14 @@ method_from_memberref (MonoImage *image, guint32 idx, MonoGenericContext *typesp MonoMethod* mono_unsafe_accessor_find_ctor (MonoClass *in_class, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) { - // This doesn't work for constructors because find_method explicitly disallows ".ctor" and ".cctor" - //return find_method (in_class, /*ic*/NULL, name, sig, from_class, error); - return find_method_in_class (in_class, ".ctor", /*qname*/NULL, /*fqname*/NULL, sig, from_class, error); + return find_method_unsafe_accessor (in_class, ".ctor", sig, from_class, TRUE, error); } MonoMethod* mono_unsafe_accessor_find_method (MonoClass *in_class, const char *name, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) { // This doesn't work for constructors because find_method explicitly disallows ".ctor" and ".cctor" - return find_method (in_class, /*ic*/NULL, name, sig, from_class, error); + return find_method_unsafe_accessor (in_class, name, sig, from_class, TRUE, error); } diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index 98896b1c9ccc27..47890b0a6bd053 100644 --- a/src/mono/mono/metadata/metadata-internals.h +++ b/src/mono/mono/metadata/metadata-internals.h @@ -1018,6 +1018,15 @@ gboolean mono_metadata_generic_inst_equal (gconstpointer ka, gconstpointer gboolean mono_metadata_signature_equal_no_ret (MonoMethodSignature *sig1, MonoMethodSignature *sig2); +gboolean +mono_metadata_signature_equal_ignore_custom_modifier (MonoMethodSignature *sig1, MonoMethodSignature *sig2); + +gboolean +mono_metadata_signature_equal_vararg (MonoMethodSignature *sig1, MonoMethodSignature *sig2); + +gboolean +mono_metadata_signature_equal_vararg_ignore_custom_modifier (MonoMethodSignature *sig1, MonoMethodSignature *sig2); + MONO_API void mono_metadata_field_info_with_mempool ( MonoImage *meta, diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index c7398d558270bd..ab2f083c7fbabf 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -49,7 +49,7 @@ typedef struct { static gboolean do_mono_metadata_parse_type (MonoType *type, MonoImage *m, MonoGenericContainer *container, gboolean transient, const char *ptr, const char **rptr, MonoError *error); -static gboolean do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only); +static gboolean do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only, gboolean skip_cmods_check); static gboolean mono_metadata_class_equal (MonoClass *c1, MonoClass *c2, gboolean signature_only); static gboolean mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gboolean signature_only); static gboolean _mono_metadata_generic_class_equal (const MonoGenericClass *g1, const MonoGenericClass *g2, @@ -1901,7 +1901,7 @@ mono_generic_inst_equal_full (const MonoGenericInst *a, const MonoGenericInst *b if (a->is_open != b->is_open || a->type_argc != b->type_argc) return FALSE; for (guint i = 0; i < a->type_argc; ++i) { - if (!do_mono_metadata_type_equal (a->type_argv [i], b->type_argv [i], signature_only)) + if (!do_mono_metadata_type_equal (a->type_argv [i], b->type_argv [i], signature_only, FALSE)) return FALSE; } return TRUE; @@ -5690,10 +5690,10 @@ mono_metadata_class_equal (MonoClass *c1, MonoClass *c2, gboolean signature_only return mono_metadata_class_equal (c1_type->data.klass, c2_type->data.klass, signature_only); if (signature_only && (c1_type->type == MONO_TYPE_ARRAY) && (c2_type->type == MONO_TYPE_ARRAY)) - return do_mono_metadata_type_equal (c1_type, c2_type, signature_only); + return do_mono_metadata_type_equal (c1_type, c2_type, signature_only, FALSE); if (signature_only && (c1_type->type == MONO_TYPE_PTR) && (c2_type->type == MONO_TYPE_PTR)) - return do_mono_metadata_type_equal (c1_type->data.type, c2_type->data.type, signature_only); + return do_mono_metadata_type_equal (c1_type->data.type, c2_type->data.type, signature_only, FALSE); if (signature_only && (c1_type->type == MONO_TYPE_FNPTR) && (c2_type->type == MONO_TYPE_FNPTR)) return mono_metadata_fnptr_equal (c1_type->data.method, c2_type->data.method, signature_only); @@ -5715,7 +5715,7 @@ mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gbo return FALSE; if (s1->explicit_this != s2->explicit_this) return FALSE; - if (! do_mono_metadata_type_equal (s1->ret, s2->ret, signature_only)) + if (! do_mono_metadata_type_equal (s1->ret, s2->ret, signature_only, FALSE)) return FALSE; if (s1->param_count != s2->param_count) return FALSE; @@ -5726,7 +5726,7 @@ mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gbo if (t1 == NULL || t2 == NULL) return (t1 == t2); - if (! do_mono_metadata_type_equal (t1, t2, signature_only)) + if (! do_mono_metadata_type_equal (t1, t2, signature_only, FALSE)) return FALSE; } } @@ -5755,7 +5755,7 @@ mono_metadata_custom_modifiers_equal (MonoType *t1, MonoType *t2, gboolean signa if (cm1_required != cm2_required) return FALSE; - if (!do_mono_metadata_type_equal (cm1_type, cm2_type, signature_only)) + if (!do_mono_metadata_type_equal (cm1_type, cm2_type, signature_only, FALSE)) return FALSE; } return TRUE; @@ -5771,17 +5771,19 @@ mono_metadata_custom_modifiers_equal (MonoType *t1, MonoType *t2, gboolean signa * Returns: #TRUE if @t1 and @t2 are equal. */ static gboolean -do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only) +do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only, gboolean skip_cmods_check) { if (t1->type != t2->type || m_type_is_byref (t1) != m_type_is_byref (t2)) return FALSE; gboolean cmod_reject = FALSE; - if (t1->has_cmods != t2->has_cmods) - cmod_reject = TRUE; - else if (t1->has_cmods && t2->has_cmods) { - cmod_reject = !mono_metadata_custom_modifiers_equal (t1, t2, signature_only); + if (!skip_cmods_check) { + if (t1->has_cmods != t2->has_cmods) + cmod_reject = TRUE; + else if (t1->has_cmods && t2->has_cmods) { + cmod_reject = !mono_metadata_custom_modifiers_equal (t1, t2, signature_only); + } } gboolean result = FALSE; @@ -5813,7 +5815,7 @@ do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only result = mono_metadata_class_equal (t1->data.klass, t2->data.klass, signature_only); break; case MONO_TYPE_PTR: - result = do_mono_metadata_type_equal (t1->data.type, t2->data.type, signature_only); + result = do_mono_metadata_type_equal (t1->data.type, t2->data.type, signature_only, skip_cmods_check); break; case MONO_TYPE_ARRAY: if (t1->data.array->rank != t2->data.array->rank) @@ -5850,7 +5852,7 @@ do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only gboolean mono_metadata_type_equal (MonoType *t1, MonoType *t2) { - return do_mono_metadata_type_equal (t1, t2, FALSE); + return do_mono_metadata_type_equal (t1, t2, FALSE, FALSE); } /** @@ -5867,11 +5869,12 @@ mono_metadata_type_equal (MonoType *t1, MonoType *t2) gboolean mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, gboolean signature_only) { - return do_mono_metadata_type_equal (t1, t2, signature_only); + return do_mono_metadata_type_equal (t1, t2, signature_only, FALSE); } enum { SIG_EQUIV_FLAG_NO_RET = 1, + SIG_EQUIV_FLAG_IGNORE_CMODS = 2, }; gboolean @@ -5899,6 +5902,11 @@ mono_metadata_signature_equal_no_ret (MonoMethodSignature *sig1, MonoMethodSigna return signature_equiv (sig1, sig2, SIG_EQUIV_FLAG_NO_RET); } +gboolean +mono_metadata_signature_equal_ignore_custom_modifier (MonoMethodSignature *sig1, MonoMethodSignature *sig2) +{ + return signature_equiv (sig1, sig2, SIG_EQUIV_FLAG_IGNORE_CMODS); +} gboolean signature_equiv (MonoMethodSignature *sig1, MonoMethodSignature *sig2, int equiv_flags) @@ -5927,13 +5935,53 @@ signature_equiv (MonoMethodSignature *sig1, MonoMethodSignature *sig2, int equiv /* if (p1->attrs != p2->attrs) return FALSE; */ - if (!do_mono_metadata_type_equal (p1, p2, TRUE)) + if (!do_mono_metadata_type_equal (p1, p2, TRUE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) return FALSE; } - if ((equiv_flags & SIG_EQUIV_FLAG_NO_RET) != 0) + if (equiv_flags == SIG_EQUIV_FLAG_NO_RET) return TRUE; - if (!do_mono_metadata_type_equal (sig1->ret, sig2->ret, TRUE)) + if (!do_mono_metadata_type_equal (sig1->ret, sig2->ret, TRUE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) + return FALSE; + return TRUE; +} + +gboolean +signature_equiv_vararg (MonoMethodSignature *sig1, MonoMethodSignature *sig2, int equiv_flags); + +gboolean +mono_metadata_signature_equal_vararg (MonoMethodSignature *sig1, MonoMethodSignature *sig2) +{ + return signature_equiv_vararg (sig1, sig2, 0); +} + +gboolean +mono_metadata_signature_equal_vararg_ignore_custom_modifier (MonoMethodSignature *sig1, MonoMethodSignature *sig2) +{ + return signature_equiv_vararg (sig1, sig2, SIG_EQUIV_FLAG_IGNORE_CMODS); +} + +gboolean +signature_equiv_vararg (MonoMethodSignature *sig1, MonoMethodSignature *sig2, int equiv_flags) +{ + int i; + + if (sig1->hasthis != sig2->hasthis || + sig1->sentinelpos != sig2->sentinelpos) + return FALSE; + + for (i = 0; i < sig1->sentinelpos; i++) { + MonoType *p1 = sig1->params[i]; + MonoType *p2 = sig2->params[i]; + + /*if (p1->attrs != p2->attrs) + return FALSE; + */ + if (!do_mono_metadata_type_equal (p1, p2, FALSE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) + return FALSE; + } + + if (!do_mono_metadata_type_equal (sig1->ret, sig2->ret, FALSE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) return FALSE; return TRUE; } diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 5077691409fe0b..13c441ea06e3c5 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -43,6 +43,8 @@ private static void _MVV() {} private void _mvv() {} // The "init" is important to have here - custom modifier test. + // The signature of set_Prop is + // instance void modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) set_Prop ( string 'value') private string Prop { get; init; } // Used to validate ambiguity is handled via custom modifiers. From c1c3ad5e2521a1af8378b54e3b9f08786ac4ac89 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 20 Jul 2023 16:08:29 -0400 Subject: [PATCH 2/9] Stop unsafe accessor method lookup from walking the type hierarchy. And address the review feedback --- src/mono/mono/metadata/loader.c | 38 ++------------------ src/mono/mono/metadata/metadata.c | 59 ++++++++++++++++++------------- 2 files changed, 36 insertions(+), 61 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index dd2253ad0928b5..2f26346cfc5259 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -775,40 +775,6 @@ find_method_in_class_unsafe_accessor (MonoClass *klass, const char *name, const return NULL; } -static MonoMethod * -find_method_unsafe_accessor (MonoClass *in_class, const char* name, MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) -{ - char *qname, *fqname; - MonoMethod *result_maybe = NULL; - - error_init (error); - qname = fqname = NULL; - - while (in_class) { - g_assert (from_class); - MonoMethod * result = find_method_in_class_unsafe_accessor (in_class, name, qname, fqname, sig, from_class, ignore_cmods, error); - if (!is_ok (error)) - return NULL; - - if (result) { - if (result_maybe != NULL) { - if (ignore_cmods) - return find_method_unsafe_accessor (in_class, name, sig, from_class, FALSE, error); - //Throw Arg_AmbiguousMatchException_UnsafeAccessor exception - } - result_maybe = result; - } - - // FIXME: don't support failing to lazily load the interfaces of one of the types. See find_method for more info - - in_class = m_class_get_parent (in_class); - from_class = m_class_get_parent (from_class); - } - g_assert (!in_class == !from_class); - - return result_maybe; -} - static MonoMethodSignature* inflate_generic_signature_checked (MonoImage *image, MonoMethodSignature *sig, MonoGenericContext *context, MonoError *error) { @@ -1154,14 +1120,14 @@ method_from_memberref (MonoImage *image, guint32 idx, MonoGenericContext *typesp MonoMethod* mono_unsafe_accessor_find_ctor (MonoClass *in_class, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) { - return find_method_unsafe_accessor (in_class, ".ctor", sig, from_class, TRUE, error); + return find_method_in_class_unsafe_accessor (in_class, ".ctor", /*qname*/NULL, /*fqname*/NULL, sig, from_class, TRUE, error); } MonoMethod* mono_unsafe_accessor_find_method (MonoClass *in_class, const char *name, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) { // This doesn't work for constructors because find_method explicitly disallows ".ctor" and ".cctor" - return find_method_unsafe_accessor (in_class, name, sig, from_class, TRUE, error); + return find_method_in_class_unsafe_accessor (in_class, name, /*qname*/NULL, /*fqname*/NULL, sig, from_class, TRUE, error); } diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index ab2f083c7fbabf..34ccb7dba4b5a7 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -46,10 +46,15 @@ typedef struct { MonoGenericContext context; } MonoInflatedMethodSignature; +enum { + MONO_TYPE_EQ_FLAGS_SIG_ONLY = 1, + MONO_TYPE_EQ_FLAG_IGNORE_CMODS = 2 +}; + static gboolean do_mono_metadata_parse_type (MonoType *type, MonoImage *m, MonoGenericContainer *container, gboolean transient, const char *ptr, const char **rptr, MonoError *error); -static gboolean do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only, gboolean skip_cmods_check); +static gboolean do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, int equiv_flags); static gboolean mono_metadata_class_equal (MonoClass *c1, MonoClass *c2, gboolean signature_only); static gboolean mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gboolean signature_only); static gboolean _mono_metadata_generic_class_equal (const MonoGenericClass *g1, const MonoGenericClass *g2, @@ -1901,7 +1906,7 @@ mono_generic_inst_equal_full (const MonoGenericInst *a, const MonoGenericInst *b if (a->is_open != b->is_open || a->type_argc != b->type_argc) return FALSE; for (guint i = 0; i < a->type_argc; ++i) { - if (!do_mono_metadata_type_equal (a->type_argv [i], b->type_argv [i], signature_only, FALSE)) + if (!do_mono_metadata_type_equal (a->type_argv [i], b->type_argv [i], MONO_TYPE_EQ_FLAGS_SIG_ONLY)) return FALSE; } return TRUE; @@ -5690,10 +5695,10 @@ mono_metadata_class_equal (MonoClass *c1, MonoClass *c2, gboolean signature_only return mono_metadata_class_equal (c1_type->data.klass, c2_type->data.klass, signature_only); if (signature_only && (c1_type->type == MONO_TYPE_ARRAY) && (c2_type->type == MONO_TYPE_ARRAY)) - return do_mono_metadata_type_equal (c1_type, c2_type, signature_only, FALSE); + return do_mono_metadata_type_equal (c1_type, c2_type, MONO_TYPE_EQ_FLAGS_SIG_ONLY); if (signature_only && (c1_type->type == MONO_TYPE_PTR) && (c2_type->type == MONO_TYPE_PTR)) - return do_mono_metadata_type_equal (c1_type->data.type, c2_type->data.type, signature_only, FALSE); + return do_mono_metadata_type_equal (c1_type->data.type, c2_type->data.type, MONO_TYPE_EQ_FLAGS_SIG_ONLY); if (signature_only && (c1_type->type == MONO_TYPE_FNPTR) && (c2_type->type == MONO_TYPE_FNPTR)) return mono_metadata_fnptr_equal (c1_type->data.method, c2_type->data.method, signature_only); @@ -5715,7 +5720,7 @@ mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gbo return FALSE; if (s1->explicit_this != s2->explicit_this) return FALSE; - if (! do_mono_metadata_type_equal (s1->ret, s2->ret, signature_only, FALSE)) + if (! do_mono_metadata_type_equal (s1->ret, s2->ret, MONO_TYPE_EQ_FLAGS_SIG_ONLY)) return FALSE; if (s1->param_count != s2->param_count) return FALSE; @@ -5726,7 +5731,7 @@ mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gbo if (t1 == NULL || t2 == NULL) return (t1 == t2); - if (! do_mono_metadata_type_equal (t1, t2, signature_only, FALSE)) + if (! do_mono_metadata_type_equal (t1, t2, MONO_TYPE_EQ_FLAGS_SIG_ONLY)) return FALSE; } } @@ -5755,7 +5760,7 @@ mono_metadata_custom_modifiers_equal (MonoType *t1, MonoType *t2, gboolean signa if (cm1_required != cm2_required) return FALSE; - if (!do_mono_metadata_type_equal (cm1_type, cm2_type, signature_only, FALSE)) + if (!do_mono_metadata_type_equal (cm1_type, cm2_type, MONO_TYPE_EQ_FLAGS_SIG_ONLY)) return FALSE; } return TRUE; @@ -5771,18 +5776,18 @@ mono_metadata_custom_modifiers_equal (MonoType *t1, MonoType *t2, gboolean signa * Returns: #TRUE if @t1 and @t2 are equal. */ static gboolean -do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only, gboolean skip_cmods_check) +do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, int equiv_flags) { if (t1->type != t2->type || m_type_is_byref (t1) != m_type_is_byref (t2)) return FALSE; gboolean cmod_reject = FALSE; - if (!skip_cmods_check) { + if ((equiv_flags & MONO_TYPE_EQ_FLAG_IGNORE_CMODS) == 0) { if (t1->has_cmods != t2->has_cmods) cmod_reject = TRUE; else if (t1->has_cmods && t2->has_cmods) { - cmod_reject = !mono_metadata_custom_modifiers_equal (t1, t2, signature_only); + cmod_reject = !mono_metadata_custom_modifiers_equal (t1, t2, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); } } @@ -5812,31 +5817,31 @@ do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only case MONO_TYPE_VALUETYPE: case MONO_TYPE_CLASS: case MONO_TYPE_SZARRAY: - result = mono_metadata_class_equal (t1->data.klass, t2->data.klass, signature_only); + result = mono_metadata_class_equal (t1->data.klass, t2->data.klass, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); break; case MONO_TYPE_PTR: - result = do_mono_metadata_type_equal (t1->data.type, t2->data.type, signature_only, skip_cmods_check); + result = do_mono_metadata_type_equal (t1->data.type, t2->data.type, equiv_flags); break; case MONO_TYPE_ARRAY: if (t1->data.array->rank != t2->data.array->rank) result = FALSE; else - result = mono_metadata_class_equal (t1->data.array->eklass, t2->data.array->eklass, signature_only); + result = mono_metadata_class_equal (t1->data.array->eklass, t2->data.array->eklass, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); break; case MONO_TYPE_GENERICINST: result = _mono_metadata_generic_class_equal ( - t1->data.generic_class, t2->data.generic_class, signature_only); + t1->data.generic_class, t2->data.generic_class, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); break; case MONO_TYPE_VAR: result = mono_metadata_generic_param_equal_internal ( - t1->data.generic_param, t2->data.generic_param, signature_only); + t1->data.generic_param, t2->data.generic_param, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); break; case MONO_TYPE_MVAR: result = mono_metadata_generic_param_equal_internal ( - t1->data.generic_param, t2->data.generic_param, signature_only); + t1->data.generic_param, t2->data.generic_param, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); break; case MONO_TYPE_FNPTR: - result = mono_metadata_fnptr_equal (t1->data.method, t2->data.method, signature_only); + result = mono_metadata_fnptr_equal (t1->data.method, t2->data.method, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); break; default: g_error ("implement type compare for %0x!", t1->type); @@ -5852,7 +5857,7 @@ do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, gboolean signature_only gboolean mono_metadata_type_equal (MonoType *t1, MonoType *t2) { - return do_mono_metadata_type_equal (t1, t2, FALSE, FALSE); + return do_mono_metadata_type_equal (t1, t2, 0); } /** @@ -5869,12 +5874,12 @@ mono_metadata_type_equal (MonoType *t1, MonoType *t2) gboolean mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, gboolean signature_only) { - return do_mono_metadata_type_equal (t1, t2, signature_only, FALSE); + return do_mono_metadata_type_equal (t1, t2, MONO_TYPE_EQ_FLAGS_SIG_ONLY); } enum { SIG_EQUIV_FLAG_NO_RET = 1, - SIG_EQUIV_FLAG_IGNORE_CMODS = 2, + SIG_EQUIV_FLAG_IGNORE_CMODS = 2 }; gboolean @@ -5919,6 +5924,8 @@ signature_equiv (MonoMethodSignature *sig1, MonoMethodSignature *sig2, int equiv if (sig1->generic_param_count != sig2->generic_param_count) return FALSE; + int flag = MONO_TYPE_EQ_FLAGS_SIG_ONLY | (((equiv_flags & SIG_EQUIV_FLAG_IGNORE_CMODS) != 0) ? MONO_TYPE_EQ_FLAG_IGNORE_CMODS : 0); + /* * We're just comparing the signatures of two methods here: * @@ -5935,13 +5942,13 @@ signature_equiv (MonoMethodSignature *sig1, MonoMethodSignature *sig2, int equiv /* if (p1->attrs != p2->attrs) return FALSE; */ - if (!do_mono_metadata_type_equal (p1, p2, TRUE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) + if (!do_mono_metadata_type_equal (p1, p2, flag)) return FALSE; } - if (equiv_flags == SIG_EQUIV_FLAG_NO_RET) + if ((equiv_flags & SIG_EQUIV_FLAG_NO_RET) != 0) return TRUE; - if (!do_mono_metadata_type_equal (sig1->ret, sig2->ret, TRUE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) + if (!do_mono_metadata_type_equal (sig1->ret, sig2->ret, flag)) return FALSE; return TRUE; } @@ -5969,6 +5976,8 @@ signature_equiv_vararg (MonoMethodSignature *sig1, MonoMethodSignature *sig2, in if (sig1->hasthis != sig2->hasthis || sig1->sentinelpos != sig2->sentinelpos) return FALSE; + + int flag = MONO_TYPE_EQ_FLAGS_SIG_ONLY | (((equiv_flags & SIG_EQUIV_FLAG_IGNORE_CMODS) != 0) ? MONO_TYPE_EQ_FLAG_IGNORE_CMODS : 0); for (i = 0; i < sig1->sentinelpos; i++) { MonoType *p1 = sig1->params[i]; @@ -5977,11 +5986,11 @@ signature_equiv_vararg (MonoMethodSignature *sig1, MonoMethodSignature *sig2, in /*if (p1->attrs != p2->attrs) return FALSE; */ - if (!do_mono_metadata_type_equal (p1, p2, FALSE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) + if (!do_mono_metadata_type_equal (p1, p2, flag)) return FALSE; } - if (!do_mono_metadata_type_equal (sig1->ret, sig2->ret, FALSE, equiv_flags == SIG_EQUIV_FLAG_IGNORE_CMODS)) + if (!do_mono_metadata_type_equal (sig1->ret, sig2->ret, flag)) return FALSE; return TRUE; } From b3d67026f103a97f6487d20450cdc71bfaa9d218 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Thu, 20 Jul 2023 17:17:47 -0400 Subject: [PATCH 3/9] Handle the AmbiguousMatchException --- src/mono/mono/metadata/loader.c | 12 ++++++++++-- src/mono/mono/metadata/marshal-lightweight.c | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 2f26346cfc5259..1f3dd2922c2bb4 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -651,7 +651,8 @@ find_method_simple (MonoClass *klass, const char *name, const char *qname, const if (method_maybe != NULL) { if (ignore_cmods) return find_method_simple (klass, name, qname, fqname, sig, from_class, FALSE, error); - // Throw Arg_AmbiguousMatchException_UnsafeAccessor exception + mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); + return NULL; } method_maybe = method; } @@ -716,7 +717,8 @@ find_method_slow (MonoClass *klass, const char *name, const char *qname, const c if (matched) { if (ignore_cmods) return find_method_slow (klass, name, qname, fqname, sig, FALSE, error); - // Throw Arg_AmbiguousMatchException_UnsafeAccessor exception + mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); + return NULL; } matched = TRUE; result->i = i; @@ -737,6 +739,8 @@ find_method_in_class_unsafe_accessor (MonoClass *klass, const char *name, const method = find_method_simple (klass, name, qname, fqname, sig, from_class, ignore_cmods, error); if (method) return method; + if (!is_ok(error) && mono_error_get_error_code (error) == MONO_ERROR_GENERIC) + return NULL; mono_class_setup_methods (klass); /* FIXME don't swallow the error here. */ /* @@ -753,8 +757,12 @@ find_method_in_class_unsafe_accessor (MonoClass *klass, const char *name, const } MethodLookupResultInfo *result = find_method_slow (klass, name, qname, fqname, sig, ignore_cmods, error); + if (!is_ok(error) && mono_error_get_error_code (error) == MONO_ERROR_GENERIC) + return NULL; + int mcount = mono_class_get_method_count (klass); + g_assert (result != NULL); if (result->matched) { if (result->i < mcount) return mono_class_get_method_by_index (from_class, result->i); diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index b117f545e14f92..192c04c1c39b2a 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2452,7 +2452,11 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m MonoClass *in_class = mono_class_is_ginst (target_class) ? mono_class_get_generic_class (target_class)->container_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) { - emit_missing_method_error (mb, find_method_error, "constructor"); + if (mono_error_get_error_code (find_method_error) == MONO_ERROR_GENERIC) + mono_mb_emit_exception_for_error (mb, find_method_error); + else { + emit_missing_method_error (mb, find_method_error, "constructor"); + } mono_error_cleanup (find_method_error); return; } @@ -2497,7 +2501,11 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor else 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) { - emit_missing_method_error (mb, find_method_error, member_name); + if (mono_error_get_error_code (find_method_error) == MONO_ERROR_GENERIC) + mono_mb_emit_exception_for_error (mb, find_method_error); + else { + emit_missing_method_error (mb, find_method_error, member_name); + } mono_error_cleanup (find_method_error); return; } From 898a294ab80902d45d6552c7feb1483123c76e84 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Fri, 21 Jul 2023 13:38:36 -0400 Subject: [PATCH 4/9] Fix test failure --- src/mono/mono/metadata/metadata.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 34ccb7dba4b5a7..d4f5651f97db50 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -1906,7 +1906,7 @@ mono_generic_inst_equal_full (const MonoGenericInst *a, const MonoGenericInst *b if (a->is_open != b->is_open || a->type_argc != b->type_argc) return FALSE; for (guint i = 0; i < a->type_argc; ++i) { - if (!do_mono_metadata_type_equal (a->type_argv [i], b->type_argv [i], MONO_TYPE_EQ_FLAGS_SIG_ONLY)) + if (!do_mono_metadata_type_equal (a->type_argv [i], b->type_argv [i], signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0)) return FALSE; } return TRUE; @@ -5695,10 +5695,10 @@ mono_metadata_class_equal (MonoClass *c1, MonoClass *c2, gboolean signature_only return mono_metadata_class_equal (c1_type->data.klass, c2_type->data.klass, signature_only); if (signature_only && (c1_type->type == MONO_TYPE_ARRAY) && (c2_type->type == MONO_TYPE_ARRAY)) - return do_mono_metadata_type_equal (c1_type, c2_type, MONO_TYPE_EQ_FLAGS_SIG_ONLY); + return do_mono_metadata_type_equal (c1_type, c2_type, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0); if (signature_only && (c1_type->type == MONO_TYPE_PTR) && (c2_type->type == MONO_TYPE_PTR)) - return do_mono_metadata_type_equal (c1_type->data.type, c2_type->data.type, MONO_TYPE_EQ_FLAGS_SIG_ONLY); + return do_mono_metadata_type_equal (c1_type->data.type, c2_type->data.type, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0); if (signature_only && (c1_type->type == MONO_TYPE_FNPTR) && (c2_type->type == MONO_TYPE_FNPTR)) return mono_metadata_fnptr_equal (c1_type->data.method, c2_type->data.method, signature_only); @@ -5720,7 +5720,7 @@ mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gbo return FALSE; if (s1->explicit_this != s2->explicit_this) return FALSE; - if (! do_mono_metadata_type_equal (s1->ret, s2->ret, MONO_TYPE_EQ_FLAGS_SIG_ONLY)) + if (! do_mono_metadata_type_equal (s1->ret, s2->ret, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0)) return FALSE; if (s1->param_count != s2->param_count) return FALSE; @@ -5731,7 +5731,7 @@ mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gbo if (t1 == NULL || t2 == NULL) return (t1 == t2); - if (! do_mono_metadata_type_equal (t1, t2, MONO_TYPE_EQ_FLAGS_SIG_ONLY)) + if (! do_mono_metadata_type_equal (t1, t2, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0)) return FALSE; } } @@ -5760,7 +5760,7 @@ mono_metadata_custom_modifiers_equal (MonoType *t1, MonoType *t2, gboolean signa if (cm1_required != cm2_required) return FALSE; - if (!do_mono_metadata_type_equal (cm1_type, cm2_type, MONO_TYPE_EQ_FLAGS_SIG_ONLY)) + if (!do_mono_metadata_type_equal (cm1_type, cm2_type, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0)) return FALSE; } return TRUE; @@ -5874,7 +5874,7 @@ mono_metadata_type_equal (MonoType *t1, MonoType *t2) gboolean mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, gboolean signature_only) { - return do_mono_metadata_type_equal (t1, t2, MONO_TYPE_EQ_FLAGS_SIG_ONLY); + return do_mono_metadata_type_equal (t1, t2, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0); } enum { From a162aa86df5773f54b73137cc1749f00b4779296 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 24 Jul 2023 12:14:04 -0400 Subject: [PATCH 5/9] Adjust Mono function pointer comparison behavior to match with CoreCLR --- src/mono/mono/metadata/metadata.c | 42 +++++++++++++++---- .../UnsafeAccessors/UnsafeAccessorsTests.cs | 3 -- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index d4f5651f97db50..c5f9d781bafb98 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -56,7 +56,7 @@ static gboolean do_mono_metadata_parse_type (MonoType *type, MonoImage *m, MonoG static gboolean do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, int equiv_flags); static gboolean mono_metadata_class_equal (MonoClass *c1, MonoClass *c2, gboolean signature_only); -static gboolean mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gboolean signature_only); +static gboolean mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, int equiv_flags); static gboolean _mono_metadata_generic_class_equal (const MonoGenericClass *g1, const MonoGenericClass *g2, gboolean signature_only); static void free_generic_inst (MonoGenericInst *ginst); @@ -5701,26 +5701,52 @@ mono_metadata_class_equal (MonoClass *c1, MonoClass *c2, gboolean signature_only return do_mono_metadata_type_equal (c1_type->data.type, c2_type->data.type, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0); if (signature_only && (c1_type->type == MONO_TYPE_FNPTR) && (c2_type->type == MONO_TYPE_FNPTR)) - return mono_metadata_fnptr_equal (c1_type->data.method, c2_type->data.method, signature_only); + return mono_metadata_fnptr_equal (c1_type->data.method, c2_type->data.method, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0); return FALSE; } +static int +mono_metadata_check_call_convention_category (unsigned int call_convention) +{ + switch (call_convention) { + case MONO_CALL_DEFAULT: + return 1; + case MONO_CALL_C: + case MONO_CALL_STDCALL: + case MONO_CALL_THISCALL: + case MONO_CALL_FASTCALL: + case MONO_CALL_UNMANAGED_MD: + return 2; + case MONO_CALL_VARARG: + return 3; + default: + g_assert_not_reached (); + } +} + static gboolean -mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gboolean signature_only) +mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, int equiv_flags) { gpointer iter1 = 0, iter2 = 0; if (s1 == s2) return TRUE; - if (s1->call_convention != s2->call_convention) - return FALSE; + + if ((equiv_flags & MONO_TYPE_EQ_FLAG_IGNORE_CMODS) == 0) { + if (s1->call_convention != s2->call_convention) + return FALSE; + } else { + if (mono_metadata_check_call_convention_category (s1->call_convention) != mono_metadata_check_call_convention_category (s2->call_convention)) + return FALSE; + } + if (s1->sentinelpos != s2->sentinelpos) return FALSE; if (s1->hasthis != s2->hasthis) return FALSE; if (s1->explicit_this != s2->explicit_this) return FALSE; - if (! do_mono_metadata_type_equal (s1->ret, s2->ret, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0)) + if (! do_mono_metadata_type_equal (s1->ret, s2->ret, equiv_flags)) return FALSE; if (s1->param_count != s2->param_count) return FALSE; @@ -5731,7 +5757,7 @@ mono_metadata_fnptr_equal (MonoMethodSignature *s1, MonoMethodSignature *s2, gbo if (t1 == NULL || t2 == NULL) return (t1 == t2); - if (! do_mono_metadata_type_equal (t1, t2, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0)) + if (! do_mono_metadata_type_equal (t1, t2, equiv_flags)) return FALSE; } } @@ -5841,7 +5867,7 @@ do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, int equiv_flags) t1->data.generic_param, t2->data.generic_param, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); break; case MONO_TYPE_FNPTR: - result = mono_metadata_fnptr_equal (t1->data.method, t2->data.method, (equiv_flags & MONO_TYPE_EQ_FLAGS_SIG_ONLY) != 0); + result = mono_metadata_fnptr_equal (t1->data.method, t2->data.method, equiv_flags); break; default: g_error ("implement type compare for %0x!", t1->type); diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 13c441ea06e3c5..2c3dc3da2681bd 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -302,7 +302,6 @@ public static void Verify_AccessMethodValue() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_IgnoreCustomModifier() { Console.WriteLine($"Running {nameof(Verify_IgnoreCustomModifier)}"); @@ -335,7 +334,6 @@ public static void Verify_PreciseMatchCustomModifier() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_UnmanagedCallConvBitAreTreatedAsCustomModifiersAndIgnored() { Console.WriteLine($"Running {nameof(Verify_UnmanagedCallConvBitAreTreatedAsCustomModifiersAndIgnored)}"); @@ -357,7 +355,6 @@ public static void Verify_UnmanagedCallConvBitAreTreatedAsCustomModifiersAndIgno } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_ManagedUnmanagedFunctionPointersDontMatch() { Console.WriteLine($"Running {nameof(Verify_ManagedUnmanagedFunctionPointersDontMatch)}"); From a22cd759e825000827ffef5af22da2cc2d252cb4 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 24 Jul 2023 17:49:25 -0400 Subject: [PATCH 6/9] Fix ambiguous match --- src/mono/mono/metadata/loader.c | 14 ++++++++++---- src/mono/mono/metadata/method-builder-ilgen.c | 2 +- src/mono/mono/utils/mono-error-internals.h | 3 +++ src/mono/mono/utils/mono-error.c | 11 +++++++++++ .../UnsafeAccessors/UnsafeAccessorsTests.cs | 3 ++- 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 1f3dd2922c2bb4..5d5a72b8bbaf64 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -649,8 +649,11 @@ find_method_simple (MonoClass *klass, const char *name, const char *qname, const gboolean found = ignore_cmods ? mono_metadata_signature_equal_ignore_custom_modifier (sig, other_sig) : mono_metadata_signature_equal (sig, other_sig); if (found) { if (method_maybe != NULL) { - if (ignore_cmods) - return find_method_simple (klass, name, qname, fqname, sig, from_class, FALSE, error); + if (ignore_cmods) { + MonoMethod *precise_match = find_method_simple (klass, name, qname, fqname, sig, from_class, FALSE, error); + if (precise_match) + return precise_match; + } mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); return NULL; } @@ -715,8 +718,11 @@ find_method_slow (MonoClass *klass, const char *name, const char *qname, const c if (found) { if (matched) { - if (ignore_cmods) - return find_method_slow (klass, name, qname, fqname, sig, FALSE, error); + if (ignore_cmods) { + MethodLookupResultInfo *precise_match = find_method_slow (klass, name, qname, fqname, sig, FALSE, error); + if (precise_match->m) + return precise_match; + } mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); return NULL; } diff --git a/src/mono/mono/metadata/method-builder-ilgen.c b/src/mono/mono/metadata/method-builder-ilgen.c index b5e1f0974bdcab..d96e0330b9eb71 100644 --- a/src/mono/mono/metadata/method-builder-ilgen.c +++ b/src/mono/mono/metadata/method-builder-ilgen.c @@ -607,7 +607,7 @@ mono_mb_emit_exception_for_error (MonoMethodBuilder *mb, MonoError *error) g_assert (mono_error_get_error_code (error) == MONO_ERROR_GENERIC && "Unsupported error code."); /* Have to copy the message because it will be referenced from JITed code while the MonoError may be freed. */ char *msg = mono_mb_strdup (mb, mono_error_get_message (error)); - mono_mb_emit_exception_full (mb, "System", mono_error_get_exception_name (error), msg); + mono_mb_emit_exception_full (mb, mono_error_get_exception_name_space (error), mono_error_get_exception_name (error), msg); } /** diff --git a/src/mono/mono/utils/mono-error-internals.h b/src/mono/mono/utils/mono-error-internals.h index 6ed41ef1945c5e..3052ed1d84e223 100644 --- a/src/mono/mono/utils/mono-error-internals.h +++ b/src/mono/mono/utils/mono-error-internals.h @@ -313,6 +313,9 @@ mono_error_set_from_boxed (MonoError *error, const MonoErrorBoxed *from); const char* mono_error_get_exception_name (MonoError *oerror); +const char* +mono_error_get_exception_name_space (MonoError *oerror); + void mono_error_set_specific (MonoError *error, int error_code, const char *missing_method); diff --git a/src/mono/mono/utils/mono-error.c b/src/mono/mono/utils/mono-error.c index 821fc33c5cd2a6..ab81a67cc3eaaf 100644 --- a/src/mono/mono/utils/mono-error.c +++ b/src/mono/mono/utils/mono-error.c @@ -209,6 +209,17 @@ mono_error_get_exception_name (MonoError *oerror) return error->exception_name; } +const char* +mono_error_get_exception_name_space (MonoError *oerror) +{ + MonoErrorInternal *error = (MonoErrorInternal*)oerror; + + if (error->error_code == MONO_ERROR_NONE) + return NULL; + + return error->exception_name_space; +} + /*Return a pointer to the internal error message, might be NULL. Caller should not release it.*/ const char* diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index 2c3dc3da2681bd..7a8284a9bf7735 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -485,9 +485,10 @@ public static void Verify_InvalidTargetUnsafeAccessor() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/86040", TestRuntimes.Mono)] public static void Verify_InvalidTargetUnsafeAccessorAmbiguousMatch() { + Console.WriteLine($"Running {nameof(Verify_InvalidTargetUnsafeAccessorAmbiguousMatch)}"); + Assert.Throws( () => CallAmbiguousMethod(CallPrivateConstructorClass(), null)); From 2af119b0bad8c479fdd76f55a75ca2fe0b7732f1 Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 24 Jul 2023 20:00:24 -0400 Subject: [PATCH 7/9] Enable Verify_InheritanceMethodResolution --- .../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 6b3bbef8e26c98..af69c37545b8f1 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -392,7 +392,6 @@ class InheritanceDerived : InheritanceBase } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/89212", TestRuntimes.Mono)] public static void Verify_InheritanceMethodResolution() { Console.WriteLine($"Running {nameof(Verify_InheritanceMethodResolution)}"); From 94faf8f8086745de40899b4578f36036374e496e Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Mon, 24 Jul 2023 20:26:33 -0400 Subject: [PATCH 8/9] Remove redundant curly braces --- src/mono/mono/metadata/marshal-lightweight.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 485f22cbd1f5be..9a0b55f3e4d606 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2448,9 +2448,8 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m if (!is_ok (find_method_error) || target_method == NULL) { if (mono_error_get_error_code (find_method_error) == MONO_ERROR_GENERIC) mono_mb_emit_exception_for_error (mb, find_method_error); - else { + else emit_missing_method_error (mb, find_method_error, "constructor"); - } mono_error_cleanup (find_method_error); return; } @@ -2497,9 +2496,8 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor if (!is_ok (find_method_error) || target_method == NULL) { if (mono_error_get_error_code (find_method_error) == MONO_ERROR_GENERIC) mono_mb_emit_exception_for_error (mb, find_method_error); - else { + else emit_missing_method_error (mb, find_method_error, member_name); - } mono_error_cleanup (find_method_error); return; } From fe6c916bcac47518c4c7748059c0dac205d2b13a Mon Sep 17 00:00:00 2001 From: Fan Yang Date: Tue, 25 Jul 2023 11:21:38 -0400 Subject: [PATCH 9/9] Address review feedback --- src/mono/mono/metadata/loader.c | 198 ---------------------- src/mono/mono/metadata/metadata.c | 4 +- src/mono/mono/metadata/unsafe-accessor.c | 207 ++++++++++++++++++++++- 3 files changed, 207 insertions(+), 202 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 5d5a72b8bbaf64..5f21d3371c3c1f 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -605,190 +605,6 @@ find_method (MonoClass *in_class, MonoClass *ic, const char* name, MonoMethodSig return result; } -static MonoMethod * -find_method_simple (MonoClass *klass, const char *name, const char *qname, const char *fqname, - MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) -{ - MonoMethod *method_maybe = NULL; - - /* FIXME: method refs from metadata-upate probably end up here */ - - /* Search directly in the metadata to avoid calling setup_methods () */ - error_init (error); - - MonoImage *klass_image = m_class_get_image (klass); - /* FIXME: !mono_class_is_ginst (from_class) condition causes test failures. */ - if (m_class_get_type_token (klass) && !image_is_dynamic (klass_image) && !m_class_get_methods (klass) && !m_class_get_rank (klass) && klass == from_class && !mono_class_is_ginst (from_class)) { - int first_idx = mono_class_get_first_method_idx (klass); - int mcount = mono_class_get_method_count (klass); - for (int i = 0; i < mcount; ++i) { - guint32 cols [MONO_METHOD_SIZE]; - MonoMethod *method; - const char *m_name; - MonoMethodSignature *other_sig; - - mono_metadata_decode_table_row (klass_image, MONO_TABLE_METHOD, first_idx + i, cols, MONO_METHOD_SIZE); - - m_name = mono_metadata_string_heap (klass_image, cols [MONO_METHOD_NAME]); - - if (!((fqname && !strcmp (m_name, fqname)) || - (qname && !strcmp (m_name, qname)) || - (name && !strcmp (m_name, name)))) - continue; - - method = mono_get_method_checked (klass_image, MONO_TOKEN_METHOD_DEF | (first_idx + i + 1), klass, NULL, error); - if (!is_ok (error)) //bail out if we hit a loader error - return NULL; - - // Check method signature - if (method) { - other_sig = mono_method_signature_checked (method, error); - if (!is_ok (error)) //bail out if we hit a loader error - return NULL; - if (other_sig) { - gboolean found = ignore_cmods ? mono_metadata_signature_equal_ignore_custom_modifier (sig, other_sig) : mono_metadata_signature_equal (sig, other_sig); - if (found) { - if (method_maybe != NULL) { - if (ignore_cmods) { - MonoMethod *precise_match = find_method_simple (klass, name, qname, fqname, sig, from_class, FALSE, error); - if (precise_match) - return precise_match; - } - mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); - return NULL; - } - method_maybe = method; - } - } - } - } - return method_maybe; - } - - return NULL; -} - -typedef struct MethodLookupResultInfo { - int i; - MonoMethod *m; - gboolean matched; -} MethodLookupResultInfo; - -static MethodLookupResultInfo * -find_method_slow (MonoClass *klass, const char *name, const char *qname, const char *fqname, - MonoMethodSignature *sig, gboolean ignore_cmods, MonoError *error) -{ - gpointer iter = NULL; - MethodLookupResultInfo *result = (MethodLookupResultInfo *)g_malloc0 (sizeof (MethodLookupResultInfo)); - int i = -1; - MonoMethod *m = NULL; - gboolean matched = FALSE; - result->i = i; - result->m = m; - result->matched = matched; - - /* FIXME: metadata-update iterating using - * mono_class_get_methods will break if `m` is NULL. Need to - * reconcile with the `if (!m)` "we must cope" comment below. - */ - while ((m = mono_class_get_methods (klass, &iter))) { - i++; - MonoMethodSignature *msig; - - /* We must cope with failing to load some of the types. */ - if (!m) - continue; - - if (!((fqname && !strcmp (m->name, fqname)) || - (qname && !strcmp (m->name, qname)) || - (name && !strcmp (m->name, name)))) - continue; - msig = mono_method_signature_checked (m, error); - if (!is_ok (error)) //bail out if we hit a loader error - return NULL; - - if (!msig) - continue; - - gboolean found = FALSE; - if (ignore_cmods) - found = sig->call_convention == MONO_CALL_VARARG ? mono_metadata_signature_equal_vararg_ignore_custom_modifier (sig, msig) : mono_metadata_signature_equal_ignore_custom_modifier (sig, msig); - else - found = sig->call_convention == MONO_CALL_VARARG ? mono_metadata_signature_equal_vararg (sig, msig) : mono_metadata_signature_equal (sig, msig); - - if (found) { - if (matched) { - if (ignore_cmods) { - MethodLookupResultInfo *precise_match = find_method_slow (klass, name, qname, fqname, sig, FALSE, error); - if (precise_match->m) - return precise_match; - } - mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); - return NULL; - } - matched = TRUE; - result->i = i; - result->m = m; - result->matched = matched; - } - } - - return result; -} - -static MonoMethod * -find_method_in_class_unsafe_accessor (MonoClass *klass, const char *name, const char *qname, const char *fqname, - MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) -{ - MonoMethod *method = NULL; - if (sig->call_convention != MONO_CALL_VARARG) - method = find_method_simple (klass, name, qname, fqname, sig, from_class, ignore_cmods, error); - if (method) - return method; - if (!is_ok(error) && mono_error_get_error_code (error) == MONO_ERROR_GENERIC) - return NULL; - - mono_class_setup_methods (klass); /* FIXME don't swallow the error here. */ - /* - We can't fail lookup of methods otherwise the runtime will fail with MissingMethodException instead of TypeLoadException. - See mono/tests/generic-type-load-exception.2.il - FIXME we should better report this error to the caller - */ - if (!m_class_get_methods (klass) || mono_class_has_failure (klass)) { - ERROR_DECL (cause_error); - mono_error_set_for_class_failure (cause_error, klass); - mono_error_set_type_load_class (error, klass, "Could not find method '%s' due to a type load error: %s", name, mono_error_get_message (cause_error)); - mono_error_cleanup (cause_error); - return NULL; - } - - MethodLookupResultInfo *result = find_method_slow (klass, name, qname, fqname, sig, ignore_cmods, error); - if (!is_ok(error) && mono_error_get_error_code (error) == MONO_ERROR_GENERIC) - return NULL; - - int mcount = mono_class_get_method_count (klass); - - g_assert (result != NULL); - if (result->matched) { - if (result->i < mcount) - return mono_class_get_method_by_index (from_class, result->i); - else if (result->m != NULL) { - // FIXME: metadata-update: hack - // it's from a metadata-update, probably - MonoMethod * m = mono_class_inflate_generic_method_full_checked ( - result->m, from_class, mono_class_get_context (from_class), error); - mono_error_assert_ok (error); - g_assert (m != NULL); - g_assert (m->klass == from_class); - g_assert (m->is_inflated); - return m; - } - } - - g_free (result); - return NULL; -} - static MonoMethodSignature* inflate_generic_signature_checked (MonoImage *image, MonoMethodSignature *sig, MonoGenericContext *context, MonoError *error) { @@ -1131,20 +947,6 @@ method_from_memberref (MonoImage *image, guint32 idx, MonoGenericContext *typesp return NULL; } -MonoMethod* -mono_unsafe_accessor_find_ctor (MonoClass *in_class, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) -{ - return find_method_in_class_unsafe_accessor (in_class, ".ctor", /*qname*/NULL, /*fqname*/NULL, sig, from_class, TRUE, error); -} - -MonoMethod* -mono_unsafe_accessor_find_method (MonoClass *in_class, const char *name, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) -{ - // This doesn't work for constructors because find_method explicitly disallows ".ctor" and ".cctor" - return find_method_in_class_unsafe_accessor (in_class, name, /*qname*/NULL, /*fqname*/NULL, sig, from_class, TRUE, error); -} - - static MonoMethod * method_from_methodspec (MonoImage *image, MonoGenericContext *context, guint32 idx, MonoError *error) { diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index c5f9d781bafb98..7c7cd4433eecf6 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -48,7 +48,7 @@ typedef struct { enum { MONO_TYPE_EQ_FLAGS_SIG_ONLY = 1, - MONO_TYPE_EQ_FLAG_IGNORE_CMODS = 2 + MONO_TYPE_EQ_FLAG_IGNORE_CMODS = 2, }; static gboolean do_mono_metadata_parse_type (MonoType *type, MonoImage *m, MonoGenericContainer *container, gboolean transient, @@ -5905,7 +5905,7 @@ mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, gboolean signature_on enum { SIG_EQUIV_FLAG_NO_RET = 1, - SIG_EQUIV_FLAG_IGNORE_CMODS = 2 + SIG_EQUIV_FLAG_IGNORE_CMODS = 2, }; gboolean diff --git a/src/mono/mono/metadata/unsafe-accessor.c b/src/mono/mono/metadata/unsafe-accessor.c index 070b05d93d5705..e7287a00c46175 100644 --- a/src/mono/mono/metadata/unsafe-accessor.c +++ b/src/mono/mono/metadata/unsafe-accessor.c @@ -4,7 +4,210 @@ #include "config.h" #include +#include +#include +#include "mono/metadata/metadata.h" +#include "mono/metadata/image.h" +#include "mono/metadata/tokentype.h" +#include "mono/metadata/metadata-internals.h" +#include "mono/metadata/class-init.h" +#include "mono/metadata/class-internals.h" +#include "mono/utils/mono-error-internals.h" #include "mono/metadata/unsafe-accessor.h" -#include "mono/utils/mono-compiler.h" -MONO_EMPTY_SOURCE_FILE (unsafe_accessor) + + +static MonoMethod * +find_method_simple (MonoClass *klass, const char *name, const char *qname, const char *fqname, + MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) +{ + MonoMethod *method_maybe = NULL; + + /* Search directly in the metadata to avoid calling setup_methods () */ + error_init (error); + + MonoImage *klass_image = m_class_get_image (klass); + /* FIXME: !mono_class_is_ginst (from_class) condition causes test failures. */ + if (m_class_get_type_token (klass) && !image_is_dynamic (klass_image) && !m_class_get_methods (klass) && !m_class_get_rank (klass) && klass == from_class && !mono_class_is_ginst (from_class)) { + int first_idx = mono_class_get_first_method_idx (klass); + int mcount = mono_class_get_method_count (klass); + for (int i = 0; i < mcount; ++i) { + guint32 cols [MONO_METHOD_SIZE]; + MonoMethod *method; + const char *m_name; + MonoMethodSignature *other_sig; + + mono_metadata_decode_table_row (klass_image, MONO_TABLE_METHOD, first_idx + i, cols, MONO_METHOD_SIZE); + + m_name = mono_metadata_string_heap (klass_image, cols [MONO_METHOD_NAME]); + + if (!((fqname && !strcmp (m_name, fqname)) || + (qname && !strcmp (m_name, qname)) || + (name && !strcmp (m_name, name)))) + continue; + + method = mono_get_method_checked (klass_image, MONO_TOKEN_METHOD_DEF | (first_idx + i + 1), klass, NULL, error); + if (!is_ok (error)) //bail out if we hit a loader error + return NULL; + + // Check method signature + if (method) { + other_sig = mono_method_signature_checked (method, error); + if (!is_ok (error)) //bail out if we hit a loader error + return NULL; + if (other_sig) { + gboolean found = ignore_cmods ? mono_metadata_signature_equal_ignore_custom_modifier (sig, other_sig) : mono_metadata_signature_equal (sig, other_sig); + if (found) { + if (method_maybe != NULL) { + if (ignore_cmods) { + MonoMethod *precise_match = find_method_simple (klass, name, qname, fqname, sig, from_class, FALSE, error); + if (precise_match) + return precise_match; + } + mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); + return NULL; + } + method_maybe = method; + } + } + } + } + return method_maybe; + } + + return NULL; +} + +typedef struct MethodLookupResultInfo { + int i; + MonoMethod *m; + gboolean matched; +} MethodLookupResultInfo; + +static MethodLookupResultInfo * +find_method_slow (MonoClass *klass, const char *name, const char *qname, const char *fqname, + MonoMethodSignature *sig, gboolean ignore_cmods, MonoError *error) +{ + gpointer iter = NULL; + MethodLookupResultInfo *result = (MethodLookupResultInfo *)g_malloc0 (sizeof (MethodLookupResultInfo)); + int i = -1; + MonoMethod *m = NULL; + gboolean matched = FALSE; + result->i = i; + result->m = m; + result->matched = matched; + + /* FIXME: metadata-update iterating using + * mono_class_get_methods will break if `m` is NULL. Need to + * reconcile with the `if (!m)` "we must cope" comment below. + */ + while ((m = mono_class_get_methods (klass, &iter))) { + i++; + MonoMethodSignature *msig; + + /* We must cope with failing to load some of the types. */ + if (!m) + continue; + + if (!((fqname && !strcmp (m->name, fqname)) || + (qname && !strcmp (m->name, qname)) || + (name && !strcmp (m->name, name)))) + continue; + msig = mono_method_signature_checked (m, error); + if (!is_ok (error)) //bail out if we hit a loader error + return NULL; + + if (!msig) + continue; + + gboolean found = FALSE; + if (ignore_cmods) + found = sig->call_convention == MONO_CALL_VARARG ? mono_metadata_signature_equal_vararg_ignore_custom_modifier (sig, msig) : mono_metadata_signature_equal_ignore_custom_modifier (sig, msig); + else + found = sig->call_convention == MONO_CALL_VARARG ? mono_metadata_signature_equal_vararg (sig, msig) : mono_metadata_signature_equal (sig, msig); + + if (found) { + if (matched) { + if (ignore_cmods) { + MethodLookupResultInfo *precise_match = find_method_slow (klass, name, qname, fqname, sig, FALSE, error); + if (precise_match->m) + return precise_match; + } + mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute."); + return NULL; + } + matched = TRUE; + result->i = i; + result->m = m; + result->matched = matched; + } + } + + return result; +} + +static MonoMethod * +find_method_in_class_unsafe_accessor (MonoClass *klass, const char *name, const char *qname, const char *fqname, + MonoMethodSignature *sig, MonoClass *from_class, gboolean ignore_cmods, MonoError *error) +{ + MonoMethod *method = NULL; + if (sig->call_convention != MONO_CALL_VARARG) + method = find_method_simple (klass, name, qname, fqname, sig, from_class, ignore_cmods, error); + if (method) + return method; + if (!is_ok(error) && mono_error_get_error_code (error) == MONO_ERROR_GENERIC) + return NULL; + + mono_class_setup_methods (klass); /* FIXME don't swallow the error here. */ + /* + We can't fail lookup of methods otherwise the runtime will fail with MissingMethodException instead of TypeLoadException. + See mono/tests/generic-type-load-exception.2.il + FIXME we should better report this error to the caller + */ + if (!m_class_get_methods (klass) || mono_class_has_failure (klass)) { + ERROR_DECL (cause_error); + mono_error_set_for_class_failure (cause_error, klass); + mono_error_set_type_load_class (error, klass, "Could not find method '%s' due to a type load error: %s", name, mono_error_get_message (cause_error)); + mono_error_cleanup (cause_error); + return NULL; + } + + MethodLookupResultInfo *result = find_method_slow (klass, name, qname, fqname, sig, ignore_cmods, error); + if (!is_ok(error) && mono_error_get_error_code (error) == MONO_ERROR_GENERIC) + return NULL; + + int mcount = mono_class_get_method_count (klass); + + g_assert (result != NULL); + if (result->matched) { + if (result->i < mcount) + return mono_class_get_method_by_index (from_class, result->i); + else if (result->m != NULL) { + // FIXME: metadata-update: hack + // it's from a metadata-update, probably + MonoMethod * m = mono_class_inflate_generic_method_full_checked ( + result->m, from_class, mono_class_get_context (from_class), error); + mono_error_assert_ok (error); + g_assert (m != NULL); + g_assert (m->klass == from_class); + g_assert (m->is_inflated); + return m; + } + } + + g_free (result); + return NULL; +} + +MonoMethod* +mono_unsafe_accessor_find_ctor (MonoClass *in_class, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) +{ + return find_method_in_class_unsafe_accessor (in_class, ".ctor", /*qname*/NULL, /*fqname*/NULL, sig, from_class, TRUE, error); +} + +MonoMethod* +mono_unsafe_accessor_find_method (MonoClass *in_class, const char *name, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error) +{ + // This doesn't work for constructors because find_method explicitly disallows ".ctor" and ".cctor" + return find_method_in_class_unsafe_accessor (in_class, name, /*qname*/NULL, /*fqname*/NULL, sig, from_class, TRUE, error); +}