Skip to content

Commit

Permalink
[mono] Some UnsafeAccessorAttribute support for methods and construct…
Browse files Browse the repository at this point in the history 
…ors (dotnet#88925)

* first cut at UnsafeAccessorKind.Constructor

* Implement UnsafeAccessorKind.Method and StaticMethod

* remove debug output

* change some asserts to BadImageFormatException

* enable some tests

* move CallAmbiguousMethod to a separate test case

* Fixup error checking; forbid generics

* match CoreCLR: callvirt for instance methods call for static

* fixups to pass new tests from dotnet#88268

* Make CallCtorAsMethod tests work


Co-authored-by: Aaron Robinson <arobins@microsoft.com>
  • Loading branch information
@lambdageek @AaronRobinsonMSFT
lambdageek and AaronRobinsonMSFT committed Jul 17, 2023
1 parent 758adb5 commit 6166de4
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 29 deletions.
16 changes: 16 additions & 0 deletions src/mono/mono/metadata/loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,22 @@ 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)
{
// 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);
}

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);
}


static MonoMethod *
method_from_methodspec (MonoImage *image, MonoGenericContext *context, guint32 idx, MonoError *error)
{
Expand Down
165 changes: 158 additions & 7 deletions src/mono/mono/metadata/marshal-lightweight.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2354,18 +2354,170 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_
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;
}
if (is_field_static && m_field_get_parent (target_field) != target_class) {
// don't look up static fields using the inheritance hierarchy
mono_mb_emit_exception_full (mb, "System", "MissingFieldException", g_strdup_printf("Field '%s' not found in '%s'", member_name, m_class_get_name (target_class)));
}

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);
}

/*
* Given an accessor method signature (where the first arg is a target class) creates the signature
* of the expected member method (ie, with the first arg removed)
*/
static MonoMethodSignature *
method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMethodSignature *accessor_sig, MonoGenericContext *ctx)
{
MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig);
g_assert (ret->param_count > 0);
ret->hasthis = hasthis;
for (int i = 1; i < ret->param_count; i++)
ret->params [i - 1] = ret->params [i];
memset (&ret->params[ret->param_count - 1], 0, sizeof (MonoType)); // just in case
ret->param_count--;
return ret;
}

/*
* Given an accessor method signature (where the return type is a target class) creates the signature
* of the expected constructor method (same args, but return type is void).
*/
static MonoMethodSignature *
ctor_sig_from_accessor_sig (MonoMethodBuilder *mb, MonoMethodSignature *accessor_sig, MonoGenericContext *ctx)
{
MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig);
ret->hasthis = TRUE; /* ctors are considered instance methods */
ret->ret = mono_get_void_type ();
return ret;
}

static void
emit_unsafe_accessor_ldargs (MonoMethodBuilder *mb, MonoMethodSignature *accessor_sig, int skip_count)
{
for (int i = skip_count; i < accessor_sig->param_count; i++)
mono_mb_emit_ldarg (mb, i);
}

static gboolean
unsafe_accessor_target_type_forbidden (MonoType *target_type)
{
switch (target_type->type)
{
case MONO_TYPE_VOID:
case MONO_TYPE_PTR:
case MONO_TYPE_FNPTR:
return TRUE;
default:
return FALSE;
}
}

static void
emit_missing_method_error (MonoMethodBuilder *mb, MonoError *failure, const char *display_member_name)
{
if (!is_ok (failure)) {
mono_mb_emit_exception_full (mb, "System", "MissingMethodException", g_strdup_printf ("Could not find %s due to: %s", display_member_name, mono_error_get_message (failure)));
} else {
mono_mb_emit_exception_full (mb, "System", "MissingMethodException", g_strdup_printf ("Could not find %s", display_member_name));
}
}

static void
emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
{
g_assert (kind == MONO_UNSAFE_ACCESSOR_CTOR);
// null or empty string member name is ok for a constructor
if (!member_name || member_name[0] == '\0')
member_name = ".ctor";
if (strcmp (member_name, ".ctor") != 0) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid UnsafeAccessorAttribute for constructor.");
return;
}

MonoType *target_type = sig->ret; // for constructors the return type is the target type
if (target_type == NULL || m_type_is_byref (target_type) || unsafe_accessor_target_type_forbidden (target_type)) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
return;
}

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;
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");
mono_error_cleanup (find_method_error);
return;
}
g_assert (target_method->klass == target_class);

emit_unsafe_accessor_ldargs (mb, sig, 0);

mono_mb_emit_op (mb, CEE_NEWOBJ, target_method);
mono_mb_emit_byte (mb, CEE_RET);
}

static void
emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
{
g_assert (kind == MONO_UNSAFE_ACCESSOR_METHOD || kind == MONO_UNSAFE_ACCESSOR_STATIC_METHOD);
g_assert (member_name != NULL);

// We explicitly allow calling a constructor as if it was an instance method, but we need some hacks in a couple of places
gboolean ctor_as_method = !strcmp (member_name, ".ctor");

if (sig->param_count < 1 || sig->params[0] == NULL || unsafe_accessor_target_type_forbidden (sig->params[0])) {
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);

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)) {
// If the non-static method access is for a value type, the instance must be byref.
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
}

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;
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);
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);
mono_error_cleanup (find_method_error);
return;
}
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?

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

mono_mb_emit_op (mb, hasthis ? CEE_CALLVIRT : CEE_CALL, target_method);
mono_mb_emit_byte (mb, CEE_RET);
}

static void
emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
{
if (accessor_method->is_generic) {
mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor_Generics");
if (accessor_method->is_inflated || accessor_method->is_generic || mono_class_is_ginst (accessor_method->klass) || ctx != NULL) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics");
return;
}

Expand All @@ -2380,16 +2532,15 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_
emit_unsafe_accessor_field_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
return;
case MONO_UNSAFE_ACCESSOR_CTOR:
// TODO
mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor");
emit_unsafe_accessor_ctor_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
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_method_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
return;
default:
g_assert_not_reached(); // some unknown wrapper kind
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_InvalidKindValue");
return;
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/mono/mono/metadata/marshal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5140,7 +5140,7 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf
MonoMethod *orig_method = NULL;
WrapperInfo *info;

if (member_name == NULL)
if (member_name == NULL && kind != MONO_UNSAFE_ACCESSOR_CTOR)
member_name = accessor_method->name;

/*
Expand Down Expand Up @@ -5178,8 +5178,7 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf
}
mono_mb_free (mb);

// TODO: remove before merging
mono_method_print_code (res);
/* mono_method_print_code (res); */

return res;
}
Expand Down
10 changes: 10 additions & 0 deletions src/mono/mono/metadata/unsafe-accessor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#ifndef __MONO_METADATA_UNSAFE_ACCESSOR_H__
#define __MONO_METADATA_UNSAFE_ACCESSOR_H__

#include <mono/metadata/class.h>
#include <mono/metadata/metadata.h>
#include <mono/utils/mono-error.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
*/
Expand All @@ -16,4 +20,10 @@ typedef enum {
MONO_UNSAFE_ACCESSOR_STATIC_FIELD,
} MonoUnsafeAccessorKind;

MonoMethod*
mono_unsafe_accessor_find_ctor (MonoClass *in_class, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error);

MonoMethod*
mono_unsafe_accessor_find_method (MonoClass *in_class, const char *name, MonoMethodSignature *sig, MonoClass *from_class, MonoError *error);

#endif /* __MONO_METADATA_UNSAFE_ACCESSOR_H__ */
Loading

0 comments on commit 6166de4

Please sign in to comment.