[Mono-dev] [PATCH] Use trampolines for vtable fixups (Windows) (updated)
Rodrigo Kumpera
kumpera at gmail.com
Wed Dec 2 13:33:17 EST 2009
Submit them separatedly, because the commits should be split anyway.
And please, don't put the patch inline in the email body.
Thanks,
Rodrigo
2009/11/1 Kornél Pál <kornelpal at gmail.com>
> I found out that reference counting in mono_image_close_except_pools was
> broken so I fixed that as well.
>
> Double freeing was possible with mono_close_exe_image and I fixed that as
> well.
>
> I can submit these two separately, but the first one is required by
> mono_vtfixup_trampoline to function correctly.
>
> Kornél
>
> Kornél Pál wrote:
>
>> Please see the attached patch that is a separated and improved version
>> of the prevously submitted patch.
>>
>> ChangeLog draft:
>> mono_image_fixup_vtable is now called only when the image is loaded and
>> creates trampolines rather than pointers to wrapper functions.
>>
>> A new trampoline type MONO_TRAMPOLINE_VTFIXUP is added that is created
>> by mono_image_fixup_vtable, and is replacing the pointer to the
>> trampoline with the result mono_marshal_get_vtfixup_ftnptr.
>>
>> Please review and if you like it, approve the patch.
>>
>> Thanks.
>>
>
> Index: mono/mono/metadata/domain.c
> ===================================================================
> --- mono/mono/metadata/domain.c (revision 145149)
> +++ mono/mono/metadata/domain.c (working copy)
> @@ -1735,8 +1735,10 @@
> void
> mono_close_exe_image (void)
> {
> - if (exe_image)
> + if (exe_image) {
> mono_image_close (exe_image);
> + exe_image = NULL;
> + }
> }
>
> /**
> Index: mono/mono/metadata/assembly.c
> ===================================================================
> --- mono/mono/metadata/assembly.c (revision 145149)
> +++ mono/mono/metadata/assembly.c (working copy)
> @@ -1522,11 +1522,6 @@
> loaded_assemblies = g_list_prepend (loaded_assemblies, ass);
> mono_assemblies_unlock ();
>
> -#ifdef PLATFORM_WIN32
> - if (image->is_module_handle)
> - mono_image_fixup_vtable (image);
> -#endif
> -
> mono_assembly_invoke_load_hook (ass);
>
> mono_profiler_assembly_loaded (ass, MONO_PROFILE_OK);
> Index: mono/mono/metadata/coree.c
> ===================================================================
> --- mono/mono/metadata/coree.c (revision 145149)
> +++ mono/mono/metadata/coree.c (working copy)
> @@ -106,20 +106,12 @@
> }
> }
>
> - if (!image) {
> - g_free (file_name);
> + g_free (file_name);
> +
> + if (!image)
> return FALSE;
> - }
>
> - /*
> - * FIXME: Find a better way to call
> mono_image_fixup_vtable. Only
> - * loader trampolines should be used and assembly loading
> should
> - * probably be delayed until the first call to an exported
> function.
> - */
> - if (image->tables [MONO_TABLE_ASSEMBLY].rows &&
> ((MonoCLIImageInfo*)
> image->image_info)->cli_cli_header.ch_vtable_fixups.rva)
> - assembly = mono_assembly_open (file_name, NULL);
> -
> - g_free (file_name);
> + mono_image_fixup_vtable (image);
> break;
> case DLL_PROCESS_DETACH:
> if (lpReserved != NULL)
> @@ -198,6 +190,8 @@
> argv [i] = g_utf16_to_utf8 (argvw [i], -1, NULL, NULL,
> NULL);
> LocalFree (argvw);
>
> + mono_image_fixup_vtable (image);
> +
> mono_runtime_run_main (method, argc, argv, NULL);
> mono_thread_manage ();
>
> @@ -919,8 +913,10 @@
> void
> mono_fixup_exe_image (MonoImage* image)
> {
> - if (!init_from_coree && image && image->is_module_handle)
> + if (!init_from_coree && image && image->is_module_handle) {
> MonoFixupExe ((HMODULE) image->raw_data);
> + mono_image_fixup_vtable (image);
> + }
> }
>
> #endif /* PLATFORM_WIN32 */
> Index: mono/mono/metadata/object.c
> ===================================================================
> --- mono/mono/metadata/object.c (revision 145149)
> +++ mono/mono/metadata/object.c (working copy)
> @@ -470,10 +470,18 @@
> return NULL;
> }
>
> +static gpointer
> +default_vtfixup_trampoline (gpointer slot, MonoImage *image, guint32
> token, guint16 type)
> +{
> + g_assert_not_reached ();
> + return NULL;
> +}
> +
> static MonoTrampoline arch_create_jit_trampoline = default_trampoline;
> static MonoJumpTrampoline arch_create_jump_trampoline =
> default_jump_trampoline;
> static MonoRemotingTrampoline arch_create_remoting_trampoline =
> default_remoting_trampoline;
> static MonoDelegateTrampoline arch_create_delegate_trampoline =
> default_delegate_trampoline;
> +static MonoVTFixupTrampoline arch_create_vtfixup_trampoline =
> default_vtfixup_trampoline;
> static MonoImtThunkBuilder imt_thunk_builder = NULL;
> #define ARCH_USE_IMT (imt_thunk_builder != NULL)
> #if (MONO_IMT_SIZE > 32)
> @@ -517,6 +525,12 @@
> }
>
> void
> +mono_install_vtfixup_trampoline (MonoVTFixupTrampoline func)
> +{
> + arch_create_vtfixup_trampoline = func? func:
> default_vtfixup_trampoline;
> +}
> +
> +void
> mono_install_imt_thunk_builder (MonoImtThunkBuilder func) {
> imt_thunk_builder = func;
> }
> @@ -564,6 +578,12 @@
> return arch_create_delegate_trampoline (klass);
> }
>
> +gpointer
> +mono_runtime_create_vtfixup_trampoline (gpointer slot, MonoImage *image,
> guint32 token, guint16 type)
> +{
> + return arch_create_vtfixup_trampoline (slot, image, token, type);
> +}
> +
> static MonoFreeMethodFunc default_mono_free_method = NULL;
>
> /**
> Index: mono/mono/metadata/class-internals.h
> ===================================================================
> --- mono/mono/metadata/class-internals.h (revision 145149)
> +++ mono/mono/metadata/class-internals.h (working copy)
> @@ -803,6 +803,7 @@
> typedef gpointer (*MonoJumpTrampoline) (MonoDomain *domain,
> MonoMethod *method, gboolean add_sync_wrapper);
> typedef gpointer (*MonoRemotingTrampoline) (MonoDomain *domain,
> MonoMethod *method, MonoRemotingTarget target);
> typedef gpointer (*MonoDelegateTrampoline) (MonoClass *klass);
> +typedef gpointer (*MonoVTFixupTrampoline) (gpointer slot, MonoImage
> *image, guint32 token, guint16 type);
>
> typedef gpointer (*MonoLookupDynamicToken) (MonoImage *image, guint32
> token, gboolean valid_token, MonoClass **handle_class, MonoGenericContext
> *context);
>
> @@ -892,6 +893,9 @@
> void
> mono_install_delegate_trampoline (MonoDelegateTrampoline func)
> MONO_INTERNAL;
>
> +void
> +mono_install_vtfixup_trampoline (MonoVTFixupTrampoline func)
> MONO_INTERNAL;
> +
> gpointer
> mono_lookup_dynamic_token (MonoImage *image, guint32 token,
> MonoGenericContext *context) MONO_INTERNAL;
>
> @@ -907,6 +911,9 @@
> gpointer
> mono_runtime_create_delegate_trampoline (MonoClass *klass) MONO_INTERNAL;
>
> +gpointer
> +mono_runtime_create_vtfixup_trampoline (gpointer slot, MonoImage *image,
> guint32 token, guint16 type) MONO_INTERNAL;
> +
> void
> mono_install_get_cached_class_info (MonoGetCachedClassInfo func)
> MONO_INTERNAL;
>
> Index: mono/mono/metadata/image.c
> ===================================================================
> --- mono/mono/metadata/image.c (revision 145149)
> +++ mono/mono/metadata/image.c (working copy)
> @@ -572,10 +572,6 @@
> if (image->modules [idx - 1]) {
> mono_image_addref (image->modules [idx -
> 1]);
> image->modules [idx - 1]->assembly =
> image->assembly;
> -#ifdef PLATFORM_WIN32
> - if (image->modules [idx -
> 1]->is_module_handle)
> - mono_image_fixup_vtable
> (image->modules [idx - 1]);
> -#endif
> /* g_print ("loaded module %s from %s
> (%p)\n", module_ref, image->name, image->assembly); */
> }
> g_free (module_ref);
> @@ -1292,7 +1288,6 @@
> void
> mono_image_fixup_vtable (MonoImage *image)
> {
> -#ifdef PLATFORM_WIN32
> MonoCLIImageInfo *iinfo;
> MonoPEDirEntry *de;
> MonoVTableFixup *vtfixup;
> @@ -1301,7 +1296,10 @@
> guint16 slot_type;
> int slot_count;
>
> - g_assert (image->is_module_handle);
> +#ifdef PLATFORM_WIN32
> + if (!image->is_module_handle)
> +#endif
> + g_assert_not_reached();
>
> iinfo = image->image_info;
> de = &iinfo->cli_cli_header.ch_vtable_fixups;
> @@ -1310,7 +1308,7 @@
> vtfixup = (MonoVTableFixup*) mono_image_rva_map (image, de->rva);
> if (!vtfixup)
> return;
> -
> +
> count = de->size / sizeof (MonoVTableFixup);
> while (count--) {
> if (!vtfixup->rva || !vtfixup->count)
> @@ -1320,24 +1318,26 @@
> g_assert (slot);
> slot_type = vtfixup->type;
> slot_count = vtfixup->count;
> - if (slot_type & VTFIXUP_TYPE_32BIT)
> +
> + switch (slot_type & (VTFIXUP_TYPE_32BIT |
> VTFIXUP_TYPE_64BIT)) {
> + case VTFIXUP_TYPE_32BIT:
> while (slot_count--) {
> - *((guint32*) slot) = (guint32)
> mono_marshal_get_vtfixup_ftnptr (image, *((guint32*) slot), slot_type);
> + *((guint32*) slot) = (guint32)
> mono_runtime_create_vtfixup_trampoline (slot, image, *((guint32*) slot),
> slot_type);
> slot = ((guint32*) slot) + 1;
> }
> - else if (slot_type & VTFIXUP_TYPE_64BIT)
> + break;
> + case VTFIXUP_TYPE_64BIT:
> while (slot_count--) {
> - *((guint64*) slot) = (guint64)
> mono_marshal_get_vtfixup_ftnptr (image, *((guint64*) slot), slot_type);
> - slot = ((guint32*) slot) + 1;
> + *((guint64*) slot) = (guint64)
> mono_runtime_create_vtfixup_trampoline (slot, image, *((guint64*) slot),
> slot_type);
> + slot = ((guint64*) slot) + 1;
> }
> - else
> + break;
> + default:
> g_assert_not_reached();
> + }
>
> vtfixup++;
> }
> -#else
> - g_assert_not_reached();
> -#endif
> }
>
> static void
> @@ -1424,6 +1424,15 @@
> return FALSE;
> }
>
> +#ifdef PLATFORM_WIN32
> + if (image->is_module_handle && image->has_entry_point &&
> image->ref_count == 0) {
> + /* Image will be closed by _CorDllMain. */
> + FreeLibrary ((HMODULE) image->raw_data);
> + mono_images_unlock ();
> + return FALSE;
> + }
> +#endif
> +
> loaded_images = image->ref_only ? loaded_images_refonly_hash :
> loaded_images_hash;
> image2 = g_hash_table_lookup (loaded_images, image->name);
> if (image == image2) {
> @@ -1435,19 +1444,6 @@
>
> mono_images_unlock ();
>
> -#ifdef PLATFORM_WIN32
> - if (image->is_module_handle && image->has_entry_point) {
> - mono_images_lock ();
> - if (image->ref_count == 0) {
> - /* Image will be closed by _CorDllMain. */
> - FreeLibrary ((HMODULE) image->raw_data);
> - mono_images_unlock ();
> - return FALSE;
> - }
> - mono_images_unlock ();
> - }
> -#endif
> -
> mono_profiler_module_event (image, MONO_PROFILE_START_UNLOAD);
>
> mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Unloading image
> %s [%p].", image->name, image);
> @@ -1477,10 +1473,11 @@
> }
>
> #ifdef PLATFORM_WIN32
> - mono_images_lock ();
> - if (image->is_module_handle && !image->has_entry_point)
> + if (image->is_module_handle && !image->has_entry_point) {
> + mono_images_lock ();
> FreeLibrary ((HMODULE) image->raw_data);
> - mono_images_unlock ();
> + mono_images_unlock ();
> + }
> #endif
>
> if (image->raw_buffer_used) {
> @@ -1926,10 +1923,6 @@
> }
>
> image->files [fileidx - 1] = res;
> -#ifdef PLATFORM_WIN32
> - if (res->is_module_handle)
> - mono_image_fixup_vtable (res);
> -#endif
> }
> mono_loader_unlock ();
> g_free (name);
> Index: mono/mono/metadata/marshal.c
> ===================================================================
> --- mono/mono/metadata/marshal.c (revision 145149)
> +++ mono/mono/metadata/marshal.c (working copy)
> @@ -8591,7 +8591,7 @@
> }
>
> gpointer
> -mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16
> type)
> +mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16
> type, MonoMethod **wrapper_method)
> {
> MonoMethod *method;
> MonoMethodSignature *sig;
> @@ -8641,9 +8641,15 @@
> mono_metadata_free_marshal_spec (mspecs
> [i]);
> g_free (mspecs);
>
> + *wrapper_method = method;
> return mono_compile_method (method);
> }
>
> + if (!(type & VTFIXUP_TYPE_CALL_MOST_DERIVED)) {
> + wrapper_method = NULL;
> + return mono_compile_method (method);
> + }
> +
> sig = mono_method_signature (method);
> mb = mono_mb_new (method->klass, method->name,
> MONO_WRAPPER_MANAGED_TO_MANAGED);
>
> @@ -8651,16 +8657,14 @@
> for (i = 0; i < param_count; i++)
> mono_mb_emit_ldarg (mb, i);
>
> - if (type & VTFIXUP_TYPE_CALL_MOST_DERIVED)
> - mono_mb_emit_op (mb, CEE_CALLVIRT, method);
> - else
> - mono_mb_emit_op (mb, CEE_CALL, method);
> + mono_mb_emit_op (mb, CEE_CALLVIRT, method);
> mono_mb_emit_byte (mb, CEE_RET);
>
> mb->dynamic = 1;
> method = mono_mb_create_method (mb, sig, param_count);
> mono_mb_free (mb);
>
> + *wrapper_method = method;
> return mono_compile_method (method);
> }
>
> Index: mono/mono/metadata/marshal.h
> ===================================================================
> --- mono/mono/metadata/marshal.h (revision 145149)
> +++ mono/mono/metadata/marshal.h (working copy)
> @@ -199,7 +199,7 @@
> mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass
> *delegate_klass, MonoObject **this_loc) MONO_INTERNAL;
>
> gpointer
> -mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16
> type) MONO_INTERNAL;
> +mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16
> type, MonoMethod **wrapper_method) MONO_INTERNAL;
>
> MonoMethod *
> mono_marshal_get_icall_wrapper (MonoMethodSignature *sig, const char
> *name, gconstpointer func, gboolean check_exceptions) MONO_INTERNAL;
> Index: mono/mono/mini/mini.c
> ===================================================================
> --- mono/mono/mini/mini.c (revision 145149)
> +++ mono/mono/mini/mini.c (working copy)
> @@ -5058,6 +5058,7 @@
> mono_install_jump_trampoline (mono_create_jump_trampoline);
> mono_install_remoting_trampoline
> (mono_jit_create_remoting_trampoline);
> mono_install_delegate_trampoline (mono_create_delegate_trampoline);
> + mono_install_vtfixup_trampoline (mono_create_vtfixup_trampoline);
> mono_install_create_domain_hook (mini_create_jit_domain_info);
> mono_install_free_domain_hook (mini_free_jit_domain_info);
> #endif
> Index: mono/mono/mini/mini.h
> ===================================================================
> --- mono/mono/mini/mini.h (revision 145149)
> +++ mono/mono/mini/mini.h (working copy)
> @@ -827,6 +827,7 @@
> MONO_TRAMPOLINE_GENERIC_VIRTUAL_REMOTING,
> MONO_TRAMPOLINE_MONITOR_ENTER,
> MONO_TRAMPOLINE_MONITOR_EXIT,
> + MONO_TRAMPOLINE_VTFIXUP,
> #ifdef ENABLE_LLVM
> MONO_TRAMPOLINE_LLVM_VCALL,
> #endif
> @@ -1482,6 +1483,7 @@
> gpointer mono_create_monitor_enter_trampoline (void)
> MONO_INTERNAL;
> gpointer mono_create_monitor_exit_trampoline (void)
> MONO_INTERNAL;
> gpointer mono_create_static_rgctx_trampoline (MonoMethod *m,
> gpointer addr) MONO_INTERNAL;
> +gpointer mono_create_vtfixup_trampoline (gpointer slot, MonoImage
> *image, guint32 token, guint16 type) MONO_INTERNAL;
> gpointer mono_create_llvm_vcall_trampoline (MonoMethod *method)
> MONO_INTERNAL;
> MonoVTable* mono_find_class_init_trampoline_by_addr (gconstpointer
> addr) MONO_INTERNAL;
> guint32 mono_find_rgctx_lazy_fetch_trampoline_by_addr
> (gconstpointer addr) MONO_INTERNAL;
> Index: mono/mono/mini/mini-trampolines.c
> ===================================================================
> --- mono/mono/mini/mini-trampolines.c (revision 145149)
> +++ mono/mono/mini/mini-trampolines.c (working copy)
> @@ -6,6 +6,8 @@
> #include <mono/metadata/metadata-internals.h>
> #include <mono/metadata/marshal.h>
> #include <mono/metadata/tabledefs.h>
> +#include <mono/metadata/assembly.h>
> +#include <mono/metadata/cil-coff.h>
> #include <mono/utils/mono-counters.h>
>
> #ifdef HAVE_VALGRIND_MEMCHECK_H
> @@ -779,6 +781,67 @@
> mono_monitor_exit (obj);
> }
>
> +static gpointer
> +mono_vtfixup_trampoline (gssize *regs, guint8 *code, guint8 *slot_info,
> guint8* tramp)
> +{
> + gpointer tramp_addr;
> + gpointer slot, slot_addr;
> + MonoImage *image;
> + guint32 token;
> + guint16 type;
> + gpointer addr;
> + MonoMethod *wrapper_method;
> + MonoImageOpenStatus status;
> +
> + tramp_addr = *((gpointer*) (gpointer) slot_info);
> + slot_info += sizeof (gpointer);
> + slot = *((gpointer*) (gpointer) slot_info);
> + slot_info += sizeof (gpointer);
> + image = *((gpointer*) (gpointer) slot_info);
> + slot_info += sizeof (gpointer);
> + token = *((guint32*) (gpointer) slot_info);
> + slot_info += sizeof (guint32);
> + type = *((guint16*) (gpointer) slot_info);
> +
> + if (type & VTFIXUP_TYPE_64BIT)
> + slot_addr = (gpointer) *((volatile gint64*) (slot));
> + else
> + slot_addr = (gpointer) *((volatile gint32*) (slot));
> +
> + if (slot_addr != tramp_addr)
> + return slot_addr;
> +
> + if (!image->assembly) {
> + /* Open image to increment LoadLibrary reference count */
> + g_assert (image == mono_image_open_full (image->name, NULL,
> FALSE));
> +
> + /* FIXME: Throw exceptions when assembly cannot be loaded
> */
> + /* FIXME: Fix mono_assembly_load_from_full to allow loading
> mscorlib.dll */
> + g_assert (mono_assembly_load_from_full (image, image->name,
> &status, FALSE));
> +
> + /* Release temporary reference */
> + mono_image_close (image);
> + }
> +
> + addr = mono_marshal_get_vtfixup_ftnptr (image, token, type,
> &wrapper_method);
> + if (type & VTFIXUP_TYPE_64BIT)
> +#if SIZEOF_VOID_P == 8
> + slot_addr = InterlockedCompareExchangePointer (slot, addr,
> tramp_addr);
> +#else
> + slot_addr = (gpointer)
> ves_icall_System_Threading_Interlocked_CompareExchange_Long ((gint64*) slot,
> (gint64) addr, (gint64) tramp_addr);
> +#endif
> + else
> + slot_addr = (gpointer) InterlockedCompareExchange
> ((gint32*) slot, (gint32) addr, (gint32) tramp_addr);
> +
> + if (slot_addr == tramp_addr)
> + return addr;
> +
> + if (wrapper_method)
> + mono_free_method (wrapper_method);
> +
> + return slot_addr;
> +}
> +
> #ifdef MONO_ARCH_HAVE_CREATE_DELEGATE_TRAMPOLINE
>
> /**
> @@ -922,6 +985,8 @@
> return mono_monitor_enter_trampoline;
> case MONO_TRAMPOLINE_MONITOR_EXIT:
> return mono_monitor_exit_trampoline;
> + case MONO_TRAMPOLINE_VTFIXUP:
> + return mono_vtfixup_trampoline;
> #ifdef ENABLE_LLVM
> case MONO_TRAMPOLINE_LLVM_VCALL:
> return mono_llvm_vcall_trampoline;
> @@ -956,6 +1021,7 @@
> mono_trampoline_code [MONO_TRAMPOLINE_GENERIC_VIRTUAL_REMOTING] =
> mono_arch_create_trampoline_code (MONO_TRAMPOLINE_GENERIC_VIRTUAL_REMOTING);
> mono_trampoline_code [MONO_TRAMPOLINE_MONITOR_ENTER] =
> mono_arch_create_trampoline_code (MONO_TRAMPOLINE_MONITOR_ENTER);
> mono_trampoline_code [MONO_TRAMPOLINE_MONITOR_EXIT] =
> mono_arch_create_trampoline_code (MONO_TRAMPOLINE_MONITOR_EXIT);
> + mono_trampoline_code [MONO_TRAMPOLINE_VTFIXUP] =
> mono_arch_create_trampoline_code (MONO_TRAMPOLINE_VTFIXUP);
> #ifdef ENABLE_LLVM
> mono_trampoline_code [MONO_TRAMPOLINE_LLVM_VCALL] =
> mono_arch_create_trampoline_code (MONO_TRAMPOLINE_LLVM_VCALL);
> #endif
> @@ -1280,7 +1346,31 @@
> #endif
> return code;
> }
> -
> +
> +gpointer
> +mono_create_vtfixup_trampoline (gpointer slot, MonoImage *image, guint32
> token, guint16 type)
> +{
> + gpointer tramp;
> +
> + MonoDomain *domain = mono_get_root_domain ();
> + guint8 *buf, *start;
> +
> + buf = start = mono_domain_code_reserve (domain, 3 * sizeof
> (gpointer) + sizeof (guint32) + sizeof (guint16));
> +
> + buf += sizeof (gpointer);
> + *(gpointer*) (gpointer) buf = slot;
> + buf += sizeof (gpointer);
> + *(gpointer*) (gpointer) buf = image;
> + buf += sizeof (gpointer);
> + *(guint32*) (gpointer) buf = token;
> + buf += sizeof (guint32);
> + *(guint16*) (gpointer) buf = type;
> +
> + tramp = mono_create_specific_trampoline (start,
> MONO_TRAMPOLINE_VTFIXUP, domain, NULL);
> + *(gpointer*) (gpointer) start = tramp;
> + return tramp;
> +}
> +
> #ifdef ENABLE_LLVM
> /*
> * mono_create_llvm_vcall_trampoline:
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20091202/d74d81c1/attachment-0001.html
More information about the Mono-devel-list
mailing list