From 4cf19ee05d3efc5372b8f308240688bacce09208 Mon Sep 17 00:00:00 2001 From: Fan Yang <52458914+fanyang-mono@users.noreply.github.com> Date: Wed, 13 Mar 2024 11:37:02 -0400 Subject: [PATCH] [mono] Add basic ref struct support for generic parameter (#99081) * Add basic ref struct support for generic parameter * Try to fix format * Address review feedback and enable more tests * Fix format * Commenting out methods with array of T to workaround AOT failure * Split up the testcases * Throw type loading exception when trying to create type arrays with ByRefLike types * Fix box and stsfld/ldsfld/ldsflda * Disable a test cause it is failing with interpreter * For load/store static field, only error out when it is a generic class with byreflike type parameter * Fix interpreter box byreflike exception ID * Relax type check * Enable more tests * Revert the change for relaxing type check * Remove unreached check * remove unused variable --- src/mono/mono/metadata/class-init.c | 2 +- src/mono/mono/metadata/class.c | 3 --- src/mono/mono/metadata/loader.c | 1 - src/mono/mono/metadata/metadata.c | 4 ++-- src/mono/mono/metadata/object-internals.h | 21 ++++++++++--------- src/mono/mono/metadata/verify.c | 3 +++ src/mono/mono/mini/interp/transform.c | 2 +- src/mono/mono/mini/method-to-ir.c | 21 +++++++++---------- src/mono/mono/mini/mini-generic-sharing.c | 4 ++++ .../ByRefLike/GenericTypeSubstitution.cs | 3 --- .../generics/ByRefLike/InvalidCSharp.il | 3 ++- .../generics/ByRefLike/InvalidCSharp.ilproj | 1 - .../generics/ByRefLike/Validate.cs | 7 +------ .../generics/ByRefLike/Validate.csproj | 1 - 14 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 8e064696fe762..ffefe2faa77a7 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -2953,7 +2953,7 @@ mono_class_init_internal (MonoClass *klass) if (klass->inited || mono_class_has_failure (klass)) return !mono_class_has_failure (klass); - /*g_print ("Init class %s\n", mono_type_get_full_name (klass));*/ + // g_print ("Init class %s\n", mono_type_get_full_name (klass)); /* * This function can recursively call itself. diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index c5fcd2a8d7a18..244567efeaf17 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -640,10 +640,7 @@ mono_type_is_valid_generic_argument (MonoType *type) { switch (type->type) { case MONO_TYPE_VOID: - case MONO_TYPE_TYPEDBYREF: return FALSE; - case MONO_TYPE_VALUETYPE: - return !m_class_is_byreflike (type->data.klass); default: return TRUE; } diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 67c613b9631ae..5f9e5c0e7d1f1 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -823,7 +823,6 @@ mono_method_search_in_array_class (MonoClass *klass, const char *name, MonoMetho int i; mono_class_setup_methods (klass); - g_assert (!mono_class_has_failure (klass)); /*FIXME this should not fail, right?*/ int mcount = mono_class_get_method_count (klass); MonoMethod **klass_methods = m_class_get_methods (klass); for (i = 0; i < mcount; ++i) { diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index f716029223298..b432e7f07dc54 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -3942,7 +3942,7 @@ compare_type_literals (MonoImage *image, int class_type, int type_type, MonoErro if (class_type == MONO_TYPE_STRING || class_type == MONO_TYPE_OBJECT) return TRUE; //XXX stringify this argument - mono_error_set_bad_image (error, image, "Expected reference type but got type kind %d", class_type); + mono_error_set_type_load_name (error, NULL, NULL, "Expected reference type but got type kind %d", class_type); return FALSE; } @@ -3966,7 +3966,7 @@ compare_type_literals (MonoImage *image, int class_type, int type_type, MonoErro return TRUE; default: //XXX stringify this argument - mono_error_set_bad_image (error, image, "Expected value type but got type kind %d", class_type); + mono_error_set_type_load_name (error, NULL, NULL, "Expected value type but got type kind %d", class_type); return FALSE; } } diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 257d06a915da9..daaba307f17db 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -1439,16 +1439,17 @@ typedef struct { /* Keep in sync with System.GenericParameterAttributes */ typedef enum { - GENERIC_PARAMETER_ATTRIBUTE_NON_VARIANT = 0, - GENERIC_PARAMETER_ATTRIBUTE_COVARIANT = 1, - GENERIC_PARAMETER_ATTRIBUTE_CONTRAVARIANT = 2, - GENERIC_PARAMETER_ATTRIBUTE_VARIANCE_MASK = 3, - - GENERIC_PARAMETER_ATTRIBUTE_NO_SPECIAL_CONSTRAINT = 0, - GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT = 4, - GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT = 8, - GENERIC_PARAMETER_ATTRIBUTE_CONSTRUCTOR_CONSTRAINT = 16, - GENERIC_PARAMETER_ATTRIBUTE_SPECIAL_CONSTRAINTS_MASK = 28 + GENERIC_PARAMETER_ATTRIBUTE_NON_VARIANT = 0x0000, + GENERIC_PARAMETER_ATTRIBUTE_COVARIANT = 0x0001, + GENERIC_PARAMETER_ATTRIBUTE_CONTRAVARIANT = 0x0002, + GENERIC_PARAMETER_ATTRIBUTE_VARIANCE_MASK = 0x0003, + + GENERIC_PARAMETER_ATTRIBUTE_NO_SPECIAL_CONSTRAINT = 0x0000, + GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT = 0x0004, + GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT = 0x0008, + GENERIC_PARAMETER_ATTRIBUTE_CONSTRUCTOR_CONSTRAINT = 0x0010, + GENERIC_PARAMETER_ATTRIBUTE_ACCEPT_BYREFLIKE_CONSTRAINTS = 0x0020, // type argument can be ByRefLike + GENERIC_PARAMETER_ATTRIBUTE_SPECIAL_CONSTRAINTS_MASK = 0x003c } GenericParameterAttributes; typedef struct { diff --git a/src/mono/mono/metadata/verify.c b/src/mono/mono/metadata/verify.c index 660f6226eb009..621599fa4eeeb 100644 --- a/src/mono/mono/metadata/verify.c +++ b/src/mono/mono/metadata/verify.c @@ -87,6 +87,9 @@ is_valid_generic_instantiation (MonoGenericContainer *gc, MonoGenericContext *co return FALSE; } + if (m_class_is_byreflike (paramClass) && (param_info->flags & GENERIC_PARAMETER_ATTRIBUTE_ACCEPT_BYREFLIKE_CONSTRAINTS) == 0) + return FALSE; + if (!param_info->constraints && !(param_info->flags & GENERIC_PARAMETER_ATTRIBUTE_SPECIAL_CONSTRAINTS_MASK)) continue; diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index a007007dfe290..d91e6b413b336 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -6905,7 +6905,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, td->ip += 5; } else { if (G_UNLIKELY (m_class_is_byreflike (klass))) { - mono_error_set_bad_image (error, image, "Cannot box IsByRefLike type '%s.%s'", m_class_get_name_space (klass), m_class_get_name (klass)); + mono_error_set_invalid_program (error, "Cannot box IsByRefLike type '%s.%s'", m_class_get_name_space (klass), m_class_get_name (klass)); goto exit; } diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index d905d8412447c..187e1a61d5c7c 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -2320,16 +2320,13 @@ emit_type_load_failure (MonoCompile* cfg, MonoClass* klass) } static void -emit_invalid_program_with_msg (MonoCompile *cfg, MonoError *error_msg, MonoMethod *caller, MonoMethod *callee) +emit_invalid_program_with_msg (MonoCompile *cfg, char *error_msg) { - g_assert (!is_ok (error_msg)); - - char *str = mono_mem_manager_strdup (cfg->mem_manager, mono_error_get_message (error_msg)); MonoInst *iargs[1]; if (cfg->compile_aot) - EMIT_NEW_LDSTRLITCONST (cfg, iargs [0], str); + EMIT_NEW_LDSTRLITCONST (cfg, iargs [0], error_msg); else - EMIT_NEW_PCONST (cfg, iargs [0], str); + EMIT_NEW_PCONST (cfg, iargs [0], error_msg); mono_emit_jit_icall (cfg, mono_throw_invalid_program, iargs); } @@ -3416,8 +3413,8 @@ mini_emit_box (MonoCompile *cfg, MonoInst *val, MonoClass *klass, int context_us MonoInst *alloc, *ins; if (G_UNLIKELY (m_class_is_byreflike (klass))) { - mono_error_set_bad_image (cfg->error, m_class_get_image (cfg->method->klass), "Cannot box IsByRefLike type '%s.%s'", m_class_get_name_space (klass), m_class_get_name (klass)); - mono_cfg_set_exception (cfg, MONO_EXCEPTION_MONO_ERROR); + mono_error_set_invalid_program (cfg->error, "Cannot box IsByRefLike type '%s.%s'", m_class_get_name_space (klass), m_class_get_name (klass)); + mono_cfg_set_exception (cfg, MONO_EXCEPTION_INVALID_PROGRAM); return NULL; } @@ -9282,7 +9279,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b mono_save_token_info (cfg, image, token, cmethod); - if (!mono_class_init_internal (cmethod->klass)) + if (mono_class_has_failure (cmethod->klass) || !mono_class_init_internal (cmethod->klass)) TYPE_LOAD_ERROR (cmethod->klass); context_used = mini_method_check_context_used (cfg, cmethod); @@ -9995,8 +9992,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b case MONO_CEE_STSFLD: { MonoClassField *field; guint foffset; - gboolean is_instance; gpointer addr = NULL; + gboolean is_instance; gboolean is_special_static; MonoType *ftype; MonoInst *store_val = NULL; @@ -10081,6 +10078,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b is_instance = FALSE; } + context_used = mini_class_check_context_used (cfg, klass); if (il_op == MONO_CEE_LDSFLD) { @@ -11845,7 +11843,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b /* if we couldn't create a wrapper because cmethod isn't supposed to have an UnmanagedCallersOnly attribute, follow CoreCLR behavior and throw when the method with the ldftn is executing, not when it is being compiled. */ - emit_invalid_program_with_msg (cfg, wrapper_error, method, cmethod); + char *err_msg = mono_mem_manager_strdup (cfg->mem_manager, mono_error_get_message (wrapper_error)); + emit_invalid_program_with_msg (cfg, err_msg); mono_error_cleanup (wrapper_error); EMIT_NEW_PCONST (cfg, ins, NULL); *sp++ = ins; diff --git a/src/mono/mono/mini/mini-generic-sharing.c b/src/mono/mono/mini/mini-generic-sharing.c index 58440dcdc3599..cd482c75e367a 100644 --- a/src/mono/mono/mini/mini-generic-sharing.c +++ b/src/mono/mono/mini/mini-generic-sharing.c @@ -584,12 +584,14 @@ inflate_info (MonoMemoryManager *mem_manager, MonoRuntimeGenericContextInfoTempl if (m_class_get_byval_arg (inflated_class)->type == MONO_TYPE_ARRAY || m_class_get_byval_arg (inflated_class)->type == MONO_TYPE_SZARRAY) { + g_assert (!mono_class_has_failure (inflated_class)); inflated_method = mono_method_search_in_array_class (inflated_class, method->name, method->signature); } else { inflated_method = mono_class_inflate_generic_method_checked (method, context, error); g_assert (is_ok (error)); /* FIXME don't swallow the error */ } + g_assert (inflated_method); mono_class_init_internal (inflated_method->klass); g_assert (inflated_method->klass == inflated_class); return inflated_method; @@ -648,12 +650,14 @@ inflate_info (MonoMemoryManager *mem_manager, MonoRuntimeGenericContextInfoTempl if (m_class_get_byval_arg (inflated_class)->type == MONO_TYPE_ARRAY || m_class_get_byval_arg (inflated_class)->type == MONO_TYPE_SZARRAY) { + g_assert (!mono_class_has_failure (inflated_class)); inflated_method = mono_method_search_in_array_class (inflated_class, method->name, method->signature); } else { inflated_method = mono_class_inflate_generic_method_checked (method, context, error); g_assert (is_ok (error)); /* FIXME don't swallow the error */ } + g_assert (inflated_method); mono_class_init_internal (inflated_method->klass); g_assert (inflated_method->klass == inflated_class); diff --git a/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs b/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs index b35e59576abba..75b22b974792c 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs @@ -12,7 +12,6 @@ public class GenericTypeSubstitution { [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void AllowByRefLike_Substituted_For_AllowByRefLike() { Console.WriteLine($"{nameof(AllowByRefLike_Substituted_For_AllowByRefLike)}..."); @@ -23,7 +22,6 @@ public static void AllowByRefLike_Substituted_For_AllowByRefLike() } [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void NonByRefLike_Substituted_For_AllowByRefLike() { Console.WriteLine($"{nameof(NonByRefLike_Substituted_For_AllowByRefLike)}..."); @@ -34,7 +32,6 @@ public static void NonByRefLike_Substituted_For_AllowByRefLike() } [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void AllowByRefLike_Substituted_For_NonByRefLike() { Console.WriteLine($"{nameof(AllowByRefLike_Substituted_For_NonByRefLike)}..."); diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index de2b842fed9e8..72f0dc67f69e8 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -344,6 +344,7 @@ ret } + .field public static class InvalidCSharp.GenericClass_Over`1 StaticField1 .field public static !T StaticField } @@ -551,7 +552,7 @@ .method public hidebysig static class [System.Runtime]System.Type GenericByRefLike_ConstraintsAreIndependent_Interface_ByRefLike_Invalid() cil managed { - newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() + newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() callvirt instance class [System.Runtime]System.Type [System.Runtime]System.Object::GetType() ret } diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.ilproj b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.ilproj index 58dc9527f8046..8e167b20da08e 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.ilproj +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.ilproj @@ -2,7 +2,6 @@ library true - true diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 2a9ef5b70873a..4d7c05d6db962 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -12,7 +12,6 @@ public class Validate { [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void Validate_TypeLoad() { Console.WriteLine($"{nameof(Validate_TypeLoad)}..."); @@ -32,7 +31,6 @@ public static void Validate_TypeLoad() } [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void Validate_Casting_Scenarios() { Console.WriteLine($"{nameof(Validate_Casting_Scenarios)}..."); @@ -47,7 +45,7 @@ public static void Validate_Casting_Scenarios() } [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] + [SkipOnMono("https://github.com/dotnet/runtime/issues/99379")] public static void Validate_RecognizedOpCodeSequences_Scenarios() { Console.WriteLine($"{nameof(Validate_RecognizedOpCodeSequences_Scenarios)}..."); @@ -59,7 +57,6 @@ public static void Validate_RecognizedOpCodeSequences_Scenarios() } [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void Validate_InvalidOpCode_Scenarios() { Console.WriteLine($"{nameof(Validate_InvalidOpCode_Scenarios)}..."); @@ -76,7 +73,6 @@ public static void Validate_InvalidOpCode_Scenarios() } [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void Validate_Inlining_Behavior() { Console.WriteLine($"{nameof(Validate_Inlining_Behavior)}..."); @@ -88,7 +84,6 @@ public static void Validate_Inlining_Behavior() } // [Fact] - [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void Validate_MemberDiscoveryViaReflection_ForSpanReadOnlySpan() { Console.WriteLine($"{nameof(Validate_MemberDiscoveryViaReflection_ForSpanReadOnlySpan)}..."); diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.csproj b/src/tests/Loader/classloader/generics/ByRefLike/Validate.csproj index 8bb4ee77df067..83580f549743d 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.csproj +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.csproj @@ -2,7 +2,6 @@ true - true