Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] pinvoke improvements part 2 #98250

Merged
merged 21 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/mono/mono/metadata/class-init.c
Original file line number Diff line number Diff line change
Expand Up @@ -2272,6 +2272,7 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_
}

size = mono_type_size (field->type, &align);
// keep in sync with marshal.c mono_marshal_load_type_info
if (m_class_is_inlinearray (klass)) {
// Limit the max size of array instance to 1MiB
const guint32 struct_max_size = 1024 * 1024;
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/icall.c
Original file line number Diff line number Diff line change
Expand Up @@ -3028,7 +3028,7 @@ ves_icall_RuntimeType_GetNamespace (MonoQCallTypeHandle type_handle, MonoObjectH

MonoClass *klass = mono_class_from_mono_type_internal (type);
MonoClass *elem;
while (!m_class_is_enumtype (klass) &&
while (!m_class_is_enumtype (klass) &&
!mono_class_is_nullable (klass) &&
(klass != (elem = m_class_get_element_class (klass))))
klass = elem;
Expand Down
23 changes: 15 additions & 8 deletions src/mono/mono/metadata/marshal.c
Original file line number Diff line number Diff line change
Expand Up @@ -3294,7 +3294,7 @@ mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t,
return mono_emit_disabled_marshal (m, argnum, t, spec, conv_arg, conv_arg_type, action);

return mono_component_marshal_ilgen()->emit_marshal_ilgen(m, argnum, t, spec, conv_arg, conv_arg_type, action, get_marshal_cb());
}
}

static void
mono_marshal_set_callconv_for_type(MonoType *type, MonoMethodSignature *csig, gboolean *skip_gc_trans /*out*/)
Expand Down Expand Up @@ -3577,7 +3577,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,

if (G_UNLIKELY (pinvoke && mono_method_has_unmanaged_callers_only_attribute (method))) {
/*
* In AOT mode and embedding scenarios, it is possible that the icall is not registered in the runtime doing the AOT compilation.
* In AOT mode and embedding scenarios, it is possible that the icall is not registered in the runtime doing the AOT compilation.
* Emit a wrapper that throws a NotSupportedException.
*/
get_marshal_cb ()->mb_emit_exception (mb, "System", "NotSupportedException", "Method canot be marked with both DllImportAttribute and UnmanagedCallersOnlyAttribute");
Expand Down Expand Up @@ -3757,7 +3757,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
}

goto leave;

emit_exception_for_error:
mono_error_cleanup (emitted_error);
info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_NONE);
Expand Down Expand Up @@ -5231,7 +5231,7 @@ mono_marshal_get_unsafe_accessor_wrapper (MonoMethod *accessor_method, MonoUnsaf

if (member_name == NULL && kind != MONO_UNSAFE_ACCESSOR_CTOR)
member_name = accessor_method->name;

/*
* Check cache
*/
Expand Down Expand Up @@ -5827,11 +5827,20 @@ mono_marshal_load_type_info (MonoClass* klass)
continue;
}

size = mono_marshal_type_size (field->type, info->fields [j].mspec,
&align, TRUE, m_class_is_unicode (klass));

// Keep in sync with class-init.c mono_class_layout_fields
if (m_class_is_inlinearray (klass)) {
// Limit the max size of array instance to 1MiB
const int struct_max_size = 1024 * 1024;
size *= m_class_inlinearray_value (klass);
g_assert ((size > 0) && (size <= struct_max_size));
}

switch (layout) {
case TYPE_ATTRIBUTE_AUTO_LAYOUT:
case TYPE_ATTRIBUTE_SEQUENTIAL_LAYOUT:
size = mono_marshal_type_size (field->type, info->fields [j].mspec,
&align, TRUE, m_class_is_unicode (klass));
align = m_class_get_packing_size (klass) ? MIN (m_class_get_packing_size (klass), align): align;
min_align = MAX (align, min_align);
info->fields [j].offset = info->native_size;
Expand All @@ -5840,8 +5849,6 @@ mono_marshal_load_type_info (MonoClass* klass)
info->native_size = info->fields [j].offset + size;
break;
case TYPE_ATTRIBUTE_EXPLICIT_LAYOUT:
size = mono_marshal_type_size (field->type, info->fields [j].mspec,
&align, TRUE, m_class_is_unicode (klass));
min_align = MAX (align, min_align);
info->fields [j].offset = m_field_get_offset (field) - MONO_ABI_SIZEOF (MonoObject);
info->native_size = MAX (info->native_size, info->fields [j].offset + size);
Expand Down
53 changes: 41 additions & 12 deletions src/mono/mono/mini/aot-runtime-wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@
#ifdef HOST_WASM

static char
type_to_c (MonoType *t)
type_to_c (MonoType *t, gboolean *is_byref_return)
{
g_assert (t);

if (is_byref_return)
*is_byref_return = 0;
if (m_type_is_byref (t))
return 'I';

Expand Down Expand Up @@ -48,7 +52,7 @@ type_to_c (MonoType *t)
return 'L';
case MONO_TYPE_VOID:
return 'V';
case MONO_TYPE_VALUETYPE:
case MONO_TYPE_VALUETYPE: {
if (m_class_is_enumtype (t->data.klass)) {
t = mono_class_enum_basetype_internal (t->data.klass);
goto handle_enum;
Expand All @@ -60,13 +64,27 @@ type_to_c (MonoType *t)
// FIXME: Handle the scenario where there are fields of struct types that contain no members
MonoType *scalar_vtype;
if (mini_wasm_is_scalar_vtype (t, &scalar_vtype))
return type_to_c (scalar_vtype);
return type_to_c (scalar_vtype, NULL);

if (is_byref_return)
*is_byref_return = 1;

return 'I';
case MONO_TYPE_GENERICINST:
if (m_class_is_valuetype (t->data.klass))
}
case MONO_TYPE_GENERICINST: {
if (m_class_is_valuetype (t->data.klass)) {
MonoType *scalar_vtype;
if (mini_wasm_is_scalar_vtype (t, &scalar_vtype))
return type_to_c (scalar_vtype, NULL);

if (is_byref_return)
*is_byref_return = 1;

return 'S';
}

return 'I';
}
default:
g_warning ("CANT TRANSLATE %s", mono_type_full_name (t));
return 'X';
Expand Down Expand Up @@ -140,18 +158,29 @@ gpointer
mono_wasm_get_interp_to_native_trampoline (MonoMethodSignature *sig)
{
char cookie [32];
int c_count;
int c_count, offset = 1;
gboolean is_byref_return = 0;

memset (cookie, 0, 32);
cookie [0] = type_to_c (sig->ret, &is_byref_return);

c_count = sig->param_count + sig->hasthis + 1;
c_count = sig->param_count + sig->hasthis + is_byref_return + 1;
g_assert (c_count < sizeof (cookie)); //ensure we don't overflow the local

cookie [0] = type_to_c (sig->ret);
if (sig->hasthis)
cookie [1] = 'I';
if (is_byref_return) {
cookie[0] = 'V';
// return value address goes in arg0
cookie[1] = 'I';
offset += 1;
}
if (sig->hasthis) {
// thisptr goes in arg0/arg1 depending on return type
cookie [offset] = 'I';
offset += 1;
}
for (int i = 0; i < sig->param_count; ++i) {
cookie [1 + sig->hasthis + i] = type_to_c (sig->params [i]);
cookie [offset + i] = type_to_c (sig->params [i], NULL);
}
cookie [c_count] = 0;

void *p = mono_wasm_interp_to_native_callback (cookie);
if (!p)
Expand Down
54 changes: 48 additions & 6 deletions src/mono/mono/mini/interp/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1340,7 +1340,10 @@ typedef enum {
PINVOKE_ARG_R8 = 3,
PINVOKE_ARG_R4 = 4,
PINVOKE_ARG_VTYPE = 5,
PINVOKE_ARG_SCALAR_VTYPE = 6
PINVOKE_ARG_SCALAR_VTYPE = 6,
// This isn't ifdefed so it's easier to write code that handles it without sprinkling
// 800 ifdefs in this file
PINVOKE_ARG_WASM_VALUETYPE_RESULT = 7,
} PInvokeArgType;

typedef struct {
Expand Down Expand Up @@ -1436,6 +1439,7 @@ get_build_args_from_sig_info (MonoMemoryManager *mem_manager, MonoMethodSignatur
ilen++;
break;
case MONO_TYPE_GENERICINST: {
// FIXME: Should mini_wasm_is_scalar_vtype stuff go in here?
MonoClass *container_class = type->data.generic_class->container_class;
type = m_class_get_byval_arg (container_class);
goto retry;
Expand Down Expand Up @@ -1473,11 +1477,32 @@ get_build_args_from_sig_info (MonoMemoryManager *mem_manager, MonoMethodSignatur
case MONO_TYPE_CLASS:
case MONO_TYPE_OBJECT:
case MONO_TYPE_STRING:
info->ret_pinvoke_type = PINVOKE_ARG_INT;
break;
#if SIZEOF_VOID_P == 8
case MONO_TYPE_I8:
case MONO_TYPE_U8:
#endif
info->ret_pinvoke_type = PINVOKE_ARG_INT;
break;
#if SIZEOF_VOID_P == 4
case MONO_TYPE_I8:
case MONO_TYPE_U8:
info->ret_pinvoke_type = PINVOKE_ARG_INT;
break;
#endif
case MONO_TYPE_VALUETYPE:
case MONO_TYPE_GENERICINST:
info->ret_pinvoke_type = PINVOKE_ARG_INT;
#ifdef HOST_WASM
// This ISSTRUCT check is important, because the type could be an enum
if (MONO_TYPE_ISSTRUCT (info->ret_mono_type)) {
// The return type was already filtered previously, so if we get here
// we're returning a struct byref instead of as a scalar
info->ret_pinvoke_type = PINVOKE_ARG_WASM_VALUETYPE_RESULT;
info->ilen++;
}
#endif
break;
case MONO_TYPE_R4:
case MONO_TYPE_R8:
Expand All @@ -1503,6 +1528,15 @@ build_args_from_sig (InterpMethodArguments *margs, MonoMethodSignature *sig, Bui
margs->ilen = info->ilen;
margs->flen = info->flen;

size_t int_i = 0;
size_t int_f = 0;

if (info->ret_pinvoke_type == PINVOKE_ARG_WASM_VALUETYPE_RESULT) {
// Allocate an empty arg0 for the address of the return value
// info->ilen was already increased earlier
int_i++;
}

if (margs->ilen > 0) {
if (margs->ilen <= 8)
margs->iargs = margs->iargs_buf;
Expand All @@ -1517,9 +1551,6 @@ build_args_from_sig (InterpMethodArguments *margs, MonoMethodSignature *sig, Bui
margs->fargs = g_malloc0 (sizeof (double) * margs->flen);
}

size_t int_i = 0;
size_t int_f = 0;

for (int i = 0; i < sig->param_count; i++) {
guint32 offset = get_arg_offset (frame->imethod, sig, i);
stackval *sp_arg = STACK_ADD_BYTES (frame->stack, offset);
Expand Down Expand Up @@ -1578,6 +1609,15 @@ build_args_from_sig (InterpMethodArguments *margs, MonoMethodSignature *sig, Bui
}

switch (info->ret_pinvoke_type) {
case PINVOKE_ARG_WASM_VALUETYPE_RESULT:
// We pass the return value address in arg0 so fill it in, we already
// reserved space for it earlier.
g_assert (frame->retval);
margs->iargs[0] = (gpointer*)frame->retval;
// The return type is void so retval should be NULL
margs->retval = NULL;
margs->is_float_ret = 0;
break;
case PINVOKE_ARG_INT:
margs->retval = (gpointer*)frame->retval;
margs->is_float_ret = 0;
Expand Down Expand Up @@ -1795,8 +1835,10 @@ ves_pinvoke_method (
g_free (ccontext.stack);
#else
// Only the vt address has been returned, we need to copy the entire content on interp stack
if (!context->has_resume_state && MONO_TYPE_ISSTRUCT (call_info->ret_mono_type))
stackval_from_data (call_info->ret_mono_type, frame.retval, (char*)frame.retval->data.p, sig->pinvoke && !sig->marshalling_disabled);
if (!context->has_resume_state && MONO_TYPE_ISSTRUCT (call_info->ret_mono_type)) {
if (call_info->ret_pinvoke_type != PINVOKE_ARG_WASM_VALUETYPE_RESULT)
stackval_from_data (call_info->ret_mono_type, frame.retval, (char*)frame.retval->data.p, sig->pinvoke && !sig->marshalling_disabled);
}

if (margs.iargs != margs.iargs_buf)
g_free (margs.iargs);
Expand Down
17 changes: 14 additions & 3 deletions src/mono/mono/mini/mini-wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,23 @@ get_storage (MonoType *type, MonoType **etype, gboolean is_return)
case MONO_TYPE_R8:
return ArgOnStack;

case MONO_TYPE_GENERICINST:
case MONO_TYPE_GENERICINST: {
if (!mono_type_generic_inst_is_valuetype (type))
return ArgOnStack;

if (mini_is_gsharedvt_variable_type (type))
return ArgGsharedVTOnStack;
/* fall through */

if (mini_wasm_is_scalar_vtype (type, etype))
return ArgVtypeAsScalar;

return is_return ? ArgValuetypeAddrInIReg : ArgValuetypeAddrOnStack;
}
case MONO_TYPE_VALUETYPE:
case MONO_TYPE_TYPEDBYREF: {
if (mini_wasm_is_scalar_vtype (type, etype))
return ArgVtypeAsScalar;

return is_return ? ArgValuetypeAddrInIReg : ArgValuetypeAddrOnStack;
}
case MONO_TYPE_VAR:
Expand Down Expand Up @@ -771,7 +777,12 @@ mini_wasm_is_scalar_vtype (MonoType *type, MonoType **etype)
if (nfields > 1)
return FALSE;
MonoType *t = mini_get_underlying_type (field->type);
if (MONO_TYPE_ISSTRUCT (t)) {
int align, field_size = mono_type_size (t, &align);
// inlinearray and fixed both work by having a single field that is bigger than its element type.
kg marked this conversation as resolved.
Show resolved Hide resolved
// we also don't want to scalarize a struct that has padding in its metadata, even if it would fit.
if (field_size != size) {
return FALSE;
} else if (MONO_TYPE_ISSTRUCT (t)) {
if (!mini_wasm_is_scalar_vtype (t, etype))
return FALSE;
} else if (!((MONO_TYPE_IS_PRIMITIVE (t) || MONO_TYPE_IS_REFERENCE (t) || MONO_TYPE_IS_POINTER (t)))) {
Expand Down
Loading