Skip to content

Commit

Permalink
[mono] Add DISABLE_NONBLITTABLE for non-blittable marshaling (#46444)
Browse files Browse the repository at this point in the history 
This flag is not set by default on any platforms, but long-term it should be useful on wasm with the pinvoke generator work being done by the interop team.

This flag reduces around 13k in dotnet.wasm as of today (12/29/2020). However, that is a fairly low-end measurement because this PR only focused on the pinvoke marshaling rather than the full functionality. The structure of our marshaling callbacks means that even with icall linking, a lot of marshaling code will be kept around and unused, including all the string marshaling. 

To solve this, we can either give up on the ilgen functionality being in a separate library so it gets linked our normally or we can just have this flag cover marshaling functionality as a whole. I've split that part off into a followup PR since it's more likely to be contested, but I implemented the latter option (and also disable things like certain JIT icalls for the same reason). We also will want analyzers set up before this is useful, so hopefully this solution is fine in combination with an extra analyzer for the marshaling methods in question?
  • Loading branch information
@CoffeeFlux
CoffeeFlux committed Jan 11, 2021
1 parent 6b4c1ad commit 55211d2
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 139 deletions.
3 changes: 3 additions & 0 deletions src/mono/cmake/config.h.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,9 @@
/* Some VES is available at runtime */
#cmakedefine ENABLE_ILGEN 1

/* Disable non-blittable marshalling */
#cmakedefine DISABLE_NONBLITTABLE

/* Disable SIMD intrinsics related optimizations. */
#cmakedefine DISABLE_SIMD 1

Expand Down
151 changes: 90 additions & 61 deletions src/mono/mono/metadata/marshal-ilgen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2450,7 +2450,6 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
{
MonoMethodBuilder *mb = m->mb;
MonoClass *klass = mono_class_from_mono_type_internal (t);
gboolean need_convert, need_free;
MonoMarshalNative encoding;

encoding = mono_marshal_get_string_encoding (m->piinfo, spec);
Expand All @@ -2471,6 +2470,10 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
mono_mb_emit_icall_id (mb, conv_to_icall (MONO_MARSHAL_CONV_ARRAY_LPARRAY, NULL));
mono_mb_emit_stloc (mb, conv_arg);
} else {
#ifdef DISABLE_NONBLITTABLE
char *msg = g_strdup ("Non-blittable marshalling conversion is disabled");
mono_mb_emit_exception_marshal_directive (mb, msg);
#else
guint32 label1, label2, label3;
int index_var, src_var, dest_ptr, esize;
MonoMarshalConv conv;
Expand Down Expand Up @@ -2582,11 +2585,14 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
}

mono_mb_patch_branch (mb, label1);
#endif
}

break;

case MARSHAL_ACTION_CONV_OUT:
case MARSHAL_ACTION_CONV_OUT: {
#ifndef DISABLE_NONBLITTABLE
gboolean need_convert, need_free;
/* Unicode character arrays are implicitly marshalled as [Out] under MS.NET */
need_convert = ((eklass == mono_defaults.char_class) && (encoding == MONO_NATIVE_LPWSTR)) || (eklass == mono_class_try_get_stringbuilder_class ()) || (t->attrs & PARAM_ATTRIBUTE_OUT);
need_free = mono_marshal_need_free (m_class_get_byval_arg (eklass), m->piinfo, spec);
Expand Down Expand Up @@ -2725,6 +2731,7 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
mono_mb_patch_branch (mb, label1);
mono_mb_patch_branch (mb, label3);
}
#endif

if (m_class_is_blittable (eklass)) {
/* free memory allocated (if any) by MONO_MARSHAL_CONV_ARRAY_LPARRAY */
Expand All @@ -2737,6 +2744,7 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
}

break;
}

case MARSHAL_ACTION_PUSH:
if (t->byref)
Expand Down Expand Up @@ -2810,16 +2818,20 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,

/* FIXME: Optimize blittable case */

#ifndef DISABLE_NONBLITTABLE
if (eklass == mono_defaults.string_class) {
is_string = TRUE;
gboolean need_free;
conv = mono_marshal_get_ptr_to_string_conv (m->piinfo, spec, &need_free);
}
else if (eklass == mono_class_try_get_stringbuilder_class ()) {
is_string = TRUE;
gboolean need_free;
conv = mono_marshal_get_ptr_to_stringbuilder_conv (m->piinfo, spec, &need_free);
}
else
conv = MONO_MARSHAL_CONV_INVALID;
#endif

mono_marshal_load_type_info (eklass);

Expand Down Expand Up @@ -2902,7 +2914,12 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
mono_mb_patch_branch (mb, label1);
break;
}

#ifdef DISABLE_NONBLITTABLE
else {
char *msg = g_strdup ("Non-blittable marshalling conversion is disabled");
mono_mb_emit_exception_marshal_directive (mb, msg);
}
#else
/* Emit marshalling loop */
index_var = mono_mb_add_local (mb, int_type);
mono_mb_emit_byte (mb, CEE_LDC_I4_0);
Expand Down Expand Up @@ -2939,6 +2956,7 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,

mono_mb_patch_branch (mb, label1);
mono_mb_patch_branch (mb, label3);
#endif

break;
}
Expand Down Expand Up @@ -2972,6 +2990,7 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,

/* FIXME: Optimize blittable case */

#ifndef DISABLE_NONBLITTABLE
if (eklass == mono_defaults.string_class) {
is_string = TRUE;
conv = mono_marshal_get_string_to_ptr_conv (m->piinfo, spec);
Expand All @@ -2982,6 +3001,7 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
}
else
conv = MONO_MARSHAL_CONV_INVALID;
#endif

mono_marshal_load_type_info (eklass);

Expand Down Expand Up @@ -3018,6 +3038,7 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
break;
}

#ifndef DISABLE_NONBLITTABLE
/* Emit marshalling loop */
index_var = mono_mb_add_local (mb, int_type);
mono_mb_emit_byte (mb, CEE_LDC_I4_0);
Expand Down Expand Up @@ -3058,10 +3079,12 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,

mono_mb_patch_branch (mb, label1);
mono_mb_patch_branch (mb, label3);
#endif

break;
}
case MARSHAL_ACTION_MANAGED_CONV_RESULT: {
#ifndef DISABLE_NONBLITTABLE
guint32 label1, label2, label3;
int index_var, src, dest, esize;
MonoMarshalConv conv = MONO_MARSHAL_CONV_INVALID;
Expand Down Expand Up @@ -3153,6 +3176,7 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,

mono_mb_patch_branch (mb, label3);
mono_mb_patch_branch (mb, label1);
#endif
break;
}
default:
Expand All @@ -3161,6 +3185,62 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
return conv_arg;
}

static int
emit_marshal_ptr_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
MonoMarshalSpec *spec, int conv_arg,
MonoType **conv_arg_type, MarshalAction action)
{
MonoMethodBuilder *mb = m->mb;

switch (action) {
case MARSHAL_ACTION_CONV_IN:
/* MS seems to allow this in some cases, ie. bxc #158 */
/*
if (MONO_TYPE_ISSTRUCT (t->data.type) && !mono_class_from_mono_type_internal (t->data.type)->blittable) {
char *msg = g_strdup_printf ("Can not marshal 'parameter #%d': Pointers can not reference marshaled structures. Use byref instead.", argnum + 1);
mono_mb_emit_exception_marshal_directive (m->mb, msg);
}
*/
break;

case MARSHAL_ACTION_PUSH:
mono_mb_emit_ldarg (mb, argnum);
break;

case MARSHAL_ACTION_CONV_RESULT:
/* no conversions necessary */
mono_mb_emit_stloc (mb, 3);
break;

default:
break;
}
return conv_arg;
}

static int
emit_marshal_scalar_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
MonoMarshalSpec *spec, int conv_arg,
MonoType **conv_arg_type, MarshalAction action)
{
MonoMethodBuilder *mb = m->mb;

switch (action) {
case MARSHAL_ACTION_PUSH:
mono_mb_emit_ldarg (mb, argnum);
break;

case MARSHAL_ACTION_CONV_RESULT:
/* no conversions necessary */
mono_mb_emit_stloc (mb, 3);
break;

default:
break;
}
return conv_arg;
}

static int
emit_marshal_boolean_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
MonoMarshalSpec *spec,
Expand Down Expand Up @@ -3312,39 +3392,6 @@ emit_marshal_boolean_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
return conv_arg;
}

static int
emit_marshal_ptr_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
MonoMarshalSpec *spec, int conv_arg,
MonoType **conv_arg_type, MarshalAction action)
{
MonoMethodBuilder *mb = m->mb;

switch (action) {
case MARSHAL_ACTION_CONV_IN:
/* MS seems to allow this in some cases, ie. bxc #158 */
/*
if (MONO_TYPE_ISSTRUCT (t->data.type) && !mono_class_from_mono_type_internal (t->data.type)->blittable) {
char *msg = g_strdup_printf ("Can not marshal 'parameter #%d': Pointers can not reference marshaled structures. Use byref instead.", argnum + 1);
mono_mb_emit_exception_marshal_directive (m->mb, msg);
}
*/
break;

case MARSHAL_ACTION_PUSH:
mono_mb_emit_ldarg (mb, argnum);
break;

case MARSHAL_ACTION_CONV_RESULT:
/* no conversions necessary */
mono_mb_emit_stloc (mb, 3);
break;

default:
break;
}
return conv_arg;
}

static int
emit_marshal_char_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
MonoMarshalSpec *spec, int conv_arg,
Expand All @@ -3371,29 +3418,6 @@ emit_marshal_char_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
return conv_arg;
}

static int
emit_marshal_scalar_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
MonoMarshalSpec *spec, int conv_arg,
MonoType **conv_arg_type, MarshalAction action)
{
MonoMethodBuilder *mb = m->mb;

switch (action) {
case MARSHAL_ACTION_PUSH:
mono_mb_emit_ldarg (mb, argnum);
break;

case MARSHAL_ACTION_CONV_RESULT:
/* no conversions necessary */
mono_mb_emit_stloc (mb, 3);
break;

default:
break;
}
return conv_arg;
}

static void
emit_virtual_stelemref_ilgen (MonoMethodBuilder *mb, const char **param_names, MonoStelemrefKind kind)
{
Expand Down Expand Up @@ -6806,10 +6830,11 @@ mono_marshal_ilgen_init (void)
MonoMarshalCallbacks cb;
cb.version = MONO_MARSHAL_CALLBACKS_VERSION;
cb.emit_marshal_array = emit_marshal_array_ilgen;
cb.emit_marshal_boolean = emit_marshal_boolean_ilgen;
cb.emit_marshal_ptr = emit_marshal_ptr_ilgen;
cb.emit_marshal_char = emit_marshal_char_ilgen;
cb.emit_marshal_scalar = emit_marshal_scalar_ilgen;
#ifndef DISABLE_NONBLITTABLE
cb.emit_marshal_boolean = emit_marshal_boolean_ilgen;
cb.emit_marshal_char = emit_marshal_char_ilgen;
cb.emit_marshal_custom = emit_marshal_custom_ilgen;
cb.emit_marshal_asany = emit_marshal_asany_ilgen;
cb.emit_marshal_vtype = emit_marshal_vtype_ilgen;
Expand All @@ -6818,6 +6843,7 @@ mono_marshal_ilgen_init (void)
cb.emit_marshal_handleref = emit_marshal_handleref_ilgen;
cb.emit_marshal_object = emit_marshal_object_ilgen;
cb.emit_marshal_variant = emit_marshal_variant_ilgen;
#endif
cb.emit_castclass = emit_castclass_ilgen;
cb.emit_struct_to_ptr = emit_struct_to_ptr_ilgen;
cb.emit_ptr_to_struct = emit_ptr_to_struct_ilgen;
Expand Down Expand Up @@ -6847,5 +6873,8 @@ mono_marshal_ilgen_init (void)
cb.mb_emit_exception = mb_emit_exception_ilgen;
cb.mb_emit_exception_for_error = mb_emit_exception_for_error_ilgen;
cb.mb_emit_byte = mb_emit_byte_ilgen;
#ifdef DISABLE_NONBLITTABLE
mono_marshal_noilgen_init_blittable (&cb);
#endif
mono_install_marshal_callbacks (&cb);
}
4 changes: 4 additions & 0 deletions src/mono/mono/metadata/marshal-internals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <config.h>
#include <glib.h>
#include <mono/metadata/object-internals.h>
#include <mono/metadata/marshal.h>

MonoObjectHandle
mono_marshal_xdomain_copy_value_handle (MonoObjectHandle val, MonoError *error);
Expand Down Expand Up @@ -43,4 +44,7 @@ typedef enum {
void
mono_marshal_noilgen_init (void);

void
mono_marshal_noilgen_init_blittable (MonoMarshalCallbacks *cb);

#endif /* __MONO_METADATA_MARSHAL_INTERNALS_H__ */
Loading

0 comments on commit 55211d2

Please sign in to comment.