[loader] Add assembly name matching on netcore to fix DefaultContext …

…test (mono/mono#18272) * [loader] Rename assembly name check strictness flag * [loader] Add predicate for assembly name matching on netcore * [loader] Enable LoadInDefaultContext test * Make mono_assembly_names_equal external, change flags in domain search * Move ifdef inside mono_loader_get_strict_assembly_name_check * Fix pedump * No reason to make this external * Feedback Commit migrated from mono/mono@33ca3d3
realmar · Jan 2, 2020 · 8cf5a2d · 8cf5a2d
1 parent ea01f5e
commit 8cf5a2d
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 47 deletions.
diff --git a/src/mono/mono/dis/main.c b/src/mono/mono/dis/main.c
@@ -1951,7 +1951,7 @@ monodis_assembly_search_hook (MonoAssemblyLoadContext *alc, MonoAssembly *reques
 
        for (tmp = loaded_assemblies; tmp; tmp = tmp->next) {
                MonoAssembly *ass = (MonoAssembly *)tmp->data;
-               if (mono_assembly_names_equal (aname, &ass->aname))
+               if (mono_assembly_check_name_match (aname, &ass->aname))
 		       return ass;
        }
        return NULL;

diff --git a/src/mono/mono/metadata/appdomain.c b/src/mono/mono/metadata/appdomain.c
@@ -2459,12 +2459,10 @@ mono_domain_assembly_preload (MonoAssemblyLoadContext *alc,
 
 	MonoAssemblyCandidatePredicate predicate = NULL;
 	void* predicate_ud = NULL;
-#if !defined(DISABLE_DESKTOP_LOADER)
-	if (G_LIKELY (mono_loader_get_strict_strong_names ())) {
+	if (mono_loader_get_strict_assembly_name_check ()) {
 		predicate = &mono_assembly_candidate_predicate_sn_same_name;
 		predicate_ud = aname;
 	}
-#endif
 	MonoAssemblyOpenRequest req;
 	mono_assembly_request_prepare_open (&req, refonly ? MONO_ASMCTX_REFONLY : MONO_ASMCTX_DEFAULT, alc);
 	req.request.predicate = predicate;
@@ -2522,12 +2520,10 @@ mono_assembly_load_from_assemblies_path (gchar **assemblies_path, MonoAssemblyNa
 {
 	MonoAssemblyCandidatePredicate predicate = NULL;
 	void* predicate_ud = NULL;
-#if !defined(DISABLE_DESKTOP_LOADER)
-	if (G_LIKELY (mono_loader_get_strict_strong_names ())) {
+	if (mono_loader_get_strict_assembly_name_check ()) {
 		predicate = &mono_assembly_candidate_predicate_sn_same_name;
 		predicate_ud = aname;
 	}
-#endif
 	MonoAssemblyOpenRequest req;
 	mono_assembly_request_prepare_open (&req, asmctx, mono_domain_default_alc (mono_domain_get ()));
 	req.request.predicate = predicate;
@@ -2553,19 +2549,15 @@ mono_domain_assembly_search (MonoAssemblyLoadContext *alc, MonoAssembly *request
 	g_assert (aname != NULL);
 	GSList *tmp;
 	MonoAssembly *ass;
-	const gboolean strong_name = aname->public_key_token[0] != 0;
-	/* If it's not a strong name, any version that has the right simple
-	 * name is good enough to satisfy the request.  .NET Framework also
-	 * ignores case differences in this case. */
-	const MonoAssemblyNameEqFlags eq_flags = (MonoAssemblyNameEqFlags)(strong_name ? MONO_ANAME_EQ_IGNORE_CASE :
-		(MONO_ANAME_EQ_IGNORE_PUBKEY | MONO_ANAME_EQ_IGNORE_VERSION | MONO_ANAME_EQ_IGNORE_CASE));
 
 #ifdef ENABLE_NETCORE
+	const MonoAssemblyNameEqFlags eq_flags = MONO_ANAME_EQ_IGNORE_PUBKEY | MONO_ANAME_EQ_IGNORE_VERSION | MONO_ANAME_EQ_IGNORE_CASE;
+
 	mono_alc_assemblies_lock (alc);
 	for (tmp = alc->loaded_assemblies; tmp; tmp = tmp->next) {
 		ass = (MonoAssembly *)tmp->data;
 		g_assert (ass != NULL);
-		// TODO: Can dynamic assemblies match here for netcore? Also, this ignores case while exact_sn_match does not.
+		// FIXME: Can dynamic assemblies match here for netcore?
 		if (assembly_is_dynamic (ass) || !mono_assembly_names_equal_flags (aname, &ass->aname, eq_flags))
 			continue;
 
@@ -2575,6 +2567,14 @@ mono_domain_assembly_search (MonoAssemblyLoadContext *alc, MonoAssembly *request
 	mono_alc_assemblies_unlock (alc);
 #else
 	MonoDomain *domain = mono_alc_domain (alc);
+
+	const gboolean strong_name = aname->public_key_token[0] != 0;
+	/* If it's not a strong name, any version that has the right simple
+	 * name is good enough to satisfy the request.  .NET Framework also
+	 * ignores case differences in this case. */
+	const MonoAssemblyNameEqFlags eq_flags = (MonoAssemblyNameEqFlags)(strong_name ? MONO_ANAME_EQ_IGNORE_CASE :
+		(MONO_ANAME_EQ_IGNORE_PUBKEY | MONO_ANAME_EQ_IGNORE_VERSION | MONO_ANAME_EQ_IGNORE_CASE));
+
 	mono_domain_assemblies_lock (domain);
 	for (tmp = domain->domain_assemblies; tmp; tmp = tmp->next) {
 		ass = (MonoAssembly *)tmp->data;
@@ -2633,6 +2633,15 @@ ves_icall_System_Reflection_Assembly_InternalLoad (MonoStringHandle name_handle,
 	if (!parsed)
 		goto fail;
 
+	MonoAssemblyCandidatePredicate predicate = NULL;
+	void* predicate_ud = NULL;
+	if (mono_loader_get_strict_assembly_name_check ()) {
+		predicate = &mono_assembly_candidate_predicate_sn_same_name;
+		predicate_ud = &aname;
+	}
+	req.request.predicate = predicate;
+	req.request.predicate_ud = predicate_ud;
+
 	ass = mono_assembly_request_byname (&aname, &req, &status);
 	if (!ass)
 		goto fail;

diff --git a/src/mono/mono/metadata/assembly-internals.h b/src/mono/mono/metadata/assembly-internals.h
@@ -134,6 +134,9 @@ MonoAssembly*          mono_assembly_request_byname (MonoAssemblyName *aname,
 gboolean
 mono_assembly_candidate_predicate_sn_same_name (MonoAssembly *candidate, gpointer wanted_name);
 
+gboolean
+mono_assembly_check_name_match (MonoAssemblyName *wanted_name, MonoAssemblyName *candidate_name);
+
 MonoAssembly*
 mono_assembly_binding_applies_to_image (MonoAssemblyLoadContext *alc, MonoImage* image, MonoImageOpenStatus *status);
 

diff --git a/src/mono/mono/metadata/assembly.c b/src/mono/mono/metadata/assembly.c
@@ -394,8 +394,6 @@ prevent_reference_assembly_from_running (MonoAssembly* candidate, gboolean refon
 
 /* Assembly name matching */
 static gboolean
-exact_sn_match (MonoAssemblyName *wanted_name, MonoAssemblyName *candidate_name);
-static gboolean
 framework_assembly_sn_match (MonoAssemblyName *wanted_name, MonoAssemblyName *candidate_name);
 
 static const char *
@@ -2592,7 +2590,7 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest
 		 * predicate.  It could be that we previously loaded a
 		 * different version that happens to have the filename that
 		 * we're currently probing. */
-		if (mono_loader_get_strict_strong_names () &&
+		if (mono_loader_get_strict_assembly_name_check () &&
 		    load_req.predicate && !load_req.predicate (image->assembly, load_req.predicate_ud)) {
 			mono_image_close (image);
 			g_free (fname);
@@ -4526,7 +4524,9 @@ mono_assembly_candidate_predicate_sn_same_name (MonoAssembly *candidate, gpointe
 		g_free (s);
 	}
 
-
+#ifdef ENABLE_NETCORE
+	return mono_assembly_check_name_match (wanted_name, candidate_name);
+#else
 	/* Wanted name has no token, not strongly named: always matches. */
 	if (0 == wanted_name->public_key_token [0]) {
 		mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Predicate: wanted has no token, returning TRUE");
@@ -4539,19 +4539,20 @@ mono_assembly_candidate_predicate_sn_same_name (MonoAssembly *candidate, gpointe
 		return FALSE;
 	}
 
-	return exact_sn_match (wanted_name, candidate_name) ||
+	return mono_assembly_check_name_match (wanted_name, candidate_name) ||
 		framework_assembly_sn_match (wanted_name, candidate_name);
+#endif
 }
 
 gboolean
-exact_sn_match (MonoAssemblyName *wanted_name, MonoAssemblyName *candidate_name)
+mono_assembly_check_name_match (MonoAssemblyName *wanted_name, MonoAssemblyName *candidate_name)
 {
 #if ENABLE_NETCORE
 	gboolean result = mono_assembly_names_equal_flags (wanted_name, candidate_name, MONO_ANAME_EQ_IGNORE_VERSION | MONO_ANAME_EQ_IGNORE_PUBKEY);
 	if (result && assembly_names_compare_versions (wanted_name, candidate_name, -1) > 0)
 		result = FALSE;
 #else
-	gboolean result = mono_assembly_names_equal (wanted_name, candidate_name);
+	gboolean result = mono_assembly_names_equal_flags (wanted_name, candidate_name, MONO_ANAME_EQ_NONE);
 #endif
 
 	mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Predicate: candidate and wanted names %s",
@@ -4664,12 +4665,10 @@ mono_assembly_load_full_gac_base_default (MonoAssemblyName *aname,
 
 	MonoAssemblyCandidatePredicate predicate = NULL;
 	void* predicate_ud = NULL;
-#if !defined(DISABLE_DESKTOP_LOADER)
-	if (G_LIKELY (mono_loader_get_strict_strong_names ())) {
+	if (mono_loader_get_strict_assembly_name_check ()) {
 		predicate = &mono_assembly_candidate_predicate_sn_same_name;
 		predicate_ud = aname;
 	}
-#endif
 
 	MonoAssemblyOpenRequest req;
 	mono_assembly_request_prepare_open (&req, asmctx, alc);

diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h
@@ -1143,10 +1143,10 @@ void
 mono_ginst_get_desc (GString *str, MonoGenericInst *ginst);
 
 void
-mono_loader_set_strict_strong_names (gboolean enabled);
+mono_loader_set_strict_assembly_name_check (gboolean enabled);
 
 gboolean
-mono_loader_get_strict_strong_names (void);
+mono_loader_get_strict_assembly_name_check (void);
 
 gboolean
 mono_type_in_image (MonoType *type, MonoImage *image);

diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c
@@ -516,12 +516,12 @@ static const gint16 tableidx [] = {
 #undef TABLEDEF
 };
 
-/* If TRUE (but also see DISABLE_STICT_STRONG_NAMES #define), Mono will check
+/* On legacy, if TRUE (but also see DISABLE_DESKTOP_LOADER #define), Mono will check
  * that the public key token, culture and version of a candidate assembly matches
- * the requested strong name.  If FALSE, as long as the name matches, the candidate
- * will be allowed.
+ * the requested strong name. On netcore, it will check the culture and version.
+ * If FALSE, as long as the name matches, the candidate will be allowed.
  */
-static gboolean check_strong_names_strictly = FALSE;
+static gboolean check_assembly_names_strictly = FALSE;
 
 // Amount initially reserved in each imageset's mempool.
 // FIXME: This number is arbitrary, a more practical number should be found
@@ -7563,15 +7563,19 @@ mono_find_image_set_owner (void *ptr)
 }
 
 void
-mono_loader_set_strict_strong_names (gboolean enabled)
+mono_loader_set_strict_assembly_name_check (gboolean enabled)
 {
-	check_strong_names_strictly = enabled;
+	check_assembly_names_strictly = enabled;
 }
 
 gboolean
-mono_loader_get_strict_strong_names (void)
+mono_loader_get_strict_assembly_name_check (void)
 {
-	return check_strong_names_strictly;
+#if !defined(DISABLE_DESKTOP_LOADER) || defined(ENABLE_NETCORE)
+	return check_assembly_names_strictly;
+#else
+	return FALSE;
+#endif
 }
 
 

diff --git a/src/mono/mono/metadata/reflection.c b/src/mono/mono/metadata/reflection.c
@@ -1945,7 +1945,7 @@ _mono_reflection_get_type_from_info (MonoAssemblyLoadContext *alc, MonoTypeNameP
 
 	if (info->assembly.name) {
 		MonoAssembly *assembly = mono_assembly_loaded_internal (alc, &info->assembly, FALSE);
-		if (!assembly && image && image->assembly && mono_assembly_names_equal (&info->assembly, &image->assembly->aname))
+		if (!assembly && image && image->assembly && mono_assembly_check_name_match (&info->assembly, &image->assembly->aname))
 			/* 
 			 * This could happen in the AOT compiler case when the search hook is not
 			 * installed.

diff --git a/src/mono/mono/mini/driver.c b/src/mono/mono/mini/driver.c
@@ -2362,9 +2362,9 @@ mono_main (int argc, char* argv[])
 		} else if (strncmp (argv [i], "--assembly-loader=", strlen("--assembly-loader=")) == 0) {
 			gchar *arg = argv [i] + strlen ("--assembly-loader=");
 			if (strcmp (arg, "strict") == 0)
-				mono_loader_set_strict_strong_names (TRUE);
+				mono_loader_set_strict_assembly_name_check (TRUE);
 			else if (strcmp (arg, "legacy") == 0)
-				mono_loader_set_strict_strong_names (FALSE);
+				mono_loader_set_strict_assembly_name_check (FALSE);
 			else
 				fprintf (stderr, "Warning: unknown argument to --assembly-loader. Should be \"strict\" or \"legacy\"\n");
 		} else if (strncmp (argv [i], MONO_HANDLERS_ARGUMENT, MONO_HANDLERS_ARGUMENT_LEN) == 0) {

diff --git a/src/mono/mono/mini/main-core.c b/src/mono/mono/mini/main-core.c
@@ -232,9 +232,9 @@ int STDAPICALLTYPE coreclr_initialize (const char* exePath, const char* appDomai
 
 	/*
 	 * Don't use Mono's legacy assembly name matching behavior - respect
-	 * the requested version and public key token.
+	 * the requested version and culture.
 	 */
-	mono_loader_set_strict_strong_names (TRUE);
+	mono_loader_set_strict_assembly_name_check (TRUE);
 
 	return 0;
 }

diff --git a/src/mono/netcore/CoreFX.issues.rsp b/src/mono/netcore/CoreFX.issues.rsp
@@ -678,14 +678,6 @@
 # Test broken on Mono, added in https://github.com/dotnet/corefx/pull/40581
 -nomethod System.Reflection.Tests.MetadataLoadContextTests.RelocatableAssembly
 
-####################################################################
-##  System.Runtime.Loader.DefaultContext.Tests
-####################################################################
-
-# Expected FileNotFoundException, got none
-# https://github.com/mono/mono/issues/15195
--nomethod System.Runtime.Loader.Tests.DefaultLoadContextTests.LoadInDefaultContext
-
 ####################################################################
 ##  System.Runtime.Loader.RefEmitLoadContext.Tests
 ####################################################################