[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
dotnet · Mar 13, 2024 · 4cf19ee · 4cf19ee
1 parent 07b705b
commit 4cf19ee
Show file tree

Hide file tree

Showing 14 changed files with 35 additions and 41 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -344,6 +344,7 @@
         ret
     }
 
+    .field public static class InvalidCSharp.GenericClass_Over`1<!T> 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<class InvalidCSharp.ByRefLikeTypeWithInterface, valuetype InvalidCSharp.ByRefLikeTypeWithInterface>::.ctor()
+        newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2<class InvalidCSharp.EmptyInterface, valuetype InvalidCSharp.ByRefLikeTypeWithInterface>::.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
@@ -2,7 +2,6 @@
   <PropertyGroup>
     <OutputType>library</OutputType>
     <MonoAotIncompatible>true</MonoAotIncompatible>
-    <DisableProjectBuild Condition="'$(RuntimeFlavor)' == 'Mono'">true</DisableProjectBuild>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="InvalidCSharp.il" />

diff --git 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
@@ -2,7 +2,6 @@
   <PropertyGroup>
     <!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
     <RequiresProcessIsolation>true</RequiresProcessIsolation>
-    <DisableProjectBuild Condition="'$(RuntimeFlavor)' == 'Mono'">true</DisableProjectBuild>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="Validate.cs" />