From 1ac0edf2873814fa34eab81a5c0e32ffd4166c89 Mon Sep 17 00:00:00 2001 From: Jay Krell Date: Fri, 13 Dec 2019 08:29:30 -0800 Subject: [PATCH] Fix on-demand initialization race conditions [marshal-ilgen.c] (mono/mono#18160) * Fix on-demand initialization race conditions [marshal-ilgen.c] * PR: Feedback more MONO_STATIC_POINTER_INIT instead of manual mono_atomic_store_seq. * PR feedback: Narrow scope of from_oadate. * One more MONO_STATIC_POINTER_INIT vs. mono_atomic_store_seq post-review. * Repair whitespace. * Alternate formating. Commit migrated from https://github.com/mono/mono/commit/b637c1db670c0d7355ae4f43f2e4ea1d0b162434 --- src/mono/mono/metadata/marshal-ilgen.c | 134 ++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 59 deletions(-) diff --git a/src/mono/mono/metadata/marshal-ilgen.c b/src/mono/mono/metadata/marshal-ilgen.c index 9cc0188..b6833d0 100644 --- a/src/mono/mono/metadata/marshal-ilgen.c +++ b/src/mono/mono/metadata/marshal-ilgen.c @@ -82,6 +82,7 @@ static GENERATE_TRY_GET_CLASS_WITH_CACHE (marshal, "System.Runtime.InteropServic static MonoMethod *sh_dangerous_add_ref; static MonoMethod *sh_dangerous_release; +// FIXME Consolidate the multiple functions named get_method_nofail. static MonoMethod* get_method_nofail (MonoClass *klass, const char *method_name, int num_params, int flags) { @@ -96,10 +97,8 @@ get_method_nofail (MonoClass *klass, const char *method_name, int num_params, in static void init_safe_handle (void) { - sh_dangerous_add_ref = get_method_nofail ( - mono_class_try_get_safehandle_class (), "DangerousAddRef", 1, 0); - sh_dangerous_release = get_method_nofail ( - mono_class_try_get_safehandle_class (), "DangerousRelease", 0, 0); + mono_atomic_store_seq (&sh_dangerous_add_ref, get_method_nofail (mono_class_try_get_safehandle_class (), "DangerousAddRef", 1, 0)); + mono_atomic_store_seq (&sh_dangerous_release, get_method_nofail (mono_class_try_get_safehandle_class (), "DangerousRelease", 0, 0)); } static MonoImage* @@ -934,6 +933,48 @@ emit_object_to_ptr_conv (MonoMethodBuilder *mb, MonoType *type, MonoMarshalConv } } +#ifndef DISABLE_COM + +// FIXME There are multiple caches of "Clear". +G_GNUC_UNUSED +static MonoMethod* +mono_get_Variant_Clear (void) +{ + MONO_STATIC_POINTER_INIT (MonoMethod, variant_clear) + variant_clear = get_method_nofail (mono_class_get_variant_class (), "Clear", 0, 0); + MONO_STATIC_POINTER_INIT_END (MonoMethod, variant_clear) + + g_assert (variant_clear); + return variant_clear; +} + +#endif + +// FIXME There are multiple caches of "GetObjectForNativeVariant". +G_GNUC_UNUSED +static MonoMethod* +mono_get_Marshal_GetObjectForNativeVariant (void) +{ + MONO_STATIC_POINTER_INIT (MonoMethod, get_object_for_native_variant) + get_object_for_native_variant = get_method_nofail (mono_defaults.marshal_class, "GetObjectForNativeVariant", 1, 0); + MONO_STATIC_POINTER_INIT_END (MonoMethod, get_object_for_native_variant) + + g_assert (get_object_for_native_variant); + return get_object_for_native_variant; +} + +// FIXME There are multiple caches of "GetNativeVariantForObject". +G_GNUC_UNUSED +static MonoMethod* +mono_get_Marshal_GetNativeVariantForObject (void) +{ + MONO_STATIC_POINTER_INIT (MonoMethod, get_native_variant_for_object) + get_native_variant_for_object = get_method_nofail (mono_defaults.marshal_class, "GetNativeVariantForObject", 2, 0); + MONO_STATIC_POINTER_INIT_END (MonoMethod, get_native_variant_for_object) + + g_assert (get_native_variant_for_object); + return get_native_variant_for_object; +} static void emit_struct_conv_full (MonoMethodBuilder *mb, MonoClass *klass, gboolean to_object, @@ -1105,31 +1146,19 @@ emit_struct_conv_full (MonoMethodBuilder *mb, MonoClass *klass, gboolean to_obje case MONO_TYPE_OBJECT: { #ifndef DISABLE_COM if (to_object) { - static MonoMethod *variant_clear = NULL; - static MonoMethod *get_object_for_native_variant = NULL; - - if (!variant_clear) - variant_clear = get_method_nofail (mono_class_get_variant_class (), "Clear", 0, 0); - if (!get_object_for_native_variant) - get_object_for_native_variant = get_method_nofail (mono_defaults.marshal_class, "GetObjectForNativeVariant", 1, 0); mono_mb_emit_ldloc (mb, 1); mono_mb_emit_ldloc (mb, 0); - mono_mb_emit_managed_call (mb, get_object_for_native_variant, NULL); + mono_mb_emit_managed_call (mb, mono_get_Marshal_GetObjectForNativeVariant (), NULL); mono_mb_emit_byte (mb, CEE_STIND_REF); mono_mb_emit_ldloc (mb, 0); - mono_mb_emit_managed_call (mb, variant_clear, NULL); + mono_mb_emit_managed_call (mb, mono_get_Variant_Clear (), NULL); } else { - static MonoMethod *get_native_variant_for_object = NULL; - - if (!get_native_variant_for_object) - get_native_variant_for_object = get_method_nofail (mono_defaults.marshal_class, "GetNativeVariantForObject", 2, 0); - mono_mb_emit_ldloc (mb, 0); mono_mb_emit_byte(mb, CEE_LDIND_REF); mono_mb_emit_ldloc (mb, 1); - mono_mb_emit_managed_call (mb, get_native_variant_for_object, NULL); + mono_mb_emit_managed_call (mb, mono_get_Marshal_GetNativeVariantForObject (), NULL); } #else char *msg = g_strdup_printf ("COM support was disabled at compilation time."); @@ -4339,16 +4368,14 @@ emit_thunk_invoke_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, Mono static void emit_marshal_custom_get_instance (MonoMethodBuilder *mb, MonoClass *klass, MonoMarshalSpec *spec) { - static MonoClass *Marshal = NULL; - static MonoMethod *get_instance; + MONO_STATIC_POINTER_INIT (MonoMethod, get_instance) - if (!Marshal) { - Marshal = mono_class_try_get_marshal_class (); + MonoClass *Marshal = mono_class_try_get_marshal_class (); g_assert (Marshal); - get_instance = get_method_nofail (Marshal, "GetCustomMarshalerInstance", 2, 0); g_assert (get_instance); - } + + MONO_STATIC_POINTER_INIT_END (MonoClass, get_instance) // HACK: We cannot use ldtoken in this type of wrapper. mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX); @@ -4405,10 +4432,13 @@ emit_marshal_custom_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, cleanup_native = get_method_nofail (klass, "CleanUpNativeData", 1, 0); g_assert (cleanup_native); + cleanup_managed = get_method_nofail (klass, "CleanUpManagedData", 1, 0); g_assert (cleanup_managed); + marshal_managed_to_native = get_method_nofail (klass, "MarshalManagedToNative", 1, 0); g_assert (marshal_managed_to_native); + marshal_native_to_managed = get_method_nofail (klass, "MarshalNativeToManaged", 1, 0); g_assert (marshal_native_to_managed); @@ -4699,11 +4729,6 @@ emit_marshal_vtype_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, case MARSHAL_ACTION_CONV_IN: if (klass == date_time_class) { /* Convert it to an OLE DATE type */ - static MonoMethod *to_oadate; - - if (!to_oadate) - to_oadate = get_method_nofail (date_time_class, "ToOADate", 0, 0); - g_assert (to_oadate); conv_arg = mono_mb_add_local (mb, double_type); @@ -4716,6 +4741,11 @@ emit_marshal_vtype_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, if (!t->byref) m->csig->params [argnum - m->csig->hasthis] = double_type; + MONO_STATIC_POINTER_INIT (MonoMethod, to_oadate) + to_oadate = get_method_nofail (date_time_class, "ToOADate", 0, 0); + g_assert (to_oadate); + MONO_STATIC_POINTER_INIT_END (MonoMethod, to_oadate) + mono_mb_emit_ldarg_addr (mb, argnum); mono_mb_emit_managed_call (mb, to_oadate, NULL); mono_mb_emit_stloc (mb, conv_arg); @@ -4801,14 +4831,16 @@ emit_marshal_vtype_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, case MARSHAL_ACTION_CONV_OUT: if (klass == date_time_class) { /* Convert from an OLE DATE type */ - static MonoMethod *from_oadate; if (!t->byref) break; if (!((t->attrs & PARAM_ATTRIBUTE_IN) && !(t->attrs & PARAM_ATTRIBUTE_OUT))) { - if (!from_oadate) + + MONO_STATIC_POINTER_INIT (MonoMethod, from_oadate) from_oadate = get_method_nofail (date_time_class, "FromOADate", 1, 0); + MONO_STATIC_POINTER_INIT_END (MonoMethod, from_oadate) + g_assert (from_oadate); mono_mb_emit_ldarg (mb, argnum); @@ -5003,10 +5035,6 @@ emit_marshal_string_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, } if (encoding == MONO_NATIVE_VBBYREFSTR) { - static MonoMethod *m; - - if (!m) - m = get_method_nofail (mono_defaults.string_class, "get_Length", -1, 0); if (!t->byref) { char *msg = g_strdup ("VBByRefStr marshalling requires a ref parameter."); @@ -5014,6 +5042,12 @@ emit_marshal_string_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, break; } + MONO_STATIC_POINTER_INIT (MonoMethod, m) + + m = get_method_nofail (mono_defaults.string_class, "get_Length", -1, 0); + + MONO_STATIC_POINTER_INIT_END (MonoMethod, m) + /* * Have to allocate a new string with the same length as the original, and * copy the contents of the buffer pointed to by CONV_ARG into it. @@ -5891,7 +5925,6 @@ emit_marshal_object_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, return conv_arg; } - static int emit_marshal_variant_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, MonoMarshalSpec *spec, @@ -5900,20 +5933,10 @@ emit_marshal_variant_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, { #ifndef DISABLE_COM MonoMethodBuilder *mb = m->mb; - static MonoMethod *get_object_for_native_variant = NULL; - static MonoMethod *get_native_variant_for_object = NULL; MonoType *variant_type = m_class_get_byval_arg (mono_class_get_variant_class ()); MonoType *variant_type_byref = m_class_get_this_arg (mono_class_get_variant_class ()); MonoType *object_type = mono_get_object_type (); - if (!get_object_for_native_variant) - get_object_for_native_variant = get_method_nofail (mono_defaults.marshal_class, "GetObjectForNativeVariant", 1, 0); - g_assert (get_object_for_native_variant); - - if (!get_native_variant_for_object) - get_native_variant_for_object = get_method_nofail (mono_defaults.marshal_class, "GetNativeVariantForObject", 2, 0); - g_assert (get_native_variant_for_object); - switch (action) { case MARSHAL_ACTION_CONV_IN: { conv_arg = mono_mb_add_local (mb, variant_type); @@ -5930,27 +5953,20 @@ emit_marshal_variant_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, if (t->byref) mono_mb_emit_byte(mb, CEE_LDIND_REF); mono_mb_emit_ldloc_addr (mb, conv_arg); - mono_mb_emit_managed_call (mb, get_native_variant_for_object, NULL); + mono_mb_emit_managed_call (mb, mono_get_Marshal_GetNativeVariantForObject (), NULL); break; } case MARSHAL_ACTION_CONV_OUT: { - static MonoMethod *variant_clear = NULL; - - if (!variant_clear) - variant_clear = get_method_nofail (mono_class_get_variant_class (), "Clear", 0, 0); - g_assert (variant_clear); - - if (t->byref && (t->attrs & PARAM_ATTRIBUTE_OUT || !(t->attrs & PARAM_ATTRIBUTE_IN))) { mono_mb_emit_ldarg (mb, argnum); mono_mb_emit_ldloc_addr (mb, conv_arg); - mono_mb_emit_managed_call (mb, get_object_for_native_variant, NULL); + mono_mb_emit_managed_call (mb, mono_get_Marshal_GetObjectForNativeVariant (), NULL); mono_mb_emit_byte (mb, CEE_STIND_REF); } mono_mb_emit_ldloc_addr (mb, conv_arg); - mono_mb_emit_managed_call (mb, variant_clear, NULL); + mono_mb_emit_managed_call (mb, mono_get_Variant_Clear (), NULL); break; } @@ -5982,7 +5998,7 @@ emit_marshal_variant_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, mono_mb_emit_ldarg (mb, argnum); else mono_mb_emit_ldarg_addr (mb, argnum); - mono_mb_emit_managed_call (mb, get_object_for_native_variant, NULL); + mono_mb_emit_managed_call (mb, mono_get_Marshal_GetObjectForNativeVariant (), NULL); mono_mb_emit_stloc (mb, conv_arg); break; } @@ -5991,7 +6007,7 @@ emit_marshal_variant_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, if (t->byref && (t->attrs & PARAM_ATTRIBUTE_OUT || !(t->attrs & PARAM_ATTRIBUTE_IN))) { mono_mb_emit_ldloc (mb, conv_arg); mono_mb_emit_ldarg (mb, argnum); - mono_mb_emit_managed_call (mb, get_native_variant_for_object, NULL); + mono_mb_emit_managed_call (mb, mono_get_Marshal_GetNativeVariantForObject (), NULL); } break; } -- 2.7.4