[runtime] Allocate the memory for gshared gparams from image sets. (#19361) (#19419)

Fixes https://github.com/mono/mono/issues/18127.

4 files changed, 68 insertions, 29 deletions

diff --git a/mono/metadata/image.c b/mono/metadata/image.c
index 8a9f6881166..b2ee484acdd 100644
--- a/mono/metadata/image.c
+++ b/mono/metadata/image.c
@@ -2450,10 +2450,6 @@ mono_image_close_except_pools (MonoImage *image)
 
 	mono_wrapper_caches_free (&image->wrapper_caches);
 
-	for (i = 0; i < image->gshared_types_len; ++i)
-		free_hash (image->gshared_types [i]);
-	g_free (image->gshared_types);
-
 	/* The ownership of signatures is not well defined */
 	g_hash_table_destroy (image->memberref_signatures);
 	g_hash_table_destroy (image->method_signatures);
diff --git a/mono/metadata/metadata-internals.h b/mono/metadata/metadata-internals.h
index dfa70648a92..9388d69b0fd 100644
--- a/mono/metadata/metadata-internals.h
+++ b/mono/metadata/metadata-internals.h
@@ -567,11 +567,6 @@ struct _MonoImage {
 	GHashTable *pinvoke_scopes;
 #endif
 
-	/* Indexed by MonoGenericParam pointers */
-	GHashTable **gshared_types;
-	/* The length of the above array */
-	int gshared_types_len;
-
 	/* The loader used to load this image */
 	MonoImageLoader *loader;
 
@@ -613,6 +608,11 @@ typedef struct {
 
 	GHashTable *aggregate_modifiers_cache;
 
+	/* Indexed by MonoGenericParam pointers */
+	GHashTable **gshared_types;
+	/* The length of the above array */
+	int gshared_types_len;
+
 	mono_mutex_t    lock;
 
 	/*
@@ -897,6 +897,12 @@ mono_image_set_strdup (MonoImageSet *set, const char *s);
 MonoImageSet *
 mono_metadata_get_image_set_for_aggregate_modifiers (MonoAggregateModContainer *amods);
 
+MonoImageSet *
+mono_metadata_get_image_set_for_type (MonoType *type);
+
+MonoImageSet *
+mono_metadata_merge_image_sets (MonoImageSet *set1, MonoImageSet *set2);
+
 #define mono_image_set_new0(image,type,size) ((type *) mono_image_set_alloc0 (image, sizeof (type)* (size)))
 
 gboolean
diff --git a/mono/metadata/metadata.c b/mono/metadata/metadata.c
index 9cb701e1107..5f6568da9b0 100644
--- a/mono/metadata/metadata.c
+++ b/mono/metadata/metadata.c
@@ -2796,6 +2796,12 @@ delete_image_set (MonoImageSet *set)
 
 	g_hash_table_destroy (set->aggregate_modifiers_cache);
 
+	for (i = 0; i < set->gshared_types_len; ++i) {
+		if (set->gshared_types [i])
+			g_hash_table_destroy (set->gshared_types [i]);
+	}
+	g_free (set->gshared_types);
+
 	mono_wrapper_caches_free (&set->wrapper_caches);
 
 	image_sets_lock ();
@@ -3312,13 +3318,13 @@ mono_metadata_get_inflated_signature (MonoMethodSignature *sig, MonoGenericConte
 }
 
 MonoImageSet *
-mono_metadata_get_image_set_for_class (MonoClass *klass)
+mono_metadata_get_image_set_for_type (MonoType *type)
 {
 	MonoImageSet *set;
 	CollectData image_set_data;
 
 	collect_data_init (&image_set_data);
-	collect_type_images (m_class_get_byval_arg (klass), &image_set_data);
+	collect_type_images (type, &image_set_data);
 	set = get_image_set (image_set_data.images, image_set_data.nimages);
 	collect_data_free (&image_set_data);
 
@@ -3326,6 +3332,12 @@ mono_metadata_get_image_set_for_class (MonoClass *klass)
 }
 
 MonoImageSet *
+mono_metadata_get_image_set_for_class (MonoClass *klass)
+{
+	return mono_metadata_get_image_set_for_type (m_class_get_byval_arg (klass));
+}
+
+MonoImageSet *
 mono_metadata_get_image_set_for_method (MonoMethodInflated *method)
 {
 	MonoImageSet *set;
@@ -3352,6 +3364,29 @@ mono_metadata_get_image_set_for_aggregate_modifiers (MonoAggregateModContainer *
 	return set;
 }
 
+MonoImageSet *
+mono_metadata_merge_image_sets (MonoImageSet *set1, MonoImageSet *set2)
+{
+	MonoImage **images = g_newa (MonoImage*, set1->nimages + set2->nimages);
+
+	/* Add images from set1 */
+	memcpy (images, set1->images, sizeof (MonoImage*) * set1->nimages);
+
+	int nimages = set1->nimages;
+	// FIXME: Quaratic
+	/* Add images from set2 */
+	for (int i = 0; i < set2->nimages; ++i) {
+		int j;
+		for (j = 0; j < set1->nimages; ++j) {
+			if (set2->images [i] == set1->images [j])
+				break;
+		}
+		if (j == set1->nimages)
+			images [nimages ++] = set2->images [i];
+	}
+	return get_image_set (images, nimages);
+}
+
 static gboolean
 type_is_gtd (MonoType *type)
 {
diff --git a/mono/mini/mini-generic-sharing.c b/mono/mini/mini-generic-sharing.c
index 77736d70fbf..2eba1e575ba 100644
--- a/mono/mini/mini-generic-sharing.c
+++ b/mono/mini/mini-generic-sharing.c
@@ -3894,12 +3894,15 @@ shared_gparam_equal (gconstpointer ka, gconstpointer kb)
 MonoType*
 mini_get_shared_gparam (MonoType *t, MonoType *constraint)
 {
+	MonoImageSet *set;
 	MonoGenericParam *par = t->data.generic_param;
 	MonoGSharedGenericParam *copy, key;
 	MonoType *res;
 	MonoImage *image = NULL;
 	char *name;
 
+	set = mono_metadata_merge_image_sets (mono_metadata_get_image_set_for_type (t), mono_metadata_get_image_set_for_type (constraint));
+
 	memset (&key, 0, sizeof (key));
 	key.parent = par;
 	key.param.gshared_constraint = constraint;
@@ -3911,23 +3914,24 @@ mini_get_shared_gparam (MonoType *t, MonoType *constraint)
 	 * Need a cache to ensure the newly created gparam
 	 * is unique wrt T/CONSTRAINT.
 	 */
-	mono_image_lock (image);
-	if (!image->gshared_types) {
-		image->gshared_types_len = MONO_TYPE_INTERNAL;
-		image->gshared_types = g_new0 (GHashTable*, image->gshared_types_len);
-	}
-	if (!image->gshared_types [constraint->type])
-		image->gshared_types [constraint->type] = g_hash_table_new (shared_gparam_hash, shared_gparam_equal);
-	res = (MonoType *)g_hash_table_lookup (image->gshared_types [constraint->type], &key);
-	mono_image_unlock (image);
+	mono_image_set_lock (set);
+	if (!set->gshared_types) {
+		set->gshared_types_len = MONO_TYPE_INTERNAL;
+		set->gshared_types = g_new0 (GHashTable*, set->gshared_types_len);
+	}
+	if (!set->gshared_types [constraint->type])
+		set->gshared_types [constraint->type] = g_hash_table_new (shared_gparam_hash, shared_gparam_equal);
+	res = (MonoType *)g_hash_table_lookup (set->gshared_types [constraint->type], &key);
+	mono_image_set_unlock (set);
 	if (res)
 		return res;
-	copy = (MonoGSharedGenericParam *)mono_image_alloc0 (image, sizeof (MonoGSharedGenericParam));
+	copy = (MonoGSharedGenericParam *)mono_image_set_alloc0 (set, sizeof (MonoGSharedGenericParam));
 	memcpy (&copy->param, par, sizeof (MonoGenericParamFull));
 	copy->param.info.pklass = NULL;
-	constraint = mono_metadata_type_dup (image, constraint);
+	// FIXME:
+	constraint = mono_metadata_type_dup (NULL, constraint);
 	name = get_shared_gparam_name (constraint->type, ((MonoGenericParamFull*)copy)->info.name);
-	copy->param.info.name = mono_image_strdup (image, name);
+	copy->param.info.name = mono_image_set_strdup (set, name);
 	g_free (name);
 
 	copy->param.owner = par->owner;
@@ -3938,12 +3942,10 @@ mini_get_shared_gparam (MonoType *t, MonoType *constraint)
 	res = mono_metadata_type_dup (NULL, t);
 	res->data.generic_param = (MonoGenericParam*)copy;
 
-	if (image) {
-		mono_image_lock (image);
-		/* Duplicates are ok */
-		g_hash_table_insert (image->gshared_types [constraint->type], copy, res);
-		mono_image_unlock (image);
-	}
+	mono_image_set_lock (set);
+	/* Duplicates are ok */
+	g_hash_table_insert (set->gshared_types [constraint->type], copy, res);
+	mono_image_set_unlock (set);
 
 	return res;
 }