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 <kornel...@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@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > >
_______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list