Merge pull request #2897 from kumpera/marshal_fixes

[marshal] Ensure marshal code emit the right indirect store when dealing with ref types.

The marshal code was using CEE_STIND_I for a lot of ref stores. This would not
generate a WB and is a heap corruption vector.

diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c
index 3d4e8146decc3263d8a87bbba9a445e391fcfb57..00ddf5da912dcca1830770965bf33e3bb80d2e89 100644 (file)
--- a/mono/metadata/marshal.c
+++ b/mono/metadata/marshal.c
@@ -1216,7 +1216,8 @@ guint
 mono_type_to_stind (MonoType *type)
 {
        if (type->byref)
-               return CEE_STIND_I;
+               return MONO_TYPE_IS_REFERENCE (type) ? CEE_STIND_REF : CEE_STIND_I;
+
 
 handle_enum:
        switch (type->type) {
@@ -1312,7 +1313,7 @@ emit_ptr_to_object_conv (MonoMethodBuilder *mb, MonoType *type, MonoMarshalConv
                mono_mb_emit_ldloc (mb, 1);
                mono_mb_emit_icon (mb, mspec->data.array_data.num_elem);
                mono_mb_emit_op (mb, CEE_NEWARR, eklass);       
-               mono_mb_emit_byte (mb, CEE_STIND_I);
+               mono_mb_emit_byte (mb, CEE_STIND_REF);
 
                if (eklass->blittable) {
                        /* copy the elements */
@@ -1550,16 +1551,23 @@ emit_ptr_to_object_conv (MonoMethodBuilder *mb, MonoType *type, MonoMarshalConv
 }
 
 static gpointer
-conv_to_icall (MonoMarshalConv conv)
+conv_to_icall (MonoMarshalConv conv, int *ind_store_type)
 {
+       int dummy;
+       if (!ind_store_type)
+               ind_store_type = &dummy;
+       *ind_store_type = CEE_STIND_I;
        switch (conv) {
        case MONO_MARSHAL_CONV_STR_LPWSTR:
                return mono_marshal_string_to_utf16;            
        case MONO_MARSHAL_CONV_LPWSTR_STR:
+               *ind_store_type = CEE_STIND_REF;
                return mono_string_from_utf16;
        case MONO_MARSHAL_CONV_LPTSTR_STR:
+               *ind_store_type = CEE_STIND_REF;
                return mono_string_new_wrapper;
        case MONO_MARSHAL_CONV_LPSTR_STR:
+               *ind_store_type = CEE_STIND_REF;
                return mono_string_new_wrapper;
        case MONO_MARSHAL_CONV_STR_LPTSTR:
 #ifdef TARGET_WIN32
@@ -1572,6 +1580,7 @@ conv_to_icall (MonoMarshalConv conv)
        case MONO_MARSHAL_CONV_STR_BSTR:
                return mono_string_to_bstr;
        case MONO_MARSHAL_CONV_BSTR_STR:
+               *ind_store_type = CEE_STIND_REF;
                return mono_string_from_bstr;
        case MONO_MARSHAL_CONV_STR_TBSTR:
        case MONO_MARSHAL_CONV_STR_ANSIBSTR:
@@ -1595,16 +1604,20 @@ conv_to_icall (MonoMarshalConv conv)
        case MONO_MARSHAL_CONV_DEL_FTN:
                return mono_delegate_to_ftnptr;
        case MONO_MARSHAL_CONV_FTN_DEL:
+               *ind_store_type = CEE_STIND_REF;
                return mono_ftnptr_to_delegate;
        case MONO_MARSHAL_CONV_LPSTR_SB:
+               *ind_store_type = CEE_STIND_REF;
                return mono_string_utf8_to_builder;
        case MONO_MARSHAL_CONV_LPTSTR_SB:
+               *ind_store_type = CEE_STIND_REF;
 #ifdef TARGET_WIN32
                return mono_string_utf16_to_builder;
 #else
                return mono_string_utf8_to_builder;
 #endif
        case MONO_MARSHAL_CONV_LPWSTR_SB:
+               *ind_store_type = CEE_STIND_REF;
                return mono_string_utf16_to_builder;
        case MONO_MARSHAL_FREE_ARRAY:
                return mono_marshal_free_array;
@@ -1623,6 +1636,7 @@ static void
 emit_object_to_ptr_conv (MonoMethodBuilder *mb, MonoType *type, MonoMarshalConv conv, MonoMarshalSpec *mspec)
 {
        int pos;
+       int stind_op;
 
        switch (conv) {
        case MONO_MARSHAL_CONV_BOOL_I4:
@@ -1657,8 +1671,8 @@ emit_object_to_ptr_conv (MonoMethodBuilder *mb, MonoType *type, MonoMarshalConv
                mono_mb_emit_ldloc (mb, 1);
                mono_mb_emit_ldloc (mb, 0);
                mono_mb_emit_byte (mb, CEE_LDIND_REF);
-               mono_mb_emit_icall (mb, conv_to_icall (conv));
-               mono_mb_emit_byte (mb, CEE_STIND_I);    
+               mono_mb_emit_icall (mb, conv_to_icall (conv, &stind_op));
+               mono_mb_emit_byte (mb, stind_op);
                break;
        }
        case MONO_MARSHAL_CONV_ARRAY_SAVEARRAY:
@@ -1667,8 +1681,8 @@ emit_object_to_ptr_conv (MonoMethodBuilder *mb, MonoType *type, MonoMarshalConv
                mono_mb_emit_ldloc (mb, 1);
                mono_mb_emit_ldloc (mb, 0);
                mono_mb_emit_byte (mb, CEE_LDIND_REF);
-               mono_mb_emit_icall (mb, conv_to_icall (conv));
-               mono_mb_emit_byte (mb, CEE_STIND_I);    
+               mono_mb_emit_icall (mb, conv_to_icall (conv, &stind_op));
+               mono_mb_emit_byte (mb, stind_op);
                break;
        case MONO_MARSHAL_CONV_STR_BYVALSTR: 
        case MONO_MARSHAL_CONV_STR_BYVALWSTR: {
@@ -1678,7 +1692,7 @@ emit_object_to_ptr_conv (MonoMethodBuilder *mb, MonoType *type, MonoMarshalConv
                mono_mb_emit_ldloc (mb, 0);     
                mono_mb_emit_byte (mb, CEE_LDIND_REF); /* src String */
                mono_mb_emit_icon (mb, mspec->data.array_data.num_elem);
-               mono_mb_emit_icall (mb, conv_to_icall (conv));
+               mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
                break;
        }
        case MONO_MARSHAL_CONV_ARRAY_BYVALARRAY: {
@@ -1940,8 +1954,9 @@ emit_struct_conv_full (MonoMethodBuilder *mb, MonoClass *klass, gboolean to_obje
                case MONO_MARSHAL_CONV_NONE: {
                        int t;
 
-                       if (ftype->byref || ftype->type == MONO_TYPE_I ||
-                           ftype->type == MONO_TYPE_U) {
+                       //XXX a byref field!?!? that's not allowed! and worse, it might miss a WB
+                       g_assert (!ftype->byref);
+                       if (ftype->type == MONO_TYPE_I || ftype->type == MONO_TYPE_U) {
                                mono_mb_emit_ldloc (mb, 1);
                                mono_mb_emit_ldloc (mb, 0);
                                mono_mb_emit_byte (mb, CEE_LDIND_I);
@@ -5109,7 +5124,7 @@ emit_marshal_string (EmitMarshalContext *m, int argnum, MonoType *t,
                        char *msg = g_strdup_printf ("string marshalling conversion %d not implemented", encoding);
                        mono_mb_emit_exception_marshal_directive (mb, msg);
                } else {
-                       mono_mb_emit_icall (mb, conv_to_icall (conv));
+                       mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
 
                        mono_mb_emit_stloc (mb, conv_arg);
                }
@@ -5150,10 +5165,11 @@ emit_marshal_string (EmitMarshalContext *m, int argnum, MonoType *t,
                        mono_mb_emit_icall (mb, mono_string_new_len_wrapper);
                        mono_mb_emit_byte (mb, CEE_STIND_REF);
                } else if (t->byref && (t->attrs & PARAM_ATTRIBUTE_OUT || !(t->attrs & PARAM_ATTRIBUTE_IN))) {
+                       int stind_op;
                        mono_mb_emit_ldarg (mb, argnum);
                        mono_mb_emit_ldloc (mb, conv_arg);
-                       mono_mb_emit_icall (mb, conv_to_icall (conv));
-                       mono_mb_emit_byte (mb, CEE_STIND_REF);
+                       mono_mb_emit_icall (mb, conv_to_icall (conv, &stind_op));
+                       mono_mb_emit_byte (mb, stind_op);
                        need_free = TRUE;
                }
 
@@ -5184,7 +5200,7 @@ emit_marshal_string (EmitMarshalContext *m, int argnum, MonoType *t,
                }
 
                mono_mb_emit_ldloc (mb, 0);
-               mono_mb_emit_icall (mb, conv_to_icall (conv));
+               mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
                mono_mb_emit_stloc (mb, 3);
 
                /* free the string */
@@ -5215,27 +5231,28 @@ emit_marshal_string (EmitMarshalContext *m, int argnum, MonoType *t,
                mono_mb_emit_ldarg (mb, argnum);
                if (t->byref)
                        mono_mb_emit_byte (mb, CEE_LDIND_I);
-               mono_mb_emit_icall (mb, conv_to_icall (conv));
+               mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
                mono_mb_emit_stloc (mb, conv_arg);
                break;
 
        case MARSHAL_ACTION_MANAGED_CONV_OUT:
                if (t->byref) {
                        if (conv_arg) {
+                               int stind_op;
                                mono_mb_emit_ldarg (mb, argnum);
                                mono_mb_emit_ldloc (mb, conv_arg);
-                               mono_mb_emit_icall (mb, conv_to_icall (conv));
-                               mono_mb_emit_byte (mb, CEE_STIND_I);
+                               mono_mb_emit_icall (mb, conv_to_icall (conv, &stind_op));
+                               mono_mb_emit_byte (mb, stind_op);
                        }
                }
                break;
 
        case MARSHAL_ACTION_MANAGED_CONV_RESULT:
-               if (conv_to_icall (conv) == mono_marshal_string_to_utf16)
+               if (conv_to_icall (conv, NULL) == mono_marshal_string_to_utf16)
                        /* We need to make a copy so the caller is able to free it */
                        mono_mb_emit_icall (mb, mono_marshal_string_to_utf16_copy);
                else
-                       mono_mb_emit_icall (mb, conv_to_icall (conv));
+                       mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
                mono_mb_emit_stloc (mb, 3);
                break;
 
@@ -5514,7 +5531,7 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
                                mono_mb_emit_stloc (mb, conv_arg);
                        } else {
                                mono_mb_emit_ldarg (mb, argnum);
-                               mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_DEL_FTN));
+                               mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_DEL_FTN, NULL));
                                mono_mb_emit_stloc (mb, conv_arg);
                        }
                } else if (klass == mono_defaults.stringbuilder_class) {
@@ -5544,7 +5561,7 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
                        if (t->byref)
                                mono_mb_emit_byte (mb, CEE_LDIND_I);
 
-                       mono_mb_emit_icall (mb, conv_to_icall (conv));
+                       mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
                        mono_mb_emit_stloc (mb, conv_arg);
                } else if (klass->blittable) {
                        mono_mb_emit_byte (mb, CEE_LDNULL);
@@ -5646,7 +5663,7 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
                                mono_mb_emit_ldarg (mb, argnum);
                                mono_mb_emit_ldloc (mb, conv_arg);
 
-                               mono_mb_emit_icall (mb, conv_to_icall (conv));
+                               mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
                        }
 
                        if (need_free) {
@@ -5662,7 +5679,7 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
                                mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
                                mono_mb_emit_op (mb, CEE_MONO_CLASSCONST, klass);
                                mono_mb_emit_ldloc (mb, conv_arg);
-                               mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_FTN_DEL));
+                               mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_FTN_DEL, NULL));
                                mono_mb_emit_byte (mb, CEE_STIND_REF);
                        }
                        break;
@@ -5745,7 +5762,7 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
                        mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
                        mono_mb_emit_op (mb, CEE_MONO_CLASSCONST, klass);
                        mono_mb_emit_ldloc (mb, 0);
-                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_FTN_DEL));
+                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_FTN_DEL, NULL));
                        mono_mb_emit_stloc (mb, 3);
                } else {
                        /* set src */
@@ -5795,7 +5812,7 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
                        mono_mb_emit_ldarg (mb, argnum);
                        if (t->byref)
                                mono_mb_emit_byte (mb, CEE_LDIND_I);
-                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_FTN_DEL));
+                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_FTN_DEL, NULL));
                        mono_mb_emit_stloc (mb, conv_arg);
                        break;
                }
@@ -5869,10 +5886,11 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
        case MARSHAL_ACTION_MANAGED_CONV_OUT:
                if (klass->delegate) {
                        if (t->byref) {
+                               int stind_op;
                                mono_mb_emit_ldarg (mb, argnum);
                                mono_mb_emit_ldloc (mb, conv_arg);
-                               mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_DEL_FTN));
-                               mono_mb_emit_byte (mb, CEE_STIND_I);
+                               mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_DEL_FTN, &stind_op));
+                               mono_mb_emit_byte (mb, stind_op);
                                break;
                        }
                }
@@ -5931,7 +5949,7 @@ emit_marshal_object (EmitMarshalContext *m, int argnum, MonoType *t,
 
        case MARSHAL_ACTION_MANAGED_CONV_RESULT:
                if (klass->delegate) {
-                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_DEL_FTN));
+                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_DEL_FTN, NULL));
                        mono_mb_emit_stloc (mb, 3);
                        break;
                }
@@ -6148,7 +6166,7 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
                        mono_mb_emit_ldarg (mb, argnum);
                        if (t->byref)
                                mono_mb_emit_byte (mb, CEE_LDIND_I);
-                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_ARRAY_LPARRAY));
+                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_CONV_ARRAY_LPARRAY, NULL));
                        mono_mb_emit_stloc (mb, conv_arg);
                } else {
                        MonoClass *eklass;
@@ -6228,12 +6246,13 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
                        /* Emit marshalling code */
 
                        if (is_string) {
+                               int stind_op;
                                mono_mb_emit_ldloc (mb, dest_ptr);
                                mono_mb_emit_ldloc (mb, src_var);
                                mono_mb_emit_ldloc (mb, index_var);
                                mono_mb_emit_byte (mb, CEE_LDELEM_REF);
-                               mono_mb_emit_icall (mb, conv_to_icall (conv));
-                               mono_mb_emit_byte (mb, CEE_STIND_I);
+                               mono_mb_emit_icall (mb, conv_to_icall (conv, &stind_op));
+                               mono_mb_emit_byte (mb, stind_op);
                        } else {
                                /* set the src_ptr */
                                mono_mb_emit_ldloc (mb, src_var);
@@ -6297,7 +6316,7 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
                                mono_mb_emit_byte (mb, CEE_CONV_OVF_I);
                                mono_mb_emit_op (mb, CEE_NEWARR, klass->element_class);
                                /* Store into argument */
-                               mono_mb_emit_byte (mb, CEE_STIND_I);
+                               mono_mb_emit_byte (mb, CEE_STIND_REF);
                        }
                }
 
@@ -6357,7 +6376,7 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
                                mono_mb_emit_ldloc (mb, src_ptr);
                                mono_mb_emit_byte (mb, CEE_LDIND_I);
 
-                               mono_mb_emit_icall (mb, conv_to_icall (conv));
+                               mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
 
                                if (need_free) {
                                        /* src */
@@ -6419,7 +6438,7 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
                        if (t->byref)
                                mono_mb_emit_byte (mb, CEE_LDIND_REF);
                        mono_mb_emit_ldloc (mb, conv_arg);
-                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_FREE_LPARRAY));
+                       mono_mb_emit_icall (mb, conv_to_icall (MONO_MARSHAL_FREE_LPARRAY, NULL));
                }
 
                break;
@@ -6594,7 +6613,7 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
                        mono_mb_emit_ldloc (mb, src_ptr);
                        mono_mb_emit_byte (mb, CEE_LDIND_I);
 
-                       mono_mb_emit_icall (mb, conv_to_icall (conv));
+                       mono_mb_emit_icall (mb, conv_to_icall (conv, NULL));
                        mono_mb_emit_byte (mb, CEE_STELEM_REF);
                }
                else {
@@ -6702,6 +6721,7 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
 
                /* Emit marshalling code */
                if (is_string) {
+                       int stind_op;
                        g_assert (conv != MONO_MARSHAL_CONV_INVALID);
 
                        /* dest */
@@ -6713,8 +6733,8 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
 
                        mono_mb_emit_byte (mb, CEE_LDELEM_REF);
 
-                       mono_mb_emit_icall (mb, conv_to_icall (conv));
-                       mono_mb_emit_byte (mb, CEE_STIND_I);
+                       mono_mb_emit_icall (mb, conv_to_icall (conv, &stind_op));
+                       mono_mb_emit_byte (mb, stind_op);
                }
                else {
                        char *msg = g_strdup ("Marshalling of non-string and non-blittable arrays to managed code is not implemented.");
@@ -6799,6 +6819,7 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
 
                /* Emit marshalling code */
                if (is_string) {
+                       int stind_op;
                        g_assert (conv != MONO_MARSHAL_CONV_INVALID);
 
                        /* dest */
@@ -6810,8 +6831,8 @@ emit_marshal_array (EmitMarshalContext *m, int argnum, MonoType *t,
 
                        mono_mb_emit_byte (mb, CEE_LDELEM_REF);
 
-                       mono_mb_emit_icall (mb, conv_to_icall (conv));
-                       mono_mb_emit_byte (mb, CEE_STIND_I);
+                       mono_mb_emit_icall (mb, conv_to_icall (conv, &stind_op));
+                       mono_mb_emit_byte (mb, stind_op);
                }
                else {
                        char *msg = g_strdup ("Marshalling of non-string arrays to managed code is not implemented.");