2020 02 backport metadata fixes (#21190)mono-6.12.0.151

* [mono] Fix race during mono_image_storage_open (#21142)

* The mono_refcount_inc call in mono_image_storage_trypublish or
  mono_image_storage_tryaddref may abort when racing against a
  mono_image_storage_dtor that already decremented the refcount.

This race triggered in some cases when building aspnetcore using a Mono-based dotnet host SDK.   The problem is that `mono_image_storage_close` runs outside the `mono_images_storage_lock` (and this may be unavoidable due to lock ordering concerns).  Therefore, we can have a refcount that was already decremented to zero, but before `mono_image_storage_dtor` finishes removing the object from `images_storage_hash`, a parallel `mono_image_storage_trypublish` may have retrieved it from there.  In that case, the `mono_refcount_inc` call will abort.

Fixed by detecting that case via `mono_refcount_tryinc` instead, and simply treating the object as if it had already been removed.  It will in time actually get removed, either by the parallel `mono_image_storage_dtor`, or else by the `g_hash_table_insert` in `mono_image_storage_trypublish` (which will safely replace it with the new object, and `mono_image_storage_dtor` will then detect that and skip removal).

Co-authored-by: uweigand <uweigand@users.noreply.github.com>

* [metadata] Handle MONO_TYPE_FNPTR case in collect_type_images (#19434)

Fixes abort when PTR-FNPTR field signature is encountered.

Fixes https://github.com/mono/mono/issues/12098
Fixes https://github.com/mono/mono/issues/17113
Fixes https://github.com/mono/mono/issues/19433

* Ensure generic parameter constraint type is included when building image (#19395)

sets.

Making associated change to type_in_image to also check the constrained type for a match. Re-adding asserts now they they no longer trigger.

fixup, accidentally used old function

adjusting coding convention to K&R

Co-authored-by: monojenkins <jo.shields+jenkins@xamarin.com>
Co-authored-by: uweigand <uweigand@users.noreply.github.com>
Co-authored-by: Jeff Smith <whydoubt@gmail.com>
Co-authored-by: Alex Thibodeau <alexthibodeau@unity3d.com>

2 files changed, 20 insertions, 5 deletions

diff --git a/mono/metadata/image.c b/mono/metadata/image.c
index b2ee484acdd..e9ba9bc6c32 100644
--- a/mono/metadata/image.c
+++ b/mono/metadata/image.c
@@ -1425,8 +1425,11 @@ mono_image_storage_trypublish (MonoImageStorage *candidate, MonoImageStorage **o
 	gboolean result;
 	mono_images_storage_lock ();
 	MonoImageStorage *val = (MonoImageStorage *)g_hash_table_lookup (images_storage_hash, candidate->key);
+	if (val && !mono_refcount_tryinc (val)) {
+		// We raced against a mono_image_storage_dtor in progress.
+		val = NULL;
+	}
 	if (val) {
-		mono_refcount_inc (val);
 		*out_storage = val;
 		result = FALSE;
 	} else {
@@ -1457,8 +1460,11 @@ mono_image_storage_tryaddref (const char *key, MonoImageStorage **found)
 	gboolean result = FALSE;
 	mono_images_storage_lock ();
 	MonoImageStorage *val = (MonoImageStorage *)g_hash_table_lookup (images_storage_hash, key);
+	if (val && !mono_refcount_tryinc (val)) {
+		// We raced against a mono_image_storage_dtor in progress.
+		val = NULL;
+	}
 	if (val) {
-		mono_refcount_inc (val);
 		*found = val;
 		result = TRUE;
 	}
diff --git a/mono/metadata/metadata.c b/mono/metadata/metadata.c
index b2987016ef2..81217077470 100644
--- a/mono/metadata/metadata.c
+++ b/mono/metadata/metadata.c
@@ -2559,7 +2559,13 @@ retry:
 		return signature_in_image (type->data.method, image);
 	case MONO_TYPE_VAR:
 	case MONO_TYPE_MVAR:
-		return image == mono_get_image_for_generic_param (type->data.generic_param);
+		if (image == mono_get_image_for_generic_param (type->data.generic_param))
+			return TRUE;
+		else if (type->data.generic_param->gshared_constraint) {
+			type = type->data.generic_param->gshared_constraint;
+			goto retry;
+		}
+		return FALSE;
 	default:
 		/* At this point, we should've avoided all potential allocations in mono_class_from_mono_type_internal () */
 		return image == m_class_get_image (mono_class_from_mono_type_internal (type));
@@ -3037,13 +3043,16 @@ retry:
 		type = m_class_get_byval_arg (type->data.array->eklass);
 		goto retry;
 	case MONO_TYPE_FNPTR:
-		//return signature_in_image (type->data.method, image);
-		g_assert_not_reached ();
+		collect_signature_images (type->data.method, data);
+		break;
 	case MONO_TYPE_VAR:
 	case MONO_TYPE_MVAR:
 	{
 		MonoImage *image = mono_get_image_for_generic_param (type->data.generic_param);
 		add_image (image, data);
+		type = type->data.generic_param->gshared_constraint;
+		if (type)
+			goto retry;
 		break;
 	}
 	case MONO_TYPE_CLASS: