Skip to content

Commit

Permalink
[Mono] UnsafeAccessorAttribute non-generic support for field (#88626)
Browse files Browse the repository at this point in the history
* Detect an UnsafeAccessorAttribute for method with interpreter

* Change field to property

* Get Kind from typed_args

* define MonoUnsafeAccessorKind enum

* Add the frontend for JIT

* Add mono_marshal_get_unsafe_accessor_wrapper and WRAPPER_SUBTYPE_UNSAFE_ACCESSOR

And the associated AOT compiler/runtime and marshaling caching boilerplate.

* [interp] get the unsafe accessor wrapper

* fix: skip visibility in unsafe accessor wrappers

that is the whole point of them

* 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

* [mini] compile wrapper

* [aot] Emit unsafe accessor wrappers to the AOT image

* Add the method to emit wrapper for field

* Fix typo

* Remove assertion for interpreter

* Fix format and replace assertion with proper exception

* Free the memory and throw proper NotImplementedException

* Enable StaticField and Field tests

Co-authored-by: Aleksey Kliger <[email protected]>
Co-authored-by: Aaron Robinson <[email protected]>
  • Loading branch information
3 people authored Jul 14, 2023
1 parent 940c26d commit 45db21d
Show file tree
Hide file tree
Showing 16 changed files with 395 additions and 6 deletions.
2 changes: 2 additions & 0 deletions src/mono/mono/metadata/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})

Expand Down
3 changes: 3 additions & 0 deletions src/mono/mono/metadata/class-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
57 changes: 57 additions & 0 deletions src/mono/mono/metadata/custom-attrs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -2056,6 +2057,62 @@ 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){
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;
}

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, "Name")) {
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;
}
}

mono_reflection_free_custom_attr_data_args_noalloc (decoded_args);
if (!cinfo->cached)
mono_custom_attrs_free(cinfo);

return TRUE;
}

/**
* mono_custom_attrs_from_class:
*/
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/metadata/loader-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
42 changes: 40 additions & 2 deletions src/mono/mono/metadata/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -2100,6 +2100,44 @@ 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;

img = m_class_get_image (method->klass);

if (mono_method_has_no_body (method)) {
return FALSE;
}

if (method->is_inflated) {
MonoMethodInflated *imethod = (MonoMethodInflated *) method;
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.
Expand Down
75 changes: 75 additions & 0 deletions src/mono/mono/metadata/marshal-lightweight.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -2319,6 +2320,79 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo
mono_mb_emit_byte (mb, CEE_RET);
}

static void
emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
{
// Field access requires a single argument for target type and a return type.
g_assert (kind == MONO_UNSAFE_ACCESSOR_FIELD || kind == MONO_UNSAFE_ACCESSOR_STATIC_FIELD);
g_assert (member_name != NULL);

MonoType *target_type = sig->params[0]; // params[0] is the field's parent
MonoType *ret_type = sig->ret;
if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
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)) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
return;
}

MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL);
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("UnsafeAccessorKind does not match expected static modifier on field '%s' in '%s'", member_name, m_class_get_name (target_class)));
return;
}

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

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:
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");
return;
case MONO_UNSAFE_ACCESSOR_METHOD:
case MONO_UNSAFE_ACCESSOR_STATIC_METHOD:
// TODO
mono_mb_emit_exception_full (mb, "System", "NotImplementedException", "UnsafeAccessor");
return;
default:
g_assert_not_reached(); // some unknown wrapper kind
}
}

static void
emit_generic_array_helper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig)
{
Expand Down Expand Up @@ -3158,6 +3232,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;
Expand Down
91 changes: 91 additions & 0 deletions src/mono/mono/metadata/marshal.c
Original file line number Diff line number Diff line change
Expand Up @@ -5094,6 +5094,96 @@ 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;

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 ()->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);
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*
Expand Down Expand Up @@ -6398,4 +6488,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);
}
Loading

0 comments on commit 45db21d

Please sign in to comment.