Merge pull request #1211 from Unity-Technologies/fix-recursive-type-exception

[metadata] Fields whose types are gparams with a reference type const…

Author: UnityAlex

mono/metadata/class-internals.h

@@ -1186,6 +1186,7 @@ GENERATE_GET_CLASS_WITH_CACHE_DECL (variant)
 #endif
 
 GENERATE_GET_CLASS_WITH_CACHE_DECL (appdomain_unloaded_exception)
+GENERATE_GET_CLASS_WITH_CACHE_DECL (valuetype)
 
 extern MonoDefaults mono_defaults;
 
@@ -1371,6 +1372,9 @@ mono_class_vtable_full (MonoDomain *domain, MonoClass *klass, MonoError *error);
 gboolean
 mono_class_is_assignable_from_slow (MonoClass *target, MonoClass *candidate);
 
+MonoClass*
+mono_generic_param_get_base_type (MonoClass *klass);
+
 gboolean
 mono_class_has_variant_generic_params (MonoClass *klass);
 

mono/metadata/class.c

@@ -86,6 +86,8 @@ static gboolean mono_class_set_failure (MonoClass *klass, MonoErrorBoxed *boxed_
 
 static gboolean class_kind_may_contain_generic_instances (MonoTypeKind kind);
 
+GENERATE_GET_CLASS_WITH_CACHE (valuetype, "System", "ValueType");
+
 
 /*
 We use gclass recording to allow recursive system f types to be referenced by a parent.
@@ -1760,6 +1762,59 @@ type_has_references (MonoClass *klass, MonoType *ftype)
 	return FALSE;
 }
 
+/**
+ * mono_class_is_gparam_with_nonblittable_parent:
+ * \param klass  a generic parameter
+ *
+ * \returns TRUE if \p klass is definitely not blittable.
+ *
+ * A parameter is definitely not blittable if it has the IL 'reference'
+ * constraint, or if it has a class specified as a parent.  If it has an IL
+ * 'valuetype' constraint or no constraint at all or only interfaces as
+ * constraints, we return FALSE because the parameter may be instantiated both
+ * with blittable and non-blittable types.
+ *
+ * If the paramter is a generic sharing parameter, we look at its gshared_constraint->blittable bit.
+ */
+static gboolean
+mono_class_is_gparam_with_nonblittable_parent (MonoClass *klass)
+{
+	MonoType *type = &klass->byval_arg;
+	g_assert (mono_type_is_generic_parameter (type));
+	MonoGenericParam *gparam = type->data.generic_param;
+	if (mono_generic_param_info (gparam) != NULL)
+	{
+		if ((mono_generic_param_info (gparam)->flags & GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT) != 0)
+			return TRUE;
+		if ((mono_generic_param_info (gparam)->flags & GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT) != 0)
+			return FALSE;
+	}
+
+	if (gparam->gshared_constraint) {
+		MonoClass *constraint_class = mono_class_from_mono_type (gparam->gshared_constraint);
+		return !constraint_class->blittable;
+	}
+
+	if (mono_generic_param_owner (gparam)->is_anonymous)
+		return FALSE;
+
+	/* We could have:  T : U,  U : Base.  So have to follow the constraints. */
+	MonoClass *parent_class = mono_generic_param_get_base_type (klass);
+	g_assert (!MONO_CLASS_IS_INTERFACE_INTERNAL (parent_class));
+	/* Parent can only be: System.Object, System.ValueType or some specific base class.
+	 *
+	 * If the parent_class is ValueType, the valuetype constraint would be set, above, so
+	 * we wouldn't get here.
+	 *
+	 * If there was a reference constraint, the parent_class would be System.Object,
+	 * but we would have returned early above.
+	 *
+	 * So if we get here, there is either no base class constraint at all,
+	 * in which case parent_class would be set to System.Object, or there is none at all.
+	 */
+	return parent_class != mono_defaults.object_class;
+}
+
 /*
  * mono_class_layout_fields:
  * @class: a class
@@ -1877,6 +1932,9 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_
 		if (blittable) {
 			if (field->type->byref || MONO_TYPE_IS_REFERENCE (field->type)) {
 				blittable = FALSE;
+			} else if (mono_type_is_generic_parameter (field->type) &&
+				mono_class_is_gparam_with_nonblittable_parent (mono_class_from_mono_type (field->type))) {
+				blittable = FALSE;
 			} else {
 				MonoClass *field_class = mono_class_from_mono_type (field->type);
 				if (field_class) {
@@ -8645,6 +8703,60 @@ mono_class_is_assignable_from_slow (MonoClass *target, MonoClass *candidate)
 	return FALSE;
 }
 
+/**
+ * mono_generic_param_get_base_type:
+ *
+ * Return the base type of the given generic parameter from its constraints.
+ *
+ * Could be another generic parameter, or it could be Object or ValueType.
+ */
+MonoClass*
+mono_generic_param_get_base_type (MonoClass *klass)
+{
+	MonoType *type = &klass->byval_arg;
+	g_assert (mono_type_is_generic_argument (type));
+
+	MonoGenericParam *gparam = type->data.generic_param;
+
+	g_assert (gparam->owner && !gparam->owner->is_anonymous);
+
+	MonoClass **constraints = mono_generic_container_get_param_info (gparam->owner, gparam->num)->constraints;
+
+	MonoClass *base_class = mono_defaults.object_class;
+
+	if (constraints) {
+		int i;
+		for (i = 0; constraints [i]; ++i) {
+			MonoClass *constraint = constraints[i];
+
+			if (MONO_CLASS_IS_INTERFACE_INTERNAL (constraint))
+				continue;
+
+			MonoType *constraint_type = &constraint->byval_arg;
+			if (mono_type_is_generic_argument (constraint_type)) {
+				MonoGenericParam *constraint_param = constraint_type->data.generic_param;
+				MonoGenericParamInfo *constraint_info = mono_generic_param_info (constraint_param);
+				if ((constraint_info->flags & GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT) == 0 &&
+				    (constraint_info->flags & GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT) == 0)
+					continue;
+			}
+
+			base_class = constraint;
+		}
+
+	}
+
+	if (base_class == mono_defaults.object_class)
+	{
+		MonoGenericParamInfo *gparam_info = mono_generic_param_info (gparam);
+		if ((gparam_info->flags & GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT) != 0) {
+			base_class = mono_class_get_valuetype_class ();
+		}
+	}
+
+	return base_class;
+}
+
 /**
  * mono_class_get_cctor:
  * \param klass A MonoClass pointer

mono/metadata/metadata-internals.h

@@ -974,5 +974,7 @@ mono_signature_get_managed_fmt_string (MonoMethodSignature *sig);
 gboolean
 mono_type_in_image (MonoType *type, MonoImage *image);
 
+#define MONO_CLASS_IS_INTERFACE_INTERNAL(c) ((mono_class_get_flags (c) & TYPE_ATTRIBUTE_INTERFACE) || mono_type_is_generic_parameter (&c->byval_arg))
+
 #endif /* __MONO_METADATA_INTERNALS_H__ */
 

mono/metadata/metadata.c

@@ -6815,6 +6815,11 @@ mono_type_is_pointer (MonoType *type)
 mono_bool
 mono_type_is_reference (MonoType *type)
 {
+	/* NOTE: changing this function to return TRUE more often may have
+	 * consequences for generic sharing in the AOT compiler.  In
+	 * particular, returning TRUE for generic parameters with a 'class'
+	 * constraint may cause crashes.
+	 */
 	return (type && (((type->type == MONO_TYPE_STRING) ||
 		(type->type == MONO_TYPE_SZARRAY) || (type->type == MONO_TYPE_CLASS) ||
 		(type->type == MONO_TYPE_OBJECT) || (type->type == MONO_TYPE_ARRAY)) ||

mono/tests/Makefile.am

@@ -374,6 +374,7 @@ TESTS_CS_SRC=		\
 	generic-stack-traces2.2.cs	\
 	bug-472600.2.cs	\
 	recursive-generics.2.cs	\
+	recursive-generics.3.cs	\
 	bug-473482.2.cs	\
 	bug-473999.2.cs	\
 	bug-479763.2.cs	\

mono/tests/recursive-generics.3.cs

@@ -0,0 +1,45 @@
+using System;
+
+class Program {
+	static void Main (string[] args)
+	{
+		// If this runs without a TLE, the test passed.  A
+		// TypeLoadException due to recursion during type
+		// initialization is a failure.
+		var subC = new SubClass ();
+		Console.WriteLine (subC.GetTest ());
+		// same as above, but try to land in generic sharing code.
+		var genSubC = new GenericSubClass<object> ();
+		Console.WriteLine (genSubC.GetTest ());
+	}
+}
+
+public struct ValueTest<U> {
+        // When U is instantiated with T, from BaseClass, we know it'll be a
+	// reference field without having to fully initialize its parent
+	// (namely BaseClass<T> itself), so we know the instantiation
+	// ValueTest<T> won't be blittable.
+	public readonly U value;
+}
+
+public abstract class BaseClass<T> where T : BaseClass<T> {
+	public ValueTest<T> valueTest = default (ValueTest<T>);
+}
+
+public class SubClass : BaseClass<SubClass> {
+	private string test = "test";
+
+	public string GetTest()
+	{
+		return test;
+	}
+}
+
+public class GenericSubClass<T> : BaseClass<GenericSubClass<T>> {
+	private string test = "test";
+
+	public string GetTest()
+	{
+		return test;
+	}
+}