[loader] LoadFrom of problematic images should reprobe (correctly)

This reverts commit 569bd3b
which itself reverted efe1fde.

Additionally, cherrypick 60f06e6: refonly load
of problematic image is allowed.

mono/metadata/assembly.c

@@ -354,6 +354,11 @@ static MonoAssembly*
 mono_assembly_invoke_search_hook_internal (MonoAssemblyName *aname, MonoAssembly *requesting, gboolean refonly, gboolean postload);
 static MonoAssembly*
 mono_assembly_load_full_internal (MonoAssemblyName *aname, MonoAssembly *requesting, const char *basedir, MonoImageOpenStatus *status, gboolean refonly);
+static MonoAssembly*
+chain_redirections_loadfrom (MonoImage *image, MonoImageOpenStatus *status);
+static MonoAssembly*
+mono_problematic_image_reprobe (MonoImage *image, MonoImageOpenStatus *status);
+
 static MonoBoolean
 mono_assembly_is_in_gac (const gchar *filanem);
 
@@ -1982,6 +1987,23 @@ mono_assembly_open_predicate (const char *filename, gboolean refonly,
 		return NULL;
 	}
 
+	if (load_from_context) {
+		MonoAssembly *redirected_asm = NULL;
+		MonoImageOpenStatus new_status = MONO_IMAGE_OK;
+		if ((redirected_asm = chain_redirections_loadfrom (image, &new_status))) {
+			mono_image_close (image);
+			image = redirected_asm->image;
+			mono_image_addref (image); /* so that mono_image_close, below, has something to do */
+			/* fall thru to if (image->assembly) below */
+		} else if (new_status != MONO_IMAGE_OK) {
+			*status = new_status;
+			mono_image_close (image);
+			g_free (fname);
+			return NULL;
+		}
+	}
+
+
 	if (image->assembly) {
 		/* We want to return the MonoAssembly that's already loaded,
 		 * but if we're using the strict assembly loader, we also need
@@ -2165,6 +2187,85 @@ mono_assembly_has_reference_assembly_attribute (MonoAssembly *assembly, MonoErro
 	return iter_data.has_attr;
 }
 
+/**
+ * chain_redirections_loadfrom:
+ * \param image a MonoImage that we wanted to load using LoadFrom context
+ * \param status set if there was an error opening the redirected image
+ *
+ * Check if we need to open a different image instead of the given one for some reason.
+ * Returns NULL and sets status to \c MONO_IMAGE_OK if the given image was good.
+ *
+ * Otherwise returns the assembly that we opened instead or sets status if
+ * there was a problem opening the redirected image.
+ *
+ */
+MonoAssembly*
+chain_redirections_loadfrom (MonoImage *image, MonoImageOpenStatus *out_status)
+{
+	MonoImageOpenStatus status;
+	MonoAssembly *redirected = NULL;
+
+	redirected = mono_problematic_image_reprobe (image, &status);
+	if (redirected || status != MONO_IMAGE_OK) {
+		*out_status = status;
+		return redirected;
+	}
+
+	*out_status = MONO_IMAGE_OK;
+	return NULL;
+}
+
+/**
+ * mono_problematic_image_reprobe:
+ * \param image A MonoImage
+ * \param status set on error
+ *
+ * If the given image is problematic for mono (see image.c), then try to load
+ * by assembly name in the default context (which should succeed with Mono's
+ * own implementations of those assemblies).
+ *
+ * Returns NULL and sets status to MONO_IMAGE_OK if no redirect is needed.
+ *
+ * Otherwise returns the assembly we were redirected to, or NULL and sets a
+ * non-ok status on failure.
+ *
+ * IMPORTANT NOTE: Don't call this if \c image was found by probing the search
+ * path, you will end up in a loop and a stack overflow.
+ */
+MonoAssembly*
+mono_problematic_image_reprobe (MonoImage *image, MonoImageOpenStatus *status)
+{
+	if (G_LIKELY (!mono_is_problematic_image (image))) {
+		*status = MONO_IMAGE_OK;
+		return NULL;
+	}
+	MonoAssemblyName probed_aname;
+	if (!mono_assembly_fill_assembly_name_full (image, &probed_aname, TRUE)) {
+		*status = MONO_IMAGE_IMAGE_INVALID;
+		return NULL;
+	}
+	if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY)) {
+		char *probed_fullname = mono_stringify_assembly_name (&probed_aname);
+		mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Requested to load from problematic image %s, probing instead for assembly with name %s", image->name, probed_fullname);
+		g_free (probed_fullname);
+	}
+	const char *new_basedir = NULL;
+	const gboolean new_refonly = FALSE;
+	MonoAssembly *new_requesting = NULL;
+	MonoImageOpenStatus new_status = MONO_IMAGE_OK;
+	// Note: this interacts with mono_image_open_a_lot (). If the path from
+	// which we tried to load the problematic image is among the probing
+	// paths, the MonoImage will be in the hash of loaded images and we
+	// would just get it back again here, except for the code there that
+	// mitigates the situation.  Instead
+	MonoAssembly *result_ass = mono_assembly_load_full_internal (&probed_aname, new_requesting, new_basedir, &new_status, new_refonly);
+
+	if (! (result_ass && new_status == MONO_IMAGE_OK)) {
+		*status = new_status;
+	}
+	mono_assembly_name_free (&probed_aname);
+	return result_ass;
+}
 /**
  * mono_assembly_open:
  * \param filename Opens the assembly pointed out by this name

mono/metadata/image-internals.h

@@ -20,4 +20,7 @@ mono_image_load_module_checked (MonoImage *image, int idx, MonoError *error);
 MonoImage *
 mono_image_open_a_lot (const char *fname, MonoImageOpenStatus *status, gboolean refonly, gboolean load_from_context);
 
+gboolean
+mono_is_problematic_image (MonoImage *image);
+
 #endif /* __MONO_METADATA_IMAGE_INTERNALS_H__ */

mono/metadata/image.c

@@ -1285,8 +1285,8 @@ hash_guid (const char *str)
 	return h;
 }
 
-static gboolean
-is_problematic_image (MonoImage *image)
+gboolean
+mono_is_problematic_image (MonoImage *image)
 {
 	int h = hash_guid (image->guid);
 
@@ -1359,7 +1359,7 @@ do_mono_image_load (MonoImage *image, MonoImageOpenStatus *status,
 	if (!mono_image_load_cli_data (image))
 		goto invalid_image;
 
-	if (!image->ref_only && is_problematic_image (image)) {
+	if (!image->ref_only && mono_is_problematic_image (image)) {
 		if (image->load_from_context) {
 			mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Loading problematic image %s", image->name);
 		} else {
@@ -1681,6 +1681,18 @@ mono_image_open_a_lot (const char *fname, MonoImageOpenStatus *status, gboolean
 		mono_images_lock ();
 		image = g_hash_table_lookup (loaded_images, absfname);
 		if (image) { // Image already loaded
+			if (!load_from_context && mono_is_problematic_image (image)) {
+				// If we previously loaded a problematic image, don't
+				// return it if we're not in LoadFrom context.
+				//
+				// Note: this has an interaction with
+				//  mono_problematic_image_reprobe - at that point we
+				//  have a problematic image opened, but we don't want
+				//  to see it again when we go searching for an image
+				//  to load.
+				mono_images_unlock ();
+				return NULL;
+			}
 			g_assert (image->is_module_handle);
 			if (image->has_entry_point && image->ref_count == 0) {
 				/* Increment reference count on images loaded outside of the runtime. */
@@ -1751,6 +1763,18 @@ mono_image_open_a_lot (const char *fname, MonoImageOpenStatus *status, gboolean
 	g_free (absfname);
 
 	if (image) { // Image already loaded
+		if (!refonly && !load_from_context && mono_is_problematic_image (image)) {
+			// If we previously loaded a problematic image, don't
+			// return it if we're not in LoadFrom context.
+			//
+			// Note: this has an interaction with
+			//  mono_problematic_image_reprobe - at that point we
+			//  have a problematic image opened, but we don't want
+			//  to see it again when we go searching for an image
+			//  to load.
+			mono_images_unlock ();
+			return NULL;
+		}
 		mono_image_addref (image);
 		mono_images_unlock ();
 		return image;