From 85296ef1be7bb34c1c92953ca66a026e5dcd0771 Mon Sep 17 00:00:00 2001 From: Ryan Lucia Date: Tue, 6 Aug 2019 06:16:15 -0400 Subject: [PATCH] [netcore] Make the load hook ALC-aware (#16012) The meat of this PR is what the title suggests, and with this commit all the basic ALC functionality should be working properly. Probably worth checking if this makes any new tests pass. Additionally, this PR includes a second commit that switches over the search hook to use loaded_assemblies from the ALCs. This may break some existing tests due to remaining unimplemented ALC behavior, notably the native loading mechanism needed for Openssl in some of the cryptography tests. My inclination is to include the commit regardless and just disable the failing tests so that any future ALC work can watch for regressions in the loader, but I would appreciate input from steveisok on how important those tests are. I've not disabled anything yet in order to see exactly what new failures are introduced. --- mono/dis/main.c | 4 +-- mono/metadata/appdomain.c | 15 ++++----- mono/metadata/assembly-internals.h | 12 +++++-- mono/metadata/assembly.c | 65 ++++++++++++++++++++++++++++++-------- mono/metadata/assembly.h | 6 ++-- mono/metadata/loader-internals.h | 17 ++++++---- mono/metadata/mono-debug.c | 6 ++-- mono/metadata/sre.c | 3 +- mono/mini/aot-runtime.c | 8 ++--- tools/pedump/pedump.c | 8 ++--- 10 files changed, 98 insertions(+), 46 deletions(-) diff --git a/mono/dis/main.c b/mono/dis/main.c index 846e7e8788a..9d2143ca9a6 100644 --- a/mono/dis/main.c +++ b/mono/dis/main.c @@ -1936,7 +1936,7 @@ monodis_preload (MonoAssemblyName *aname, static GList *loaded_assemblies = NULL; static void -monodis_assembly_load_hook (MonoAssembly *assembly, gpointer user_data) +monodis_assembly_load_hook (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error) { loaded_assemblies = g_list_prepend (loaded_assemblies, assembly); } @@ -2046,7 +2046,7 @@ main (int argc, char *argv []) #endif mono_thread_info_runtime_init (&ticallbacks); - mono_install_assembly_load_hook (monodis_assembly_load_hook, NULL); + mono_install_assembly_load_hook_v2 (monodis_assembly_load_hook, NULL); mono_install_assembly_search_hook_v2 (monodis_assembly_search_hook, NULL, FALSE, FALSE); /* diff --git a/mono/metadata/appdomain.c b/mono/metadata/appdomain.c index 699f6473df6..f8eba91e8e0 100644 --- a/mono/metadata/appdomain.c +++ b/mono/metadata/appdomain.c @@ -122,7 +122,7 @@ mono_domain_assembly_search (MonoAssemblyLoadContext *alc, MonoAssembly *request static void -mono_domain_fire_assembly_load (MonoAssembly *assembly, gpointer user_data); +mono_domain_fire_assembly_load (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error_out); static gboolean mono_domain_asmctx_from_path (const char *fname, MonoAssembly *requesting_assembly, gpointer user_data, MonoAssemblyContextKind *out_asmctx); @@ -313,7 +313,7 @@ mono_runtime_init_checked (MonoDomain *domain, MonoThreadStartCB start_cb, MonoT mono_install_assembly_search_hook_v2 (mono_domain_assembly_search, GUINT_TO_POINTER (TRUE), TRUE, FALSE); mono_install_assembly_search_hook_v2 (mono_domain_assembly_postload_search, GUINT_TO_POINTER (FALSE), FALSE, TRUE); mono_install_assembly_search_hook_v2 (mono_domain_assembly_postload_search, GUINT_TO_POINTER (TRUE), TRUE, TRUE); - mono_install_assembly_load_hook (mono_domain_fire_assembly_load, NULL); + mono_install_assembly_load_hook_v2 (mono_domain_fire_assembly_load, NULL); mono_install_assembly_asmctx_from_path_hook (mono_domain_asmctx_from_path, NULL); mono_thread_init (start_cb, attach_cb); @@ -365,7 +365,8 @@ mono_runtime_init_checked (MonoDomain *domain, MonoThreadStartCB start_cb, MonoT mono_locks_tracer_init (); /* mscorlib is loaded before we install the load hook */ - mono_domain_fire_assembly_load (mono_defaults.corlib->assembly, NULL); + mono_domain_fire_assembly_load (mono_domain_default_alc (domain), mono_defaults.corlib->assembly, NULL, error); + goto_if_nok (error, exit); exit: HANDLE_FUNCTION_RETURN (); @@ -1495,14 +1496,13 @@ add_assembly_to_alc (MonoAssemblyLoadContext *alc, MonoAssembly *ass) #endif static void -mono_domain_fire_assembly_load (MonoAssembly *assembly, gpointer user_data) +mono_domain_fire_assembly_load (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error_out) { HANDLE_FUNCTION_ENTER (); static MonoClassField *assembly_load_field; static MonoMethod *assembly_load_method; ERROR_DECL (error); - MonoDomain *domain = mono_domain_get (); - MonoAssemblyLoadContext *alc = mono_domain_ambient_alc (domain); // FIXME: pass alc via mono_assembly_invoke_load_hook + MonoDomain *domain = mono_alc_domain (alc); MonoClass *klass; MonoObjectHandle appdomain; @@ -2391,8 +2391,7 @@ mono_domain_assembly_search (MonoAssemblyLoadContext *alc, MonoAssembly *request 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)); -// TODO: this is currently broken due to the lack of proper ALC resolution logic and the load hook not using the correct ALC -#if 0 //def ENABLE_NETCORE +#ifdef ENABLE_NETCORE mono_alc_assemblies_lock (alc); for (tmp = alc->loaded_assemblies; tmp; tmp = tmp->next) { ass = (MonoAssembly *)tmp->data; diff --git a/mono/metadata/assembly-internals.h b/mono/metadata/assembly-internals.h index 75702c83312..5aa7f3d533c 100644 --- a/mono/metadata/assembly-internals.h +++ b/mono/metadata/assembly-internals.h @@ -56,12 +56,20 @@ void mono_install_assembly_asmctx_from_path_hook (MonoAssemblyAsmCtxFromPathFunc typedef MonoAssembly * (*MonoAssemblyPreLoadFuncV2) (MonoAssemblyLoadContext *alc, MonoAssemblyName *aname, char **assemblies_path, gboolean refonly, gpointer user_data, MonoError *error); -void mono_install_assembly_preload_hook_v2 (MonoAssemblyPreLoadFuncV2 func, void *user_data, gboolean refonly); +void mono_install_assembly_preload_hook_v2 (MonoAssemblyPreLoadFuncV2 func, gpointer user_data, gboolean refonly); typedef MonoAssembly * (*MonoAssemblySearchFuncV2) (MonoAssemblyLoadContext *alc, MonoAssembly *requesting, MonoAssemblyName *aname, gboolean refonly, gboolean postload, gpointer user_data, MonoError *error); void -mono_install_assembly_search_hook_v2 (MonoAssemblySearchFuncV2 func, void *user_data, gboolean refonly, gboolean postload); +mono_install_assembly_search_hook_v2 (MonoAssemblySearchFuncV2 func, gpointer user_data, gboolean refonly, gboolean postload); + +typedef void (*MonoAssemblyLoadFuncV2) (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error); + +void +mono_install_assembly_load_hook_v2 (MonoAssemblyLoadFuncV2 func, gpointer user_data); + +void +mono_assembly_invoke_load_hook_internal (MonoAssemblyLoadContext *alc, MonoAssembly *ass); /* If predicate returns true assembly should be loaded, if false ignore it. */ typedef gboolean (*MonoAssemblyCandidatePredicate)(MonoAssembly *, gpointer); diff --git a/mono/metadata/assembly.c b/mono/metadata/assembly.c index 043019526d2..aadcdeb7cea 100644 --- a/mono/metadata/assembly.c +++ b/mono/metadata/assembly.c @@ -1835,42 +1835,79 @@ mono_assembly_load_references (MonoImage *image, MonoImageOpenStatus *status) typedef struct AssemblyLoadHook AssemblyLoadHook; struct AssemblyLoadHook { AssemblyLoadHook *next; - MonoAssemblyLoadFunc func; + union { + MonoAssemblyLoadFunc v1; + MonoAssemblyLoadFuncV2 v2; + } func; + int version; gpointer user_data; }; static AssemblyLoadHook *assembly_load_hook = NULL; -/** - * mono_assembly_invoke_load_hook: - */ void -mono_assembly_invoke_load_hook (MonoAssembly *ass) +mono_assembly_invoke_load_hook_internal (MonoAssemblyLoadContext *alc, MonoAssembly *ass) { AssemblyLoadHook *hook; for (hook = assembly_load_hook; hook; hook = hook->next) { - hook->func (ass, hook->user_data); + if (hook->version == 1) { + hook->func.v1 (ass, hook->user_data); + } else { + ERROR_DECL (hook_error); + g_assert (hook->version == 2); + hook->func.v2 (alc, ass, hook->user_data, hook_error); + mono_error_assert_ok (hook_error); /* FIXME: proper error handling */ + } } } /** - * mono_install_assembly_load_hook: + * mono_assembly_invoke_load_hook: */ void -mono_install_assembly_load_hook (MonoAssemblyLoadFunc func, gpointer user_data) +mono_assembly_invoke_load_hook (MonoAssembly *ass) +{ + mono_assembly_invoke_load_hook_internal (mono_domain_default_alc (mono_domain_get ()), ass); +} + +static void +mono_install_assembly_load_hook_v1 (MonoAssemblyLoadFunc func, gpointer user_data) { AssemblyLoadHook *hook; g_return_if_fail (func != NULL); hook = g_new0 (AssemblyLoadHook, 1); - hook->func = func; + hook->version = 1; + hook->func.v1 = func; + hook->user_data = user_data; + hook->next = assembly_load_hook; + assembly_load_hook = hook; +} + +void +mono_install_assembly_load_hook_v2 (MonoAssemblyLoadFuncV2 func, gpointer user_data) +{ + g_return_if_fail (func != NULL); + + AssemblyLoadHook *hook = g_new0 (AssemblyLoadHook, 1); + hook->version = 2; + hook->func.v2 = func; hook->user_data = user_data; hook->next = assembly_load_hook; assembly_load_hook = hook; } +/** + * mono_install_assembly_load_hook: + */ +void +mono_install_assembly_load_hook (MonoAssemblyLoadFunc func, gpointer user_data) +{ + mono_install_assembly_load_hook_v1 (func, user_data); +} + static void free_assembly_load_hooks (void) { @@ -1948,7 +1985,7 @@ mono_install_assembly_search_hook_internal_v1 (MonoAssemblySearchFunc func, gpoi } void -mono_install_assembly_search_hook_v2 (MonoAssemblySearchFuncV2 func, void *user_data, gboolean refonly, gboolean postload) +mono_install_assembly_search_hook_v2 (MonoAssemblySearchFuncV2 func, gpointer user_data, gboolean refonly, gboolean postload) { if (func == NULL) return; @@ -2509,7 +2546,7 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest return NULL; } else { /* Already loaded by another appdomain */ - mono_assembly_invoke_load_hook (image->assembly); + mono_assembly_invoke_load_hook_internal (load_req.alc, image->assembly); mono_image_close (image); g_free (fname); return image->assembly; @@ -3050,7 +3087,7 @@ mono_assembly_request_load_from (MonoImage *image, const char *fname, mono_image_fixup_vtable (image); #endif - mono_assembly_invoke_load_hook (ass); + mono_assembly_invoke_load_hook_internal (req->alc, ass); MONO_PROFILER_RAISE (assembly_loaded, (ass)); @@ -4708,9 +4745,9 @@ MonoAssembly * mono_assembly_loaded_internal (MonoAssemblyLoadContext *alc, MonoAssemblyName *aname, gboolean refonly) { MonoAssembly *res; - MonoAssemblyName maped_aname; + MonoAssemblyName mapped_aname; - aname = mono_assembly_remap_version (aname, &maped_aname); + aname = mono_assembly_remap_version (aname, &mapped_aname); res = mono_assembly_invoke_search_hook_internal (alc, NULL, aname, refonly, FALSE); diff --git a/mono/metadata/assembly.h b/mono/metadata/assembly.h index d0e4a3d7fa6..a4fffaf6578 100644 --- a/mono/metadata/assembly.h +++ b/mono/metadata/assembly.h @@ -63,7 +63,8 @@ MONO_API char* mono_stringify_assembly_name (MonoAssemblyName *aname); /* Installs a function which is called each time a new assembly is loaded. */ typedef void (*MonoAssemblyLoadFunc) (MonoAssembly *assembly, void* user_data); -MONO_API void mono_install_assembly_load_hook (MonoAssemblyLoadFunc func, void* user_data); +MONO_API MONO_RT_EXTERNAL_ONLY void +mono_install_assembly_load_hook (MonoAssemblyLoadFunc func, void* user_data); /* * Installs a new function which is used to search the list of loaded @@ -103,7 +104,8 @@ MONO_API void mono_install_assembly_preload_hook (MonoAssemblyPreLoadFu MONO_API void mono_install_assembly_refonly_preload_hook (MonoAssemblyPreLoadFunc func, void* user_data); -MONO_API void mono_assembly_invoke_load_hook (MonoAssembly *ass); +MONO_API MONO_RT_EXTERNAL_ONLY void +mono_assembly_invoke_load_hook (MonoAssembly *ass); MONO_API MonoAssemblyName* mono_assembly_name_new (const char *name); MONO_API const char* mono_assembly_name_get_name (MonoAssemblyName *aname); diff --git a/mono/metadata/loader-internals.h b/mono/metadata/loader-internals.h index d240f6a3655..9d61ed42451 100644 --- a/mono/metadata/loader-internals.h +++ b/mono/metadata/loader-internals.h @@ -6,6 +6,7 @@ #define _MONO_METADATA_LOADER_INTERNALS_H_ #include +#include #include #include #include @@ -48,12 +49,6 @@ mono_alc_assemblies_lock (MonoAssemblyLoadContext *alc); void mono_alc_assemblies_unlock (MonoAssemblyLoadContext *alc); -static inline MonoDomain * -mono_alc_domain (MonoAssemblyLoadContext *alc) -{ - return alc->domain; -} - gboolean mono_alc_is_default (MonoAssemblyLoadContext *alc); @@ -68,6 +63,16 @@ mono_alc_invoke_resolve_using_resolve_satellite_nofail (MonoAssemblyLoadContext #endif /* ENABLE_NETCORE */ +static inline MonoDomain * +mono_alc_domain (MonoAssemblyLoadContext *alc) +{ +#ifdef ENABLE_NETCORE + return alc->domain; +#else + return mono_domain_get (); +#endif +} + MonoLoadedImages * mono_alc_get_loaded_images (MonoAssemblyLoadContext *alc); diff --git a/mono/metadata/mono-debug.c b/mono/metadata/mono-debug.c index 295afda66bb..05e178c9cbd 100644 --- a/mono/metadata/mono-debug.c +++ b/mono/metadata/mono-debug.c @@ -66,7 +66,7 @@ static gboolean is_attached = FALSE; static MonoDebugHandle *mono_debug_open_image (MonoImage *image, const guint8 *raw_contents, int size); static MonoDebugHandle *mono_debug_get_image (MonoImage *image); -static void add_assembly (MonoAssembly *assembly, gpointer user_data); +static void add_assembly (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error); static MonoDebugHandle *open_symfile_from_bundle (MonoImage *image); @@ -114,7 +114,7 @@ mono_debug_init (MonoDebugFormat format) mono_debug_handles = g_hash_table_new_full (NULL, NULL, NULL, (GDestroyNotify) free_debug_handle); - mono_install_assembly_load_hook (add_assembly, NULL); + mono_install_assembly_load_hook_v2 (add_assembly, NULL); mono_debugger_unlock (); } @@ -245,7 +245,7 @@ mono_debug_open_image (MonoImage *image, const guint8 *raw_contents, int size) } static void -add_assembly (MonoAssembly *assembly, gpointer user_data) +add_assembly (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error) { MonoDebugHandle *handle; MonoImage *image; diff --git a/mono/metadata/sre.c b/mono/metadata/sre.c index f0468abadc1..782a9b4eddb 100644 --- a/mono/metadata/sre.c +++ b/mono/metadata/sre.c @@ -17,6 +17,7 @@ #include #include #include "mono/metadata/assembly.h" +#include "mono/metadata/assembly-internals.h" #include "mono/metadata/class-init.h" #include "mono/metadata/debug-helpers.h" #include "mono/metadata/dynamic-image-internals.h" @@ -1386,7 +1387,7 @@ mono_reflection_dynimage_basic_init (MonoReflectionAssemblyBuilder *assemblyb, M MONO_PROFILER_RAISE (assembly_loaded, (&assembly->assembly)); - mono_assembly_invoke_load_hook ((MonoAssembly*)assembly); + mono_assembly_invoke_load_hook_internal (alc, (MonoAssembly*)assembly); } #endif /* !DISABLE_REFLECTION_EMIT */ diff --git a/mono/mini/aot-runtime.c b/mono/mini/aot-runtime.c index a823c849a52..5b6b5f19b8b 100644 --- a/mono/mini/aot-runtime.c +++ b/mono/mini/aot-runtime.c @@ -2054,7 +2054,7 @@ init_amodule_got (MonoAotModule *amodule) } static void -load_aot_module (MonoAssembly *assembly, gpointer user_data) +load_aot_module (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error) { char *aot_name, *found_aot_name; MonoAotModule *amodule; @@ -2100,7 +2100,7 @@ if (container_assm_name && !container_amodule) { assm = mono_assembly_request_open (exe, &req, &status); } g_assert (assm); - load_aot_module (assm, NULL); + load_aot_module (alc, assm, NULL, error); container_amodule = assm->image->aot_module; } @@ -2516,7 +2516,7 @@ mono_aot_init (void) mono_os_mutex_init_recursive (&aot_page_mutex); aot_modules = g_hash_table_new (NULL, NULL); - mono_install_assembly_load_hook (load_aot_module, NULL); + mono_install_assembly_load_hook_v2 (load_aot_module, NULL); mono_counters_register ("Async JIT info size", MONO_COUNTER_INT|MONO_COUNTER_JIT, &async_jit_info_size); char *lastaot = g_getenv ("MONO_LASTAOT"); @@ -4689,7 +4689,7 @@ mono_aot_get_method (MonoDomain *domain, MonoMethod *method, MonoError *error) /* This cannot be AOTed during startup, so do it now */ if (!mscorlib_aot_loaded) { mscorlib_aot_loaded = TRUE; - load_aot_module (m_class_get_image (klass)->assembly, NULL); + load_aot_module (mono_domain_default_alc (domain), m_class_get_image (klass)->assembly, NULL, error); amodule = m_class_get_image (klass)->aot_module; } } diff --git a/tools/pedump/pedump.c b/tools/pedump/pedump.c index 5540a40841b..8414ae36e20 100644 --- a/tools/pedump/pedump.c +++ b/tools/pedump/pedump.c @@ -47,7 +47,7 @@ gboolean verify_partial_md = FALSE; static char *assembly_directory[2]; static MonoAssembly *pedump_preload (MonoAssemblyName *aname, gchar **assemblies_path, gpointer user_data); -static void pedump_assembly_load_hook (MonoAssembly *assembly, gpointer user_data); +static void pedump_assembly_load_hook (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error); static MonoAssembly *pedump_assembly_search_hook (MonoAssemblyLoadContext *alc, MonoAssembly *requesting, MonoAssemblyName *aname, gboolean refonly, gboolean postload, gpointer user_data, MonoError *error); /* unused @@ -467,7 +467,7 @@ verify_image_file (const char *fname) mono_assembly_fill_assembly_name (image, &assembly->aname); /*Finish initializing the runtime*/ - mono_install_assembly_load_hook (pedump_assembly_load_hook, NULL); + mono_install_assembly_load_hook_v2 (pedump_assembly_load_hook, NULL); mono_install_assembly_search_hook_v2 (pedump_assembly_search_hook, NULL, FALSE, FALSE); mono_init_version ("pedump", image->version); @@ -478,7 +478,7 @@ verify_image_file (const char *fname) mono_marshal_init (); } else { /*Finish initializing the runtime*/ - mono_install_assembly_load_hook (pedump_assembly_load_hook, NULL); + mono_install_assembly_load_hook_v2 (pedump_assembly_load_hook, NULL); mono_install_assembly_search_hook_v2 (pedump_assembly_search_hook, NULL, FALSE, FALSE); mono_init_version ("pedump", NULL); @@ -646,7 +646,7 @@ pedump_preload (MonoAssemblyName *aname, static GList *loaded_assemblies = NULL; static void -pedump_assembly_load_hook (MonoAssembly *assembly, gpointer user_data) +pedump_assembly_load_hook (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer user_data, MonoError *error) { loaded_assemblies = g_list_prepend (loaded_assemblies, assembly); } -- 2.11.4.GIT