Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mono] UnsafeAccessorAttribute non-generic support for field #88626

Merged
merged 35 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
bc9d6ed
Detect an UnsafeAccessorAttribute for method with interpreter
fanyang-mono Jul 5, 2023
1643710
Change field to property
fanyang-mono Jul 6, 2023
b0def6c
Get Kind from typed_args
fanyang-mono Jul 6, 2023
0c51e8a
define MonoUnsafeAccessorKind enum
lambdageek Jul 6, 2023
94ce7d2
Add the frontend for JIT
fanyang-mono Jul 6, 2023
a49a8e0
Add mono_marshal_get_unsafe_accessor_wrapper and WRAPPER_SUBTYPE_UNSA…
lambdageek Jul 7, 2023
4701689
[interp] get the unsafe accessor wrapper
lambdageek Jul 7, 2023
495ce38
fix: skip visibility in unsafe accessor wrappers
lambdageek Jul 7, 2023
35c4d45
fix: decode the length and copy the name from UnsafeAccessorAttribute
lambdageek Jul 7, 2023
0c8f4da
[mini] compile wrapper
lambdageek Jul 10, 2023
8d039e6
[aot] Emit unsafe accessor wrappers to the AOT image
lambdageek Jul 10, 2023
080559a
Add the method to emit wrapper for field
fanyang-mono Jul 10, 2023
2ec266b
Fix typo
fanyang-mono Jul 10, 2023
8571969
Remove assertion for interpreter
fanyang-mono Jul 10, 2023
b7e35c6
Fix format and replace assertion with proper exception
fanyang-mono Jul 10, 2023
449aa6f
Free the memory and throw proper NotImplementedException
fanyang-mono Jul 10, 2023
da84576
Enable StaticField and Field tests
fanyang-mono Jul 10, 2023
23a4d9a
Remove unused variables
fanyang-mono Jul 11, 2023
342c8d4
Only allow static method
fanyang-mono Jul 11, 2023
375d460
Return immediately once hit an exception
fanyang-mono Jul 11, 2023
9222fd2
Free cinfo at the right location
fanyang-mono Jul 11, 2023
ab2e8c5
Disable test, cause Constructor kind is not supported yet.
fanyang-mono Jul 11, 2023
2f8031a
Stop inlining UnsafeAccessor wrapper for interpreter
fanyang-mono Jul 11, 2023
10e43de
Merge branch 'main' into unsafeAccessor
fanyang-mono Jul 12, 2023
4d874c3
Revert inlining change
fanyang-mono Jul 12, 2023
ade7c78
Update exception message
fanyang-mono Jul 12, 2023
58822ca
Fix loading static field and add static status check and a test
fanyang-mono Jul 13, 2023
3686590
Add another test and address feedback
fanyang-mono Jul 13, 2023
8afb00a
Update src/tests/baseservices/compilerservices/UnsafeAccessors/Unsafe…
fanyang-mono Jul 13, 2023
a3f9e77
Update src/tests/baseservices/compilerservices/UnsafeAccessors/Unsafe…
fanyang-mono Jul 13, 2023
9f6b532
Update src/tests/baseservices/compilerservices/UnsafeAccessors/Unsafe…
fanyang-mono Jul 13, 2023
0c15ccd
Update src/tests/baseservices/compilerservices/UnsafeAccessors/Unsafe…
fanyang-mono Jul 13, 2023
f21b78e
Update src/tests/baseservices/compilerservices/UnsafeAccessors/Unsafe…
fanyang-mono Jul 13, 2023
781f512
Address more review feedback
fanyang-mono Jul 13, 2023
17febef
Fix function call to field
fanyang-mono Jul 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
fanyang-mono marked this conversation as resolved.
Show resolved Hide resolved
}

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);
fanyang-mono marked this conversation as resolved.
Show resolved Hide resolved
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) {
kotlarmilos marked this conversation as resolved.
Show resolved Hide resolved
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);
fanyang-mono marked this conversation as resolved.
Show resolved Hide resolved
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
70 changes: 70 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,74 @@ 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;
}

mono_mb_emit_ldarg (mb, 0);

MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL);
if (target_field == NULL || target_field->type->type != ret_type->type) {
fanyang-mono marked this conversation as resolved.
Show resolved Hide resolved
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;
}

mono_mb_emit_op (mb, CEE_LDFLDA, target_field);
mono_mb_emit_byte (mb, CEE_RET);
}

static void
emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
{
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 @@ -3154,6 +3223,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);
}