Skip to content

Commit

Permalink
[mono] UnsafeAccessor: Add support for non-generic method instance (d…
Browse files Browse the repository at this point in the history
…otnet#101442)

* Support non generic instance

* Create helper function for shared code

* Use an existing API

* Add an assert

---------

Co-authored-by: Larry Ewing <lewing@microsoft.com>
  • Loading branch information
2 people authored and Ruihan-Yin committed May 30, 2024
1 parent 7dcc521 commit d422d0a
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/class.c
Original file line number Diff line number Diff line change
Expand Up @@ -5264,7 +5264,7 @@ mono_class_get_fields_internal (MonoClass *klass, gpointer *iter)
* mono_class_get_methods:
* \param klass the \c MonoClass to act on
*
* This routine is an iterator routine for retrieving the fields in a class.
* This routine is an iterator routine for retrieving the methods in a class.
*
* You must pass a \c gpointer that points to zero and is treated as an opaque handle to
* iterate over all of the elements. When no more values are
Expand Down
73 changes: 64 additions & 9 deletions src/mono/mono/metadata/marshal-lightweight.c
Original file line number Diff line number Diff line change
Expand Up @@ -2371,6 +2371,44 @@ emit_missing_method_error (MonoMethodBuilder *mb, MonoError *failure, const char
}
}

static MonoMethodSignature *
update_signature (MonoMethod *accessor_method)
{
MonoClass *accessor_method_class_instance = accessor_method->klass;
MonoClass *accessor_method_class = mono_class_get_generic_type_definition (accessor_method_class_instance);

const char *accessor_method_name = accessor_method->name;

gpointer iter = NULL;
MonoMethod *m = NULL;
while ((m = mono_class_get_methods (accessor_method_class, &iter))) {
if (!m)
continue;

if (strcmp (m->name, accessor_method_name))
continue;

return mono_metadata_signature_dup_full (get_method_image (m), mono_method_signature_internal (m));
}
g_assert_not_reached ();
}

static MonoMethod *
inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_method, MonoError *error)
{
MonoMethod *result = method;
MonoGenericContext context = { NULL, NULL };
if (mono_class_is_ginst (klass))
context.class_inst = mono_class_get_generic_class (klass)->context.class_inst;
if (accessor_method->is_inflated)
context.method_inst = mono_method_get_context (accessor_method)->method_inst;
if ((context.class_inst != NULL) || (context.method_inst != NULL))
result = mono_class_inflate_generic_method_checked (method, &context, error);
mono_error_assert_ok (error);

return result;
}

static void
emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
{
Expand All @@ -2389,12 +2427,17 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m
return;
}

MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig, ctx);

MonoClass *target_class = mono_class_from_mono_type_internal (target_type);

ERROR_DECL(find_method_error);
MonoClass *in_class = mono_class_is_ginst (target_class) ? mono_class_get_generic_class (target_class)->container_class : target_class;
if (accessor_method->is_inflated) {
sig = update_signature(accessor_method);
}

MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig, ctx);

MonoClass *in_class = mono_class_get_generic_type_definition (target_class);

MonoMethod *target_method = mono_unsafe_accessor_find_ctor (in_class, member_sig, target_class, find_method_error);
if (!is_ok (find_method_error) || target_method == NULL) {
if (mono_error_get_error_code (find_method_error) == MONO_ERROR_GENERIC)
Expand All @@ -2404,6 +2447,9 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m
mono_error_cleanup (find_method_error);
return;
}

target_method = inflate_method (target_class, target_method, accessor_method, find_method_error);

g_assert (target_method->klass == target_class);

emit_unsafe_accessor_ldargs (mb, sig, 0);
Expand All @@ -2425,11 +2471,9 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
return;
}
gboolean hasthis = kind == MONO_UNSAFE_ACCESSOR_METHOD;
MonoType *target_type = sig->params[0];

MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig, ctx);

MonoType *target_type = sig->params[0];
gboolean hasthis = kind == MONO_UNSAFE_ACCESSOR_METHOD;
MonoClass *target_class = mono_class_from_mono_type_internal (target_type);

if (hasthis && m_class_is_valuetype (target_class) && !m_type_is_byref (target_type)) {
Expand All @@ -2438,7 +2482,14 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor
}

ERROR_DECL(find_method_error);
MonoClass *in_class = mono_class_is_ginst (target_class) ? mono_class_get_generic_class (target_class)->container_class : target_class;
if (accessor_method->is_inflated) {
sig = update_signature(accessor_method);
}

MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig, ctx);

MonoClass *in_class = mono_class_get_generic_type_definition (target_class);

MonoMethod *target_method = NULL;
if (!ctor_as_method)
target_method = mono_unsafe_accessor_find_method (in_class, member_name, member_sig, target_class, find_method_error);
Expand All @@ -2452,11 +2503,15 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor
mono_error_cleanup (find_method_error);
return;
}

target_method = inflate_method (target_class, target_method, accessor_method, find_method_error);

if (!hasthis && target_method->klass != target_class) {
emit_missing_method_error (mb, find_method_error, member_name);
return;
}
g_assert (target_method->klass == target_class); // are instance methods allowed to be looked up using inheritance?

g_assert (target_method->klass == target_class);

emit_unsafe_accessor_ldargs (mb, sig, !hasthis ? 1 : 0);

Expand Down
22 changes: 6 additions & 16 deletions src/mono/mono/metadata/unsafe-accessor.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "mono/metadata/class-internals.h"
#include "mono/utils/mono-error-internals.h"
#include "mono/metadata/unsafe-accessor.h"

#include <mono/metadata/debug-helpers.h>


static MonoMethod *
Expand Down Expand Up @@ -134,7 +134,10 @@ find_method_slow (MonoClass *klass, const char *name, const char *qname, const c
return precise_match;
}
mono_error_set_generic_error (error, "System.Reflection", "AmbiguousMatchException", "Ambiguity in binding of UnsafeAccessorAttribute.");
return NULL;
result->i = -1;
result->m = NULL;
result->matched = FALSE;
return result;
}
matched = TRUE;
result->i = i;
Expand Down Expand Up @@ -176,23 +179,10 @@ find_method_in_class_unsafe_accessor (MonoClass *klass, const char *name, const
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;
}
return result->m;
}

g_free (result);
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/mini/method-to-ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -7864,7 +7864,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
UNVERIFIED;

if (!cfg->gshared)
g_assert (!mono_method_check_context_used (cmethod));
g_assertf (!mono_method_check_context_used (cmethod), "cmethod is %s", mono_method_get_full_name (cmethod));

CHECK_STACK (n);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ sealed class Derived2 : GenericBase<string>
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/89439", TestRuntimes.Mono)]
public static void Verify_Generic_InheritanceMethodResolution()
{
string expect = "abc";
Expand Down Expand Up @@ -230,7 +229,6 @@ sealed class Accessors<V>
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/89439", TestRuntimes.Mono)]
public static void Verify_Generic_CallCtor()
{
Console.WriteLine($"Running {nameof(Verify_Generic_CallCtor)}");
Expand Down Expand Up @@ -312,7 +310,6 @@ public static void Verify_Generic_GenericTypeNonGenericInstanceMethod()
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/89439", TestRuntimes.Mono)]
public static void Verify_Generic_GenericTypeGenericInstanceMethod()
{
Console.WriteLine($"Running {nameof(Verify_Generic_GenericTypeGenericInstanceMethod)}");
Expand Down Expand Up @@ -366,7 +363,6 @@ public static void Verify_Generic_GenericTypeNonGenericStaticMethod()
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/89439", TestRuntimes.Mono)]
public static void Verify_Generic_GenericTypeGenericStaticMethod()
{
Console.WriteLine($"Running {nameof(Verify_Generic_GenericTypeGenericStaticMethod)}");
Expand Down

0 comments on commit d422d0a

Please sign in to comment.