[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.

diff --git a/mono/metadata/image.c b/mono/metadata/image.c
index c58c4b6794a056cdcec340a869748af3bb66bfa0..161bd0031a37893599330767c38280d1b288328b 100644 (file)
--- 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 2b3eea28a151551d82226202078524a6b435e4a3..6dbf07f79414184cba5de202f48ecdc4484bd312 100644 (file)
--- 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 ba5f9d72e1aa81d635a3e6e5d1d1979da9985620..96f6a5da7c90f3c924b02fe338f3a3eb4f728f95 100644 (file)
--- 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 */