[loader] LoadFrom of problematic images should reprobe

The "problematic images" are Windows-specific assemblies that are often
included with applications that do not work on Mono, but for which Mono has its
own implementation.

Previously (9ec8ec5) we changed mono to deny
loading problematic images using Assembly.Load or when one assembly references
another and we find a problematic image using the usual assembly search.

For Assembly.LoadFrom, we used to emit a warning and then load the assembly anyway.

In this commit, we change the behavior: If Assembly.LoadFrom tries to open a
problematic image, we will instead probe for an assembly of that name in the
default context.  That should eventually end up opening Mono's version of that
assembly.

Example:
  Suppose a problematic System.Runtime.InteropServices.RuntimeInformation.dll
  is in the application bin path: /home/user/myapp/bin

  1. User code does (for whatever reason)
  Assembly.LoadFrom ("/home/user/myapp/bin/System.Runtime.InteropServices.RuntimeInformation.dll")

  2. We find the image and try to open it; add it to the images absfpath to
  image hash; note that there are no binding redirects; but that it is a
  problematic image.  We extract the assembly
  name ("System.Runtime.InteropServices.RuntimeInformation", version x.y.z.w
  and public key token 123456789a) from the problematic image.

  3. We probe for "System.Runtime.InteropServices.RuntimeInformation version
  x.y.z.w public key token 123456789a" in the default context.

  4. We find
  /home/user/myapp/bin/System.Runtime.InteropServices.RuntimeInformation.dll in
  the application bin path; we try to open it, we see it's already in the
  images hash, but it's a problematic image,  and we're not in a loadfrom
  context (we re-probed with default!) so we keep probing

  5. We find
  <prefix>/lib/mono/4.5/Facaded/System.Runtime.InteropServices.RuntimeInformation.dll,
  it's not a problematic image, so we open that and return it to (3)

  6. We return to (2) and close the problematic image and return the good one
  instead.

  7. Assembly.LoadFrom returns with the good image.

Fixes mono#8726

mono/metadata/assembly.c

@@ -358,6 +358,10 @@ static MonoAssembly*
 mono_assembly_load_full_gac_base_default (MonoAssemblyName *aname, const char *basedir, MonoAssemblyContextKind asmctx, MonoImageOpenStatus *status);
 static MonoAssembly*
 mono_assembly_load_full_nodomain (MonoAssemblyName *aname, MonoAssemblyContextKind asmctx, MonoImageOpenStatus *status);
+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);
@@ -2157,7 +2161,7 @@ mono_assembly_open_predicate (const char *filename, MonoAssemblyContextKind asmc
 	if (asmctx == MONO_ASMCTX_LOADFROM || asmctx == MONO_ASMCTX_INDIVIDUAL) {
 		MonoAssembly *redirected_asm = NULL;
 		MonoImageOpenStatus new_status = MONO_IMAGE_OK;
-		if ((redirected_asm = mono_assembly_binding_applies_to_image (image, &new_status))) {
+		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 */
@@ -2353,6 +2357,40 @@ 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_assembly_binding_applies_to_image (image, &status);
+	if (redirected || status != MONO_IMAGE_OK) {
+		*out_status = status;
+		return redirected;
+	}
+
+	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_assembly_binding_applies_to_image:
  * \param image The image whose assembly name we should check
@@ -2410,6 +2448,57 @@ mono_assembly_binding_applies_to_image (MonoImage* image, MonoImageOpenStatus *s
 	return result_ass;
 }
 
+/**
+ * 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;
+	MonoAssemblyContextKind new_asmctx = MONO_ASMCTX_DEFAULT;
+	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_asmctx, &new_status);
+
+	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

@@ -1286,8 +1286,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);
 
@@ -1360,7 +1360,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 (!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;