From b1dccbc236c337bb12ec4c7075851b9045c97865 Mon Sep 17 00:00:00 2001 From: Alexander Kyte Date: Fri, 10 Jun 2016 05:05:51 -0400 Subject: [PATCH] [runtime] Ensure uniqueness of icall wrappers (#3143) The llvmonly backend will use `mono_aot_init_gshared_method_this` and other initializers to initialize methods. These will transitively call `mono_icall_get_wrapper_full` when icall wrappers are needed. The init functions will set the initialized flag after initialization. They set it in a non-atomic manner. This means that in a multithreaded environment, it's possible to call the functions twice. When `mono_icall_get_wrapper_full` is called without this patch, the pointers to wrappers it gets from `mono_marshal_get_icall_wrapper` will not have the same addresses. They will hash to the same value though. This will lead to them not being found or inserted by `find_aot_method_in_amodule`. The fact that we already have a value with this hash means that the new wrapper doesn't get inserted. The fact that the pointers differ mean that we don't say that we've found our desired wrapper. This leads to an assertion in `mono_icall_get_wrapper_full` crashing the runtime. Since we intend to have the semantics of a wrapper for each icall, this change uses a cache to ensure uniqueness. The observed crashes do not happen when this change is applied. --- mono/metadata/image.c | 1 + mono/metadata/marshal.c | 6 +++++- mono/metadata/metadata-internals.h | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mono/metadata/image.c b/mono/metadata/image.c index c58c4b6794a..161bd0031a3 100644 --- a/mono/metadata/image.c +++ b/mono/metadata/image.c @@ -1802,6 +1802,7 @@ mono_image_close_except_pools (MonoImage *image) free_hash (image->stfld_wrapper_cache); free_hash (image->isinst_cache); free_hash (image->castclass_cache); + free_hash (image->icall_wrapper_cache); free_hash (image->proxy_isinst_cache); free_hash (image->var_cache_slow); free_hash (image->mvar_cache_slow); diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c index 2b3eea28a15..6dbf07f7941 100644 --- a/mono/metadata/marshal.c +++ b/mono/metadata/marshal.c @@ -4458,6 +4458,10 @@ mono_marshal_get_icall_wrapper (MonoMethodSignature *sig, const char *name, gcon int i; WrapperInfo *info; + GHashTable *cache = get_cache (&mono_defaults.object_class->image->icall_wrapper_cache, mono_aligned_addr_hash, NULL); + if ((res = mono_marshal_find_in_cache (cache, (gpointer) func))) + return res; + g_assert (sig->pinvoke); mb = mono_mb_new (mono_defaults.object_class, name, MONO_WRAPPER_MANAGED_TO_NATIVE); @@ -4492,7 +4496,7 @@ mono_marshal_get_icall_wrapper (MonoMethodSignature *sig, const char *name, gcon info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_ICALL_WRAPPER); info->d.icall.func = (gpointer)func; - res = mono_mb_create (mb, csig, csig->param_count + 16, info); + res = mono_mb_create_and_cache_full (cache, (gpointer) func, mb, csig, csig->param_count + 16, info, NULL); mono_mb_free (mb); return res; diff --git a/mono/metadata/metadata-internals.h b/mono/metadata/metadata-internals.h index ba5f9d72e1a..96f6a5da7c9 100644 --- a/mono/metadata/metadata-internals.h +++ b/mono/metadata/metadata-internals.h @@ -334,6 +334,8 @@ struct _MonoImage { GHashTable *ldflda_wrapper_cache; GHashTable *stfld_wrapper_cache; GHashTable *isinst_cache; + + GHashTable *icall_wrapper_cache; GHashTable *castclass_cache; GHashTable *proxy_isinst_cache; GHashTable *rgctx_template_hash; /* LOCKING: templates lock */ -- 2.25.1