From 8cf5a2d82a0d35ca73ca1d5ee8a03a44d71eb017 Mon Sep 17 00:00:00 2001 From: Ryan Lucia Date: Thu, 2 Jan 2020 15:21:45 -0500 Subject: [PATCH] [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 https://github.com/mono/mono/commit/33ca3d33e1994cabf9f9b19034624a3495bbc9cb --- src/mono/mono/dis/main.c | 2 +- src/mono/mono/metadata/appdomain.c | 35 ++++++++++++++++++----------- src/mono/mono/metadata/assembly-internals.h | 3 +++ src/mono/mono/metadata/assembly.c | 19 ++++++++-------- src/mono/mono/metadata/metadata-internals.h | 4 ++-- src/mono/mono/metadata/metadata.c | 20 ++++++++++------- src/mono/mono/metadata/reflection.c | 2 +- src/mono/mono/mini/driver.c | 4 ++-- src/mono/mono/mini/main-core.c | 4 ++-- src/mono/netcore/CoreFX.issues.rsp | 8 ------- 10 files changed, 54 insertions(+), 47 deletions(-) diff --git a/src/mono/mono/dis/main.c b/src/mono/mono/dis/main.c index 056da62..1f69815 100644 --- 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 index 35363c8..4914489 100644 --- 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 index 7d9a0da..55a23c9 100644 --- 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 index d9e0238..eac2b7c 100644 --- 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 index de3fcaa..36cf51d 100644 --- 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 index ecc4414..6c2ba24 100644 --- 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 index 69e39b9..4972e49 100644 --- 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 index 9b5c3b9..0d6873d 100644 --- 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 index 3e3a9d7..0b303a3 100644 --- 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 index 0fd5a67..ca3110c 100644 --- a/src/mono/netcore/CoreFX.issues.rsp +++ b/src/mono/netcore/CoreFX.issues.rsp @@ -679,14 +679,6 @@ -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 #################################################################### -- 2.7.4