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.

Sign up for GitHub

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

Jump to bottom

[mono] generic wrapper methods for unsafe accessors #101732

Merged
merged 31 commits into from
May 13, 2024
Merged

[mono] generic wrapper methods for unsafe accessors #101732

Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
81285fb
WIP HACK: always AOT wrappers, even for generics, not the actual acce…
lambdageek Apr 25, 2024
3d59833
cleanup
lambdageek Apr 29, 2024
fbd3b89
checkpoint generic wrapper methods
lambdageek Apr 30, 2024
34381f7
fix msvc build
lambdageek Apr 30, 2024
ecaff9e
hack
lambdageek Apr 30, 2024
06be3b4
fix build
lambdageek May 1, 2024
430f822
clean WIP printfs
lambdageek May 1, 2024
5789ab7
use generic method owner caches
lambdageek May 1, 2024
374c1e7
lookup unsafe accessor wrapper instances in aot-runtime
lambdageek May 1, 2024
db8cbad
cleanup marshaling - dont' use ctx as a flag
lambdageek May 1, 2024
f252a47
handle some generic field accessors
lambdageek May 2, 2024
e424b64
issues.targets: enable some unsafe accessor AOT tests
lambdageek May 2, 2024
03c72f7
WIP: looks like methods are working too?
lambdageek May 2, 2024
6fcefc6
[metadata] add ability to inflate wrapper data
lambdageek May 6, 2024
ccbd0ba
[marshal] refactor unsafe accessor; opt into inflate_wrapper_data
lambdageek May 6, 2024
0c824fc
Merge remote-tracking branch 'origin/main' into hack-generic-wrappers
lambdageek May 6, 2024
56516b2
comment out printfs
lambdageek May 6, 2024
769ae55
inflate MonoMethod wrapper data; impl ctor generic unsafe accessors
lambdageek May 6, 2024
9688f2b
fix windows build
lambdageek May 6, 2024
70aaa2e
[aot] handle case of partial generic sharing for unsafe accessor
lambdageek May 7, 2024
e814387
[aot] for unsafe accessor wrappers with no name, record a length 0
lambdageek May 7, 2024
03208db
[aot-runtime] try to fix gsharedvt lookup
lambdageek May 8, 2024
c244dd2
better comments; remove fied FIXMEs
lambdageek May 9, 2024
2ff358b
update HelloWorld sample to support either normal AOT or FullAOT
lambdageek May 9, 2024
25f9e99
revert HelloWorld sample code changes
lambdageek May 9, 2024
0374f2b
rename helper methods
lambdageek May 9, 2024
9786200
apply suggestions from code review
lambdageek May 10, 2024
ceb5345
DRY. compute inflate_generic_data in one place
lambdageek May 13, 2024
342fe1d
Just do one loop for inflating generic wrapper data
lambdageek May 13, 2024
a667f53
better comments
lambdageek May 13, 2024
0bf0f61
DRY. move common AOT code to mini.c
lambdageek May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/mono/mono/metadata/class-internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ struct _MonoMethodWrapper {
MonoMethodHeader *header;
MonoMemoryManager *mem_manager;
void *method_data;
unsigned int inflate_wrapper_data : 1; /* method_data[MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX] is an MonoMethodBuilderInflateWrapperData array */
};

struct _MonoDynamicMethod {
Expand Down
12 changes: 12 additions & 0 deletions src/mono/mono/metadata/class.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <mono/metadata/gc-internals.h>
#include <mono/metadata/mono-debug.h>
#include <mono/metadata/metadata-update.h>
#include <mono/metadata/method-builder-ilgen.h>
#include <mono/utils/mono-string.h>
#include <mono/utils/mono-error-internals.h>
#include <mono/utils/mono-logger-internals.h>
Expand Down Expand Up @@ -1263,6 +1264,17 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k

resw->method_data = (void **)g_malloc (sizeof (gpointer) * (len + 1));
memcpy (resw->method_data, mw->method_data, sizeof (gpointer) * (len + 1));
if (mw->inflate_wrapper_data) {
mono_mb_inflate_generic_wrapper_data (context, (gpointer*)resw->method_data, error);
if (!is_ok (error)) {
g_free (resw->method_data);
goto fail;
}
// we can't set inflate_wrapper_data to 0 on the result, it's possible it
// will need to be inflated again (for example in the method_inst ==
// generic_container->context.method_inst case, below)
resw->inflate_wrapper_data = 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Consistency between 1 and TRUE.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MonoMethodWrapper:inflate_wrapper_data is a bitfield, so I use 1

MonoMethodBuilder:inflate_wrapper_data is a gboolean, so I use TRUE

}
}

if (iresult->context.method_inst) {
Expand Down
110 changes: 49 additions & 61 deletions src/mono/mono/metadata/marshal-lightweight.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2136,6 +2136,16 @@ mb_skip_visibility_ilgen (MonoMethodBuilder *mb)
mb->skip_visibility = 1;
}

static void
mb_inflate_wrapper_data_ilgen (MonoMethodBuilder *mb)
{
g_assert (!mb->dynamic); // dynamic methods with inflated data not implemented yet - needs at least mono_free_method changes, probably more
mb->inflate_wrapper_data = TRUE;
int idx = mono_mb_add_data (mb, NULL);
// note: match index used in create_method_ilgen
g_assertf (idx == MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX, "mb_inflate_wrapper_data called after data already added");
}

static void
emit_synchronized_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method)
{
Expand Down Expand Up @@ -2265,20 +2275,26 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo
mono_mb_emit_byte (mb, CEE_RET);
}

static gboolean
unsafe_accessor_target_type_forbidden (MonoType *target_type);

static void
emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, 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) {

MonoType *target_type = sig->param_count == 1 ? sig->params[0] : NULL; // params[0] is the field's parent

if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID || unsafe_accessor_target_type_forbidden (target_type)) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
return;
}

MonoType *ret_type = sig->ret;

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);
Expand Down Expand Up @@ -2307,6 +2323,8 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_
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);
if (inflate_generic_data)
mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_FIELD);
mono_mb_emit_byte (mb, CEE_RET);
}

Expand All @@ -2315,7 +2333,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_
* 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)
method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMethodSignature *accessor_sig)
{
MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig);
g_assert (ret->param_count > 0);
Expand All @@ -2332,7 +2350,7 @@ method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMetho
* 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)
ctor_sig_from_accessor_sig (MonoMethodBuilder *mb, MonoMethodSignature *accessor_sig)
{
MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig);
ret->hasthis = TRUE; /* ctors are considered instance methods */
Expand Down Expand Up @@ -2373,28 +2391,6 @@ 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)
{
Expand All @@ -2412,7 +2408,7 @@ inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_metho
}

static void
emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name)
{
g_assert (kind == MONO_UNSAFE_ACCESSOR_CTOR);
// null or empty string member name is ok for a constructor
Expand All @@ -2424,22 +2420,19 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m
}

MonoType *target_type = sig->ret; // for constructors the return type is the target type
MonoClass *target_class = mono_class_from_mono_type_internal (target_type);

ERROR_DECL(find_method_error);
if (accessor_method->is_inflated) {
sig = update_signature(accessor_method);
target_type = sig->ret;
}

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

MonoClass *target_class = mono_class_from_mono_type_internal (target_type);

ERROR_DECL(find_method_error);

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

MonoClass *in_class = mono_class_get_generic_type_definition (target_class);
MonoClass *in_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) {
Expand All @@ -2458,21 +2451,27 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m
emit_unsafe_accessor_ldargs (mb, sig, 0);

mono_mb_emit_op (mb, CEE_NEWOBJ, target_method);
if (inflate_generic_data)
mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_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)
emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, 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");


MonoType *target_type = sig->param_count >= 1 ? sig->params[0] : NULL;

if (sig->param_count < 1 || target_type == NULL || unsafe_accessor_target_type_forbidden (target_type)) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
return;
}

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

Expand All @@ -2482,19 +2481,10 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor
}

ERROR_DECL(find_method_error);
if (accessor_method->is_inflated) {
sig = update_signature(accessor_method);
target_type = sig->params[0];
}

if (sig->param_count < 1 || target_type == NULL || unsafe_accessor_target_type_forbidden (target_type)) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
return;
}

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

MonoClass *in_class = mono_class_get_generic_type_definition (target_class);
MonoClass *in_class = target_class;

MonoMethod *target_method = NULL;
if (!ctor_as_method)
Expand Down Expand Up @@ -2522,17 +2512,14 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor
emit_unsafe_accessor_ldargs (mb, sig, !hasthis ? 1 : 0);

mono_mb_emit_op (mb, hasthis ? CEE_CALLVIRT : CEE_CALL, target_method);
if (inflate_generic_data)
mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_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)
emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name)
{
if (accessor_method->is_generic || ctx != NULL) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics");
return;
}

if (!m_method_is_static (accessor_method)) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic");
return;
Expand All @@ -2541,14 +2528,14 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_
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);
emit_unsafe_accessor_field_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name);
return;
case MONO_UNSAFE_ACCESSOR_CTOR:
emit_unsafe_accessor_ctor_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
emit_unsafe_accessor_ctor_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name);
return;
case MONO_UNSAFE_ACCESSOR_METHOD:
case MONO_UNSAFE_ACCESSOR_STATIC_METHOD:
emit_unsafe_accessor_method_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
emit_unsafe_accessor_method_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name);
return;
default:
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_InvalidKindValue");
Expand Down Expand Up @@ -3404,6 +3391,7 @@ mono_marshal_lightweight_init (void)
cb.emit_return = emit_return_ilgen;
cb.emit_vtfixup_ftnptr = emit_vtfixup_ftnptr_ilgen;
cb.mb_skip_visibility = mb_skip_visibility_ilgen;
cb.mb_inflate_wrapper_data = mb_inflate_wrapper_data_ilgen;
cb.mb_emit_exception = mb_emit_exception_ilgen;
cb.mb_emit_exception_for_error = mb_emit_exception_for_error_ilgen;
cb.mb_emit_byte = mb_emit_byte_ilgen;
Expand Down
Loading