Skip to content

Commit 2385d08

Browse files
authored
[mono] generic wrapper methods for unsafe accessors (#101732)
1. Adds support for compiling generic wrapper methods to Mono. In some situations we need to emit a wrapper method that is generic. This makes the MethodBuilder infrastructure support that. 2. Adds support for inflating generic wrapper data. Wrappers have pointer data associated with them that is used by the code generator (For example instead of emitting field tokens, we record the `MonoClassField*` directly and then emit a fake token that is just the index of the `MonoClassField*` in the `MonoMethodWrapper:method_data` array). The pointer data associated with a wrapper is normally just consumed verbatim. But if the wrapper is part of a generic class, or if the wrapper is a generic method, the wrapper data might have generic parameters (for example it might be a MonoClassField for `MyList<T>` instead of `MyList<string>`). Add support for tagging the data with its kind and inflating it if the wrapper method is inflated. 3. Applies (1) and (2) to unsafe accessor methods - the unsafe accesor wrapper generation now always tries to get the generic definition and to generate a wrapper for that generic definition and then inflate it. 4. Some AOT changes so that FullAOT substitutes lookups for an unsafe accessor by lookups for the wrapper. Including if the unsafe accessor or wrapper is generic. This also enabled gshared and gsharedvt for unsafe accessor wrappers. This also fixes #92883 Contributes to #99830, #89439 **NOT DONE** - We don't check constraints on the generic target types yet --- * always AOT wrappers, even for generics, not the actual accessor * add generic wrapper methods * use generic method owner caches * lookup unsafe accessor wrapper instances in aot-runtime if someone needs the unsafe accessor, lookup the wrapper instead. Needed when there's a call for extra instances * cleanup marshaling - dont' use ctx as a flag * handle some generic field accessors As long as the target is just some type that mentions a generic field, we're ok - the regular gsharing ldflda works. It just can't be a type variable. * issues.targets: enable some unsafe accessor AOT tests * [metadata] add ability to inflate wrapper data When we create generic wrappers (or wrappers in a generic class), if the wrapper data needs to refer to a field, method, or parameter type of the definition, that data might need to be inflated if the containing class is inflated (or the generic wrapper method is inflated). Add a new function to opt into inflation: ```c get_marshal_cb ()->mb_inflate_wrapper_data (mb); ``` Add a new function to be called after mono_mb_emit_op (or any other call that calls mono_mb_add_data): ```c mono_mb_emit_op (mb, CEE_LDFLDA, field); mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_FIELD); ``` Note: mono_mb_set_wrapper_data_kind asserts if you haven't called mb_inflate_wrapper_data. TODO: add more wrapper data kinds for MonoMethod* and MonoClass* etc * [marshal] refactor unsafe accessor; opt into inflate_wrapper_data Try to separate the ideas of "the call to the UnsafeAccessor method was inflated, so we need to inflate the wrapper" and "the UnsafeAccessor method is a generic method definition, so the wrapper should be a generic method definition, too" * inflate MonoMethod wrapper data; impl ctor generic unsafe accessors * fix windows build * [aot] handle case of partial generic sharing for unsafe accessor In partial generic sharing, a call to an instance like `Foo<int>` is replaced by `Foo<T_INT>` where T is constrained to `int` and enums that use `int` as a base type. In that case the AOT compiler will emit the unsafe accessor wrapper instantiated with `T_INT`. So in the AOT lookup we have to find calls to `UnsafeAccessor<int>` and replace them with calls to `(wrapper) UnsafeAccessor<T_INT>` to match what the AOT compiler is doing * [aot] for unsafe accessor wrappers with no name, record a length 0 This is needed because for inflated unsafe accessors we write the inflated bit after the name. So if we're accessing a constructor and we didn't record a name in the AOT image, we would erroneously read the inflated bit as the name length. * [aot-runtime] try to fix gsharedvt lookup * better comments; remove fied FIXMEs * update HelloWorld sample to support either normal AOT or FullAOT * rename helper methods * apply suggestions from code review * DRY. compute inflate_generic_data in one place * Just do one loop for inflating generic wrapper data * better comments * DRY. move common AOT code to mini.c
1 parent 29cb8a6 commit 2385d08

16 files changed

+533
-104
lines changed

src/mono/mono/metadata/class-internals.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ struct _MonoMethodWrapper {
9696
MonoMethodHeader *header;
9797
MonoMemoryManager *mem_manager;
9898
void *method_data;
99+
unsigned int inflate_wrapper_data : 1; /* method_data[MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX] is an MonoMethodBuilderInflateWrapperData array */
99100
};
100101

101102
struct _MonoDynamicMethod {

src/mono/mono/metadata/class.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include <mono/metadata/gc-internals.h>
4242
#include <mono/metadata/mono-debug.h>
4343
#include <mono/metadata/metadata-update.h>
44+
#include <mono/metadata/method-builder-ilgen.h>
4445
#include <mono/utils/mono-string.h>
4546
#include <mono/utils/mono-error-internals.h>
4647
#include <mono/utils/mono-logger-internals.h>
@@ -1263,6 +1264,17 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k
12631264

12641265
resw->method_data = (void **)g_malloc (sizeof (gpointer) * (len + 1));
12651266
memcpy (resw->method_data, mw->method_data, sizeof (gpointer) * (len + 1));
1267+
if (mw->inflate_wrapper_data) {
1268+
mono_mb_inflate_generic_wrapper_data (context, (gpointer*)resw->method_data, error);
1269+
if (!is_ok (error)) {
1270+
g_free (resw->method_data);
1271+
goto fail;
1272+
}
1273+
// we can't set inflate_wrapper_data to 0 on the result, it's possible it
1274+
// will need to be inflated again (for example in the method_inst ==
1275+
// generic_container->context.method_inst case, below)
1276+
resw->inflate_wrapper_data = 1;
1277+
}
12661278
}
12671279

12681280
if (iresult->context.method_inst) {

src/mono/mono/metadata/marshal-lightweight.c

Lines changed: 49 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -2136,6 +2136,16 @@ mb_skip_visibility_ilgen (MonoMethodBuilder *mb)
21362136
mb->skip_visibility = 1;
21372137
}
21382138

2139+
static void
2140+
mb_inflate_wrapper_data_ilgen (MonoMethodBuilder *mb)
2141+
{
2142+
g_assert (!mb->dynamic); // dynamic methods with inflated data not implemented yet - needs at least mono_free_method changes, probably more
2143+
mb->inflate_wrapper_data = TRUE;
2144+
int idx = mono_mb_add_data (mb, NULL);
2145+
// note: match index used in create_method_ilgen
2146+
g_assertf (idx == MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX, "mb_inflate_wrapper_data called after data already added");
2147+
}
2148+
21392149
static void
21402150
emit_synchronized_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method)
21412151
{
@@ -2265,20 +2275,26 @@ emit_array_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo
22652275
mono_mb_emit_byte (mb, CEE_RET);
22662276
}
22672277

2278+
static gboolean
2279+
unsafe_accessor_target_type_forbidden (MonoType *target_type);
2280+
22682281
static void
2269-
emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
2282+
emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name)
22702283
{
22712284
// Field access requires a single argument for target type and a return type.
22722285
g_assert (kind == MONO_UNSAFE_ACCESSOR_FIELD || kind == MONO_UNSAFE_ACCESSOR_STATIC_FIELD);
22732286
g_assert (member_name != NULL);
22742287

2275-
MonoType *target_type = sig->params[0]; // params[0] is the field's parent
2276-
MonoType *ret_type = sig->ret;
2277-
if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID) {
2288+
2289+
MonoType *target_type = sig->param_count == 1 ? sig->params[0] : NULL; // params[0] is the field's parent
2290+
2291+
if (sig->param_count != 1 || target_type == NULL || sig->ret->type == MONO_TYPE_VOID || unsafe_accessor_target_type_forbidden (target_type)) {
22782292
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
22792293
return;
22802294
}
22812295

2296+
MonoType *ret_type = sig->ret;
2297+
22822298
MonoClass *target_class = mono_class_from_mono_type_internal (target_type);
22832299
gboolean target_byref = m_type_is_byref (target_type);
22842300
gboolean target_valuetype = m_class_is_valuetype (target_class);
@@ -2307,6 +2323,8 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_
23072323
if (kind == MONO_UNSAFE_ACCESSOR_FIELD)
23082324
mono_mb_emit_ldarg (mb, 0);
23092325
mono_mb_emit_op (mb, kind == MONO_UNSAFE_ACCESSOR_FIELD ? CEE_LDFLDA : CEE_LDSFLDA, target_field);
2326+
if (inflate_generic_data)
2327+
mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_FIELD);
23102328
mono_mb_emit_byte (mb, CEE_RET);
23112329
}
23122330

@@ -2315,7 +2333,7 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_
23152333
* of the expected member method (ie, with the first arg removed)
23162334
*/
23172335
static MonoMethodSignature *
2318-
method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMethodSignature *accessor_sig, MonoGenericContext *ctx)
2336+
method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMethodSignature *accessor_sig)
23192337
{
23202338
MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig);
23212339
g_assert (ret->param_count > 0);
@@ -2332,7 +2350,7 @@ method_sig_from_accessor_sig (MonoMethodBuilder *mb, gboolean hasthis, MonoMetho
23322350
* of the expected constructor method (same args, but return type is void).
23332351
*/
23342352
static MonoMethodSignature *
2335-
ctor_sig_from_accessor_sig (MonoMethodBuilder *mb, MonoMethodSignature *accessor_sig, MonoGenericContext *ctx)
2353+
ctor_sig_from_accessor_sig (MonoMethodBuilder *mb, MonoMethodSignature *accessor_sig)
23362354
{
23372355
MonoMethodSignature *ret = mono_metadata_signature_dup_full (get_method_image (mb->method), accessor_sig);
23382356
ret->hasthis = TRUE; /* ctors are considered instance methods */
@@ -2373,28 +2391,6 @@ emit_missing_method_error (MonoMethodBuilder *mb, MonoError *failure, const char
23732391
}
23742392
}
23752393

2376-
static MonoMethodSignature *
2377-
update_signature (MonoMethod *accessor_method)
2378-
{
2379-
MonoClass *accessor_method_class_instance = accessor_method->klass;
2380-
MonoClass *accessor_method_class = mono_class_get_generic_type_definition (accessor_method_class_instance);
2381-
2382-
const char *accessor_method_name = accessor_method->name;
2383-
2384-
gpointer iter = NULL;
2385-
MonoMethod *m = NULL;
2386-
while ((m = mono_class_get_methods (accessor_method_class, &iter))) {
2387-
if (!m)
2388-
continue;
2389-
2390-
if (strcmp (m->name, accessor_method_name))
2391-
continue;
2392-
2393-
return mono_metadata_signature_dup_full (get_method_image (m), mono_method_signature_internal (m));
2394-
}
2395-
g_assert_not_reached ();
2396-
}
2397-
23982394
static MonoMethod *
23992395
inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_method, MonoError *error)
24002396
{
@@ -2412,7 +2408,7 @@ inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_metho
24122408
}
24132409

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

24262422
MonoType *target_type = sig->ret; // for constructors the return type is the target type
2427-
MonoClass *target_class = mono_class_from_mono_type_internal (target_type);
2428-
2429-
ERROR_DECL(find_method_error);
2430-
if (accessor_method->is_inflated) {
2431-
sig = update_signature(accessor_method);
2432-
target_type = sig->ret;
2433-
}
24342423

24352424
if (target_type == NULL || m_type_is_byref (target_type) || unsafe_accessor_target_type_forbidden (target_type)) {
24362425
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
24372426
return;
24382427
}
2428+
2429+
MonoClass *target_class = mono_class_from_mono_type_internal (target_type);
2430+
2431+
ERROR_DECL(find_method_error);
24392432

2440-
MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig, ctx);
2433+
MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig);
24412434

2442-
MonoClass *in_class = mono_class_get_generic_type_definition (target_class);
2435+
MonoClass *in_class = target_class;
24432436

24442437
MonoMethod *target_method = mono_unsafe_accessor_find_ctor (in_class, member_sig, target_class, find_method_error);
24452438
if (!is_ok (find_method_error) || target_method == NULL) {
@@ -2458,21 +2451,27 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_m
24582451
emit_unsafe_accessor_ldargs (mb, sig, 0);
24592452

24602453
mono_mb_emit_op (mb, CEE_NEWOBJ, target_method);
2454+
if (inflate_generic_data)
2455+
mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_METHOD);
24612456
mono_mb_emit_byte (mb, CEE_RET);
24622457
}
24632458

24642459
static void
2465-
emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
2460+
emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name)
24662461
{
24672462
g_assert (kind == MONO_UNSAFE_ACCESSOR_METHOD || kind == MONO_UNSAFE_ACCESSOR_STATIC_METHOD);
24682463
g_assert (member_name != NULL);
24692464

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

2473-
2468+
MonoType *target_type = sig->param_count >= 1 ? sig->params[0] : NULL;
2469+
2470+
if (sig->param_count < 1 || target_type == NULL || unsafe_accessor_target_type_forbidden (target_type)) {
2471+
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
2472+
return;
2473+
}
24742474

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

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

24842483
ERROR_DECL(find_method_error);
2485-
if (accessor_method->is_inflated) {
2486-
sig = update_signature(accessor_method);
2487-
target_type = sig->params[0];
2488-
}
2489-
2490-
if (sig->param_count < 1 || target_type == NULL || unsafe_accessor_target_type_forbidden (target_type)) {
2491-
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
2492-
return;
2493-
}
24942484

2495-
MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig, ctx);
2485+
MonoMethodSignature *member_sig = method_sig_from_accessor_sig (mb, hasthis, sig);
24962486

2497-
MonoClass *in_class = mono_class_get_generic_type_definition (target_class);
2487+
MonoClass *in_class = target_class;
24982488

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

25242514
mono_mb_emit_op (mb, hasthis ? CEE_CALLVIRT : CEE_CALL, target_method);
2515+
if (inflate_generic_data)
2516+
mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_METHOD);
25252517
mono_mb_emit_byte (mb, CEE_RET);
25262518
}
25272519

25282520
static void
2529-
emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoGenericContext *ctx, MonoUnsafeAccessorKind kind, const char *member_name)
2521+
emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, gboolean inflate_generic_data, MonoMethod *accessor_method, MonoMethodSignature *sig, MonoUnsafeAccessorKind kind, const char *member_name)
25302522
{
2531-
if (accessor_method->is_generic || ctx != NULL) {
2532-
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_Generics");
2533-
return;
2534-
}
2535-
25362523
if (!m_method_is_static (accessor_method)) {
25372524
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_NonStatic");
25382525
return;
@@ -2541,14 +2528,14 @@ emit_unsafe_accessor_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *accessor_
25412528
switch (kind) {
25422529
case MONO_UNSAFE_ACCESSOR_FIELD:
25432530
case MONO_UNSAFE_ACCESSOR_STATIC_FIELD:
2544-
emit_unsafe_accessor_field_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
2531+
emit_unsafe_accessor_field_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name);
25452532
return;
25462533
case MONO_UNSAFE_ACCESSOR_CTOR:
2547-
emit_unsafe_accessor_ctor_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
2534+
emit_unsafe_accessor_ctor_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name);
25482535
return;
25492536
case MONO_UNSAFE_ACCESSOR_METHOD:
25502537
case MONO_UNSAFE_ACCESSOR_STATIC_METHOD:
2551-
emit_unsafe_accessor_method_wrapper (mb, accessor_method, sig, ctx, kind, member_name);
2538+
emit_unsafe_accessor_method_wrapper (mb, inflate_generic_data, accessor_method, sig, kind, member_name);
25522539
return;
25532540
default:
25542541
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "UnsafeAccessor_InvalidKindValue");
@@ -3404,6 +3391,7 @@ mono_marshal_lightweight_init (void)
34043391
cb.emit_return = emit_return_ilgen;
34053392
cb.emit_vtfixup_ftnptr = emit_vtfixup_ftnptr_ilgen;
34063393
cb.mb_skip_visibility = mb_skip_visibility_ilgen;
3394+
cb.mb_inflate_wrapper_data = mb_inflate_wrapper_data_ilgen;
34073395
cb.mb_emit_exception = mb_emit_exception_ilgen;
34083396
cb.mb_emit_exception_for_error = mb_emit_exception_for_error_ilgen;
34093397
cb.mb_emit_byte = mb_emit_byte_ilgen;

0 commit comments

Comments
 (0)