Software › Gnome › Mono › Mono - Dev
[patch] coreclr/com check + icall cleanup
|
|
Sebastien Pouliot-2
|
Hello,
Two small/easy patches for review. The first one avoid calling mono_com_init when coreclr is enabled*. This avoid a crash if some assembly use [ComImport] on a type and throw a TypeLoadException - which is what happens in Silverlight. * For some reason (I guess it use COM for it's platform code, while Moonlight does not) Silverlight expose [ComImport] but otherwise does not support COM (as least for application code). Second patch removes some internal calls (all strings except one) that are not used (anymore) in the class libraries. Sebastien p.s. both patches were created from 2-6 branch but I'll commit them against HEAD too. [cominitcheck.20091029.diff] Index: mono/metadata/ChangeLog =================================================================== --- mono/metadata/ChangeLog (revision 145051) +++ mono/metadata/ChangeLog (working copy) @@ -1,3 +1,10 @@ +2009-10-29 Sebastien Pouliot <[hidden email]> + + * class.c: When CoreCLR is enabled don't call mono_init_com_types + if MONO_CLASS_IS_IMPORT return true. Instead we return a + TypeLoadException to be thrown later. This is the exception + thrown by Silverlight 2 if a type is marked with [ComImport] + 2009-10-25 Zoltan Varga <[hidden email]> * threads.c (start_wrapper): Call mono_profiler_thread_start () later after Index: mono/metadata/class.c =================================================================== --- mono/metadata/class.c (revision 145051) +++ mono/metadata/class.c (working copy) @@ -4245,9 +4245,14 @@ if (!MONO_CLASS_IS_INTERFACE (class)) { /* Imported COM Objects always derive from __ComObject. */ if (MONO_CLASS_IS_IMPORT (class)) { - mono_init_com_types (); - if (parent == mono_defaults.object_class) - parent = mono_defaults.com_object_class; + if ((mono_security_get_mode () == MONO_SECURITY_MODE_CORE_CLR)) { + /* Silverlight 2 provides [ComImport] attribute but won't load an assembly using it */ + mono_class_set_failure (class, MONO_EXCEPTION_TYPE_LOAD, NULL); + } else { + mono_init_com_types (); + if (parent == mono_defaults.object_class) + parent = mono_defaults.com_object_class; + } } if (!parent) { /* set the parent to something useful and safe, but mark the type as broken */ @@ -4295,8 +4300,14 @@ mono_class_setup_supertypes (class); } else { /* initialize com types if COM interfaces are present */ - if (MONO_CLASS_IS_IMPORT (class)) - mono_init_com_types (); + if (MONO_CLASS_IS_IMPORT (class)) { + if ((mono_security_get_mode () == MONO_SECURITY_MODE_CORE_CLR)) { + /* Silverlight 2 provides [ComImport] attribute but won't load an assembly using it */ + mono_class_set_failure (class, MONO_EXCEPTION_TYPE_LOAD, NULL); + } else { + mono_init_com_types (); + } + } class->parent = NULL; } [icalls.20091029.diff] Index: mono/metadata/string-icalls.c =================================================================== --- mono/metadata/string-icalls.c (revision 145050) +++ mono/metadata/string-icalls.c (working copy) @@ -36,69 +36,6 @@ g_assert_not_reached (); } -MonoString * -ves_icall_System_String_InternalJoin (MonoString *separator, MonoArray * value, gint32 sindex, gint32 count) -{ - MonoString * ret; - MonoString *current; - gint32 length; - gint32 pos; - gint32 insertlen; - gint32 destpos; - gint32 srclen; - gunichar2 *insert; - gunichar2 *dest; - gunichar2 *src; - - MONO_ARCH_SAVE_REGS; - - insert = mono_string_chars(separator); - insertlen = mono_string_length(separator); - - length = 0; - for (pos = sindex; pos != sindex + count; pos++) { - current = mono_array_get (value, MonoString *, pos); - if (current != NULL) - length += mono_string_length (current); - - if (pos < sindex + count - 1) - length += insertlen; - } - - ret = mono_string_new_size( mono_domain_get (), length); - dest = mono_string_chars(ret); - destpos = 0; - - for (pos = sindex; pos != sindex + count; pos++) { - current = mono_array_get (value, MonoString *, pos); - if (current != NULL) { - src = mono_string_chars (current); - srclen = mono_string_length (current); - - memcpy (dest + destpos, src, srclen * sizeof(gunichar2)); - destpos += srclen; - } - - if (pos < sindex + count - 1) { - memcpy(dest + destpos, insert, insertlen * sizeof(gunichar2)); - destpos += insertlen; - } - } - - return ret; -} - -void -ves_icall_System_String_InternalCopyTo (MonoString *me, gint32 sindex, MonoArray *dest, gint32 dindex, gint32 count) -{ - gunichar2 *destptr = (gunichar2 *) mono_array_addr(dest, gunichar2, dindex); - gunichar2 *src = mono_string_chars(me); - - MONO_ARCH_SAVE_REGS; - - memcpy(destptr, src + sindex, sizeof(gunichar2) * count); -} - /* System.StringSplitOptions */ typedef enum { STRINGSPLITOPTIONS_NONE = 0, @@ -267,112 +204,7 @@ return FALSE; } -MonoString * -ves_icall_System_String_InternalTrim (MonoString *me, MonoArray *chars, gint32 typ) -{ - MonoString * ret; - gunichar2 *src, *dest; - gint32 srclen, newlen, arrlen; - gint32 i, lenfirst, lenlast; - - MONO_ARCH_SAVE_REGS; - - srclen = mono_string_length(me); - src = mono_string_chars(me); - arrlen = mono_array_length(chars); - - lenfirst = 0; - lenlast = 0; - - if (0 == typ || 1 == typ) { - for (i = 0; i != srclen; i++) { - if (string_icall_is_in_array(chars, arrlen, src[i])) - lenfirst++; - else - break; - } - } - - if (0 == typ || 2 == typ) { - for (i = srclen - 1; i > lenfirst - 1; i--) { - if (string_icall_is_in_array(chars, arrlen, src[i])) - lenlast++; - else - break; - } - } - - newlen = srclen - lenfirst - lenlast; - if (newlen == srclen) - return me; - - ret = mono_string_new_size( mono_domain_get (), newlen); - dest = mono_string_chars(ret); - - memcpy(dest, src + lenfirst, newlen *sizeof(gunichar2)); - - return ret; -} - -gint32 -ves_icall_System_String_InternalLastIndexOfAny (MonoString *me, MonoArray *anyOf, gint32 sindex, gint32 count) -{ - gint32 pos; - gint32 loop; - gint32 arraysize; - gunichar2 *src; - - MONO_ARCH_SAVE_REGS; - - arraysize = mono_array_length(anyOf); - src = mono_string_chars(me); - - for (pos = sindex; pos > sindex - count; pos--) { - for (loop = 0; loop != arraysize; loop++) - if ( src [pos] == mono_array_get(anyOf, gunichar2, loop) ) - return pos; - } - - return -1; -} - MonoString * -ves_icall_System_String_InternalPad (MonoString *me, gint32 width, gunichar2 chr, MonoBoolean right) -{ - MonoString * ret; - gunichar2 *src; - gunichar2 *dest; - gint32 fillcount; - gint32 srclen; - gint32 i; - - MONO_ARCH_SAVE_REGS; - - srclen = mono_string_length(me); - src = mono_string_chars(me); - - ret = mono_string_new_size( mono_domain_get (), width); - dest = mono_string_chars(ret); - fillcount = width - srclen; - - if (right) { - memcpy(dest, src, srclen * sizeof(gunichar2)); - for (i = srclen; i != width; i++) - dest[i] = chr; - - return ret; - } - - /* left fill */ - for (i = 0; i != fillcount; i++) - dest[i] = chr; - - memcpy(dest + fillcount, src, srclen * sizeof(gunichar2)); - - return ret; -} - -MonoString * ves_icall_System_String_InternalAllocateStr (gint32 length) { MONO_ARCH_SAVE_REGS; @@ -380,61 +212,6 @@ return mono_string_new_size(mono_domain_get (), length); } -void -ves_icall_System_String_InternalStrcpy_Str (MonoString *dest, gint32 destPos, MonoString *src) -{ - gunichar2 *srcptr; - gunichar2 *destptr; - - MONO_ARCH_SAVE_REGS; - - srcptr = mono_string_chars (src); - destptr = mono_string_chars (dest); - - g_memmove (destptr + destPos, srcptr, mono_string_length(src) * sizeof(gunichar2)); -} - -void -ves_icall_System_String_InternalStrcpy_StrN (MonoString *dest, gint32 destPos, MonoString *src, gint32 startPos, gint32 count) -{ - gunichar2 *srcptr; - gunichar2 *destptr; - - MONO_ARCH_SAVE_REGS; - - srcptr = mono_string_chars (src); - destptr = mono_string_chars (dest); - g_memmove (destptr + destPos, srcptr + startPos, count * sizeof(gunichar2)); -} - -void -ves_icall_System_String_InternalStrcpy_Chars (MonoString *dest, gint32 destPos, MonoArray *src) -{ - gunichar2 *srcptr; - gunichar2 *destptr; - - MONO_ARCH_SAVE_REGS; - - srcptr = mono_array_addr (src, gunichar2, 0); - destptr = mono_string_chars (dest); - - g_memmove (destptr + destPos, srcptr, mono_array_length (src) * sizeof(gunichar2)); -} - -void -ves_icall_System_String_InternalStrcpy_CharsN (MonoString *dest, gint32 destPos, MonoArray *src, gint32 startPos, gint32 count) -{ - gunichar2 *srcptr; - gunichar2 *destptr; - - MONO_ARCH_SAVE_REGS; - - srcptr = mono_array_addr (src, gunichar2, 0); - destptr = mono_string_chars (dest); - - g_memmove (destptr + destPos, srcptr + startPos, count * sizeof(gunichar2)); -} - MonoString * ves_icall_System_String_InternalIntern (MonoString *str) { @@ -451,13 +228,3 @@ return mono_string_is_interned(str); } -gunichar2 -ves_icall_System_String_get_Chars (MonoString *me, gint32 idx) -{ - MONO_ARCH_SAVE_REGS; - - if ((idx < 0) || (idx >= mono_string_length (me))) - mono_raise_exception (mono_get_exception_index_out_of_range ()); - return mono_string_chars(me)[idx]; -} - Index: mono/metadata/string-icalls.h =================================================================== --- mono/metadata/string-icalls.h (revision 145050) +++ mono/metadata/string-icalls.h (working copy) @@ -17,46 +17,16 @@ void ves_icall_System_String_ctor_RedirectToCreateString (void) MONO_INTERNAL; -MonoString * -ves_icall_System_String_InternalJoin (MonoString *separator, MonoArray * value, gint32 sindex, gint32 count) MONO_INTERNAL; - -void -ves_icall_System_String_InternalCopyTo (MonoString *me, gint32 sindex, MonoArray *dest, gint32 dindex, gint32 count) MONO_INTERNAL; - MonoArray * ves_icall_System_String_InternalSplit (MonoString *me, MonoArray *separator, gint32 count, gint32 options) MONO_INTERNAL; -MonoString * -ves_icall_System_String_InternalTrim (MonoString *me, MonoArray *chars, gint32 typ) MONO_INTERNAL; - -gint32 -ves_icall_System_String_InternalLastIndexOfAny (MonoString *me, MonoArray *anyOf, gint32 sindex, gint32 count) MONO_INTERNAL; - MonoString * -ves_icall_System_String_InternalPad (MonoString *me, gint32 width, gunichar2 chr, MonoBoolean right) MONO_INTERNAL; - -MonoString * ves_icall_System_String_InternalAllocateStr (gint32 length) MONO_INTERNAL; -void -ves_icall_System_String_InternalStrcpy_Str (MonoString *dest, gint32 destPos, MonoString *src) MONO_INTERNAL; - -void -ves_icall_System_String_InternalStrcpy_StrN (MonoString *dest, gint32 destPos, MonoString *src, gint32 startPos, gint32 count) MONO_INTERNAL; - -void -ves_icall_System_String_InternalStrcpy_Chars (MonoString *dest, gint32 destPos, MonoArray *src) MONO_INTERNAL; - -void -ves_icall_System_String_InternalStrcpy_CharsN (MonoString *dest, gint32 destPos, MonoArray *src, gint32 startPos, gint32 count) MONO_INTERNAL; - MonoString * ves_icall_System_String_InternalIntern (MonoString *str) MONO_INTERNAL; MonoString * ves_icall_System_String_InternalIsInterned (MonoString *str) MONO_INTERNAL; -gunichar2 -ves_icall_System_String_get_Chars (MonoString *me, gint32 idx) MONO_INTERNAL; - #endif /* _MONO_CLI_STRING_ICALLS_H_ */ Index: mono/metadata/ChangeLog =================================================================== --- mono/metadata/ChangeLog (revision 145050) +++ mono/metadata/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2009-10-29 Sebastien Pouliot <[hidden email]> + + * string-icalls.c|h: Remove string internal calls that are not + used anymore by the class libraries. + * icall.c: Remove System_Reflection_FieldInfo_internal_from_handle + which is not used in the class librairies. + * icall-def.h: Update tables. + 2009-10-25 Zoltan Varga <[hidden email]> * threads.c (start_wrapper): Call mono_profiler_thread_start () later after Index: mono/metadata/icall-def.h =================================================================== --- mono/metadata/icall-def.h (revision 145050) +++ mono/metadata/icall-def.h (working copy) @@ -1,6 +1,6 @@ /* * This file contains the default set of the mono internal calls. - * Each type that ahs internal call methods must be declared here + * Each type that has internal call methods must be declared here * with the ICALL_TYPE macro as follows: * * ICALL_TYPE(typeid, typename, first_icall_id) @@ -551,8 +551,7 @@ ICALL_TYPE(FIELDI, "System.Reflection.FieldInfo", FILEDI_1) ICALL(FILEDI_1, "GetTypeModifiers", ves_icall_System_Reflection_FieldInfo_GetTypeModifiers) ICALL(FILEDI_2, "GetUnmanagedMarshal", ves_icall_System_Reflection_FieldInfo_GetUnmanagedMarshal) -ICALL(FILEDI_3, "internal_from_handle", ves_icall_System_Reflection_FieldInfo_internal_from_handle) -ICALL(FILEDI_4, "internal_from_handle_type", ves_icall_System_Reflection_FieldInfo_internal_from_handle_type) +ICALL(FILEDI_3, "internal_from_handle_type", ves_icall_System_Reflection_FieldInfo_internal_from_handle_type) ICALL_TYPE(MEMBERI, "System.Reflection.MemberInfo", MEMBERI_1) ICALL(MEMBERI_1, "get_MetadataToken", mono_reflection_get_token) @@ -785,20 +784,9 @@ ICALL(STRING_7, ".ctor(sbyte*,int,int)", ves_icall_System_String_ctor_RedirectToCreateString) ICALL(STRING_8, ".ctor(sbyte*,int,int,System.Text.Encoding)", ves_icall_System_String_ctor_RedirectToCreateString) ICALL(STRING_9, "InternalAllocateStr", ves_icall_System_String_InternalAllocateStr) -ICALL(STRING_11, "InternalCopyTo", ves_icall_System_String_InternalCopyTo) -ICALL(STRING_14, "InternalIntern", ves_icall_System_String_InternalIntern) -ICALL(STRING_15, "InternalIsInterned", ves_icall_System_String_InternalIsInterned) -ICALL(STRING_16, "InternalJoin", ves_icall_System_String_InternalJoin) -ICALL(STRING_17, "InternalLastIndexOfAny", ves_icall_System_String_InternalLastIndexOfAny) -ICALL(STRING_18, "InternalPad", ves_icall_System_String_InternalPad) -ICALL(STRING_21, "InternalReplace(string,string,System.Globalization.CompareInfo)", ves_icall_System_String_InternalReplace_Str_Comp) -ICALL(STRING_22, "InternalSplit", ves_icall_System_String_InternalSplit) -ICALL(STRING_23, "InternalStrcpy(string,int,char[])", ves_icall_System_String_InternalStrcpy_Chars) -ICALL(STRING_24, "InternalStrcpy(string,int,char[],int,int)", ves_icall_System_String_InternalStrcpy_CharsN) -ICALL(STRING_25, "InternalStrcpy(string,int,string)", ves_icall_System_String_InternalStrcpy_Str) -ICALL(STRING_26, "InternalStrcpy(string,int,string,int,int)", ves_icall_System_String_InternalStrcpy_StrN) -ICALL(STRING_27, "InternalTrim", ves_icall_System_String_InternalTrim) -ICALL(STRING_28, "get_Chars", ves_icall_System_String_get_Chars) +ICALL(STRING_10, "InternalIntern", ves_icall_System_String_InternalIntern) +ICALL(STRING_11, "InternalIsInterned", ves_icall_System_String_InternalIsInterned) +ICALL(STRING_12, "InternalSplit", ves_icall_System_String_InternalSplit) ICALL_TYPE(TENC, "System.Text.Encoding", TENC_1) ICALL(TENC_1, "InternalCodePage", ves_icall_System_Text_Encoding_InternalCodePage) Index: mono/metadata/icall.c =================================================================== --- mono/metadata/icall.c (revision 145050) +++ mono/metadata/icall.c (working copy) @@ -1621,16 +1621,6 @@ return mono_field_get_object (mono_domain_get (), klass, handle); } -static MonoReflectionField* -ves_icall_System_Reflection_FieldInfo_internal_from_handle (MonoClassField *handle) -{ - MONO_ARCH_SAVE_REGS; - - g_assert (handle); - - return mono_field_get_object (mono_domain_get (), handle->parent, handle); -} - static MonoArray* ves_icall_System_Reflection_FieldInfo_GetTypeModifiers (MonoReflectionField *field, MonoBoolean optional) { _______________________________________________ Mono-devel-list mailing list [hidden email] http://lists.ximian.com/mailman/listinfo/mono-devel-list |
||||||||||||||||
Rodrigo Kumpera
|
The icall removal patch is ok.
The second one is tricky. Do we really want to completely disable COM support when running under the sandbox? It does make sense for moonlight, but not for other users of coreclr. I believe we should only fail COM for non-platform assemblies which has the same result for moonlight but won't bite future users of the sandbox code. On Thu, Oct 29, 2009 at 4:43 PM, Sebastien Pouliot <[hidden email]> wrote: Hello, _______________________________________________ Mono-devel-list mailing list [hidden email] http://lists.ximian.com/mailman/listinfo/mono-devel-list |
||||||||||||||||
|
Sebastien Pouliot-2
|
On Wed, 2009-11-04 at 16:31 -0200, Rodrigo Kumpera wrote:
> The icall removal patch is ok. committed > The second one is tricky. Do we really want to completely disable COM > support when running under the sandbox? I don't see this as an immediate issue but... > It does make sense for moonlight, but not for other users of coreclr. > > I believe we should only fail COM for non-platform assemblies which > has the same result for moonlight a new patch is attached. > but won't > bite future users of the sandbox code. Well it won't change anything for Moonlight[1] but it will still bite any other (well future) user of coreclr unless the BCL they provide offers the required COM types [2]. Otherwise it will simply abort (like id does today). Sebastien [1] unless someone adds a [ComImport] somewhere in the platform code - but that would not pass our test suite :) [2] A added a FIXME in the patch about this. In any case the g_abort should make it clear enough to runtime embedders > > > On Thu, Oct 29, 2009 at 4:43 PM, Sebastien Pouliot > <[hidden email]> wrote: > Hello, > > Two small/easy patches for review. > > The first one avoid calling mono_com_init when coreclr is > enabled*. > This avoid a crash if some assembly use [ComImport] on a type > and throw > a TypeLoadException - which is what happens in Silverlight. > > * For some reason (I guess it use COM for it's platform > code, > while Moonlight does not) Silverlight expose > [ComImport] but > otherwise does not support COM (as least for > application code). > > Second patch removes some internal calls (all strings except > one) that > are not used (anymore) in the class libraries. > > Sebastien > > p.s. both patches were created from 2-6 branch but I'll commit > them > against HEAD too. > > _______________________________________________ > Mono-devel-list mailing list > [hidden email] > http://lists.ximian.com/mailman/listinfo/mono-devel-list > > > _______________________________________________ > Mono-devel-list mailing list > [hidden email] > http://lists.ximian.com/mailman/listinfo/mono-devel-list [cominitcheck.20091105.diff] Index: mono/metadata/ChangeLog =================================================================== --- mono/metadata/ChangeLog (revision 145149) +++ mono/metadata/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2009-11-05 Sebastien Pouliot <[hidden email]> + + * class.c: When CoreCLR is enabled don't call mono_init_com_types + if MONO_CLASS_IS_IMPORT return true unless the type reside in + platform (trusted) code. Instead we return a TypeLoadException to + be thrown later. This is the exception thrown by Silverlight 2 if + a type, inside application (user) code is marked with [ComImport] + 2009-10-31 Zoltan Varga <[hidden email]> * appdomain.c (mono_domain_try_unload): Applied patch from Romain Tartière. Index: mono/metadata/class.c =================================================================== --- mono/metadata/class.c (revision 145487) +++ mono/metadata/class.c (working copy) @@ -4274,6 +4274,30 @@ } /* + * COM initialization (using mono_init_com_types) is delayed until needed. + * However when a [ComImport] attribute is present on a type it will trigger + * the initialization. This is not a problem unless the BCL being executed + * lacks the types that COM depends on (e.g. Variant on Silverlight). + */ +static void +init_com_from_comimport (MonoClass *class) +{ + /* we don't always allow COM initialization under the CoreCLR (e.g. Moonlight does not require it) */ + if ((mono_security_get_mode () == MONO_SECURITY_MODE_CORE_CLR)) { + /* but some other CoreCLR user could requires it for their platform (i.e. trusted) code */ + if (!mono_security_core_clr_determine_platform_image (class->image)) { + /* but it can not be made available for application (i.e. user code) since all COM calls + * are considered native calls. In this case we fail with a TypeLoadException (just like + * Silverlight 2 does */ + mono_class_set_failure (class, MONO_EXCEPTION_TYPE_LOAD, NULL); + return; + } + } + /* FIXME : we should add an extra checks to ensure COM can be initialized properly before continuing */ + mono_init_com_types (); +} + +/* * LOCKING: this assumes the loader lock is held */ void @@ -4299,7 +4323,7 @@ if (!MONO_CLASS_IS_INTERFACE (class)) { /* Imported COM Objects always derive from __ComObject. */ if (MONO_CLASS_IS_IMPORT (class)) { - mono_init_com_types (); + init_com_from_comimport (class); if (parent == mono_defaults.object_class) parent = mono_defaults.com_object_class; } @@ -4350,7 +4374,7 @@ } else { /* initialize com types if COM interfaces are present */ if (MONO_CLASS_IS_IMPORT (class)) - mono_init_com_types (); + init_com_from_comimport (class); class->parent = NULL; } _______________________________________________ Mono-devel-list mailing list [hidden email] http://lists.ximian.com/mailman/listinfo/mono-devel-list |
||||||||||||||||
|
Rodrigo Kumpera
|
The new patch looks good.
On Thu, Nov 5, 2009 at 6:30 PM, Sebastien Pouliot <[hidden email]> wrote:
_______________________________________________ Mono-devel-list mailing list [hidden email] http://lists.ximian.com/mailman/listinfo/mono-devel-list |
||||||||||||||||
| Free Embeddable Forum Powered by Nabble | Help |
