Skip to content

Commit

Permalink
cleanup marshaling - dont' use ctx as a flag
Browse files Browse the repository at this point in the history
  • Loading branch information
@lambdageek
lambdageek committed May 1, 2024
1 parent 374c1e7 commit db8cbad
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 23 deletions.
29 changes: 17 additions & 12 deletions src/mono/mono/metadata/marshal-lightweight.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2266,7 +2266,7 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo
}

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, 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);
Expand Down Expand Up @@ -2315,7 +2315,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 +2332,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 @@ -2410,7 +2410,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, 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 @@ -2434,7 +2434,7 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m
sig = update_signature(accessor_method);
}

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

Expand All @@ -2459,7 +2459,7 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m
}

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, 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);
Expand All @@ -2486,7 +2486,7 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor
sig = update_signature(accessor_method);
}

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

Expand Down Expand Up @@ -2520,9 +2520,14 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor
}

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, MonoMethod *accessor_method, MonoMethodSignature *sig, gboolean to_be_inflated, MonoUnsafeAccessorKind kind, const char *member_name)
{
if (accessor_method->is_generic || ctx != NULL) {
// to_be_inflated means we're emitting a non-generic accessor method belonging to a gtd
// that will be inflated by marshal.c
if (to_be_inflated)
g_assert (!accessor_method->is_inflated && !accessor_method->is_generic);

if (accessor_method->is_generic || to_be_inflated) {
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics");
return;
}
Expand All @@ -2535,14 +2540,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, 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, 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, accessor_method, sig, kind, member_name);
return;
default:
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_InvalidKindValue");
Expand Down
39 changes: 29 additions & 10 deletions src/mono/mono/metadata/marshal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5230,20 +5230,25 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf
MonoMethod *orig_method = NULL;
WrapperInfo *info;
gboolean is_generic = FALSE;
gboolean is_inflated = FALSE;

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

printf("CAME IN: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0);

if (accessor_method->is_generic) {
// FullAOT compilation with gshared or gsharedvt will get here.
/* got a generic method definition. need to make a generic method definition wrapper */
g_assert (!accessor_method->is_inflated);
is_generic = TRUE;
}
} else if (accessor_method->is_inflated) {
// FIXME: this always tries to compile a generic version of the accessor method and
// then inflate it. But maybe we dont' want to do that (particularly for field
// accessors). In particular if there is no method_inst, we're looking at an
// accessor method inside a generic class (alwayst a ginst? or sometimes a gtd?)
// In that case we might just want to compile the instance.

/* FIXME: Support generic methods too */
if (accessor_method->is_inflated) {
// non-full AOT may get here
g_assert (!is_generic);
orig_method = accessor_method;
ctx = &((MonoMethodInflated*)accessor_method)->context;
Expand All @@ -5252,13 +5257,21 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf
if (!container)
container = mono_class_try_get_generic_container (accessor_method->klass); //FIXME is this a case of a try?
g_assert (container);
is_inflated = TRUE;
}

printf("work on: %s (generic = %d, inflated = %d)\n", mono_method_full_name(accessor_method, TRUE), accessor_method->is_generic?1:0, accessor_method->is_inflated?1:0);

/*
* the method is either a generic method definition, or it might be inflated, not both at
* the same time.
*/
g_assert (!(is_generic && is_inflated));

/*
* Check cache
*/
if (ctx) {
/* FIXME get the right cache */
if (is_inflated) {
cache = get_cache (&((MonoMethodInflated*)orig_method)->owner->wrapper_caches.unsafe_accessor_cache , mono_aligned_addr_hash, NULL);
res = check_generic_wrapper_cache (cache, orig_method, orig_method, accessor_method);
if (res)
Expand All @@ -5268,10 +5281,13 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf
if ((res = mono_marshal_find_in_cache (cache, accessor_method)))
return res;
}


printf ("Cache miss\n");
mb = mono_mb_new (accessor_method->klass, accessor_method->name, MONO_WRAPPER_OTHER);
if (is_generic) {
// If the accessor method was generic, make the wrapper generic, too.

// Load a copy of the generic params of the accessor method
mb->method->is_generic = is_generic;
container = mono_class_try_get_generic_container (accessor_method->klass);
container = mono_metadata_load_generic_params (m_class_get_image (accessor_method->klass), accessor_method->token, container, /*owner:*/mb->method);
Expand All @@ -5281,6 +5297,8 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf
inst_ctx.method_inst = container->context.method_inst;

ERROR_DECL (error);
// make a copy of the accessor signature, but replace the params of the accessor
// method, by the params we just loaded
sig = mono_inflate_generic_signature (mono_method_signature_internal (accessor_method), &inst_ctx, error);
mono_error_assert_ok (error); // FIXME
} else {
Expand All @@ -5291,14 +5309,15 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf
get_marshal_cb ()->mb_skip_visibility (mb);

// TODO: pass container, too
get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
// TODO: passing the ctx is not super useful because accessor_method is always the generic version of the method, even if is_inflated == TRUE. pass `is_inflated` instead
get_marshal_cb ()->emit_unsafe_accessor_wrapper (mb, accessor_method, sig, is_inflated, 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) {
if (is_inflated) {
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);
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/marshal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ typedef struct {
void (*emit_synchronized_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method);
void (*emit_unbox_wrapper) (MonoMethodBuilder *mb, MonoMethod *method);
void (*emit_array_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *sig, MonoGenericContext *ctx);
void (*emit_unsafe_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name);
void (*emit_unsafe_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, gboolean to_be_inflated, MonoUnsafeAccessorKind kind, const char *member_name);
void (*emit_generic_array_helper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig);
void (*emit_thunk_invoke_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *csig);
void (*emit_create_string_hack) (MonoMethodBuilder *mb, MonoMethodSignature *csig, MonoMethod *res);
Expand Down

0 comments on commit db8cbad

Please sign in to comment.