From 0eb2f4ee94fa7eafbc0eed447ef0ce284b1bb24d Mon Sep 17 00:00:00 2001 From: Esme Povirk Date: Fri, 21 Jul 2023 12:28:30 -0500 Subject: [PATCH 1/4] Add testcase for safearray managed-out marshaling. --- mono/tests/cominterop.cs | 24 ++++++++++++- mono/tests/libtest.c | 75 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/mono/tests/cominterop.cs b/mono/tests/cominterop.cs index fe2fb55b7a07..ac51082e1c25 100644 --- a/mono/tests/cominterop.cs +++ b/mono/tests/cominterop.cs @@ -376,6 +376,9 @@ public static extern int mono_test_marshal_safearray_mixed ( [DllImport("libtest")] public static extern int mono_test_marshal_safearray_in_ccw([MarshalAs (UnmanagedType.Interface)] ITest itest); + [DllImport("libtest")] + public static extern int mono_test_marshal_safearray_out_ccw([MarshalAs (UnmanagedType.Interface)] ITest itest); + [DllImport("libtest")] public static extern int mono_test_marshal_lparray_out_ccw([MarshalAs (UnmanagedType.Interface)] ITest itest); @@ -972,6 +975,8 @@ public static int Main () return 97; if (mono_test_marshal_lparray_out_ccw(test) != 0) return 98; + if (mono_test_marshal_safearray_out_ccw(test) != 0) + return 99; } #endregion // SafeArray Tests @@ -1192,6 +1197,8 @@ ITest Test [MethodImplAttribute (MethodImplOptions.InternalCall, MethodCodeType = MethodCodeType.Runtime)] [PreserveSig ()] int PointClassWithoutExplicitLayout ([MarshalAs(UnmanagedType.Interface), In, Out] PointWithoutExplicitLayout pt); + [MethodImplAttribute (MethodImplOptions.InternalCall, MethodCodeType = MethodCodeType.Runtime)] + int[] SafeArrayOut (); } [ComImport ()] @@ -1330,6 +1337,8 @@ public virtual extern ITest Test [MethodImplAttribute (MethodImplOptions.InternalCall, MethodCodeType = MethodCodeType.Runtime)] [PreserveSig ()] public virtual extern int PointClassWithoutExplicitLayout ([MarshalAs(UnmanagedType.Interface), In, Out] PointWithoutExplicitLayout pt); + [MethodImplAttribute (MethodImplOptions.InternalCall, MethodCodeType = MethodCodeType.Runtime)] + public virtual extern int[] SafeArrayOut (); } [System.Runtime.InteropServices.GuidAttribute ("00000000-0000-0000-0000-000000000002")] @@ -1721,6 +1730,11 @@ public int PointClassWithoutExplicitLayout (PointWithoutExplicitLayout pt) { return 0; } + + public int[] SafeArrayOut () + { + return new int[]{1, 2, 3, 4, 5}; + } } [ComVisible (true)] @@ -2008,8 +2022,16 @@ public static int TestITest (ITest itest) if (itest.PointClassWithoutExplicitLayout (pt3) != 0) return 1; - if (isWindows) + if (isWindows) { + int[] safeArray = itest.SafeArrayOut(); + if (safeArray.Length != 5) + return 1; + for (int i=0; i<5; i++) + if (safeArray[i] != i+1) + return 1; + itest.ArrayIn2 (new object[] { "Test", 2345 }); + } } catch (Exception ex) { return 1; diff --git a/mono/tests/libtest.c b/mono/tests/libtest.c index 5867e8b3cd99..03fc776c35cb 100644 --- a/mono/tests/libtest.c +++ b/mono/tests/libtest.c @@ -3585,6 +3585,7 @@ typedef struct point* (STDCALL *PointClassRet)(MonoComObject* pUnk); void (STDCALL *PointClassWithIface)(MonoComObject* pUnk, point *pt); int (STDCALL *PointClassWithoutExplicitLayout)(MonoComObject* pUnk, MonoComObject *obj); + int (STDCALL *SafeArrayOut)(MonoComObject* pUnk, gpointer *safearray); } MonoIUnknown; struct MonoComObject @@ -3846,6 +3847,29 @@ PointClassWithoutExplicitLayout(MonoComObject* pUnk, MonoComObject *obj) return 0; } +LIBTEST_API int STDCALL +SafeArrayOut(MonoComObject* pUnk, gpointer *safearray) +{ +#ifdef WIN32 + SAFEARRAY *sa; + SAFEARRAYBOUND rgsabound = {5, 0}; + LONG idx; + INT value; + sa = SafeArrayCreate(VT_I4, 1, &rgsabound); + if (!sa) + return 1; + for (idx = 0; idx < 5; idx++) + { + value = idx + 1; + SafeArrayPutElement(sa, &idx, &value); + } + *safearray = sa; + return 0; +#else + return 0x80004005; +#endif +} + static void create_com_object (MonoComObject** pOut); LIBTEST_API int STDCALL @@ -3891,6 +3915,7 @@ static void create_com_object (MonoComObject** pOut) (*pOut)->vtbl->PointClassRet = PointClassRet; (*pOut)->vtbl->PointClassWithIface = PointClassWithIface; (*pOut)->vtbl->PointClassWithoutExplicitLayout = PointClassWithoutExplicitLayout; + (*pOut)->vtbl->SafeArrayOut = SafeArrayOut; } static MonoComObject* same_object = NULL; @@ -6332,6 +6357,56 @@ mono_test_marshal_safearray_in_ccw(MonoComObject *pUnk) return ret; } +LIBTEST_API int STDCALL +mono_test_marshal_safearray_out_ccw(MonoComObject *pUnk) +{ + SAFEARRAY *sa; + int ret; + + ret = pUnk->vtbl->SafeArrayOut(pUnk, (gpointer*)&sa); + if (!ret) { + if (SafeArrayGetDim(sa) != 1) + ret = 1; + + if (!ret) { + VARTYPE vt; + ret = SafeArrayGetVartype(sa, &vt); + if (!ret && vt != VT_I4) + ret = 2; + } + + if (!ret) { + LONG lbound; + ret = SafeArrayGetLBound(sa, 1, &lbound); + if (!ret && lbound != 0) + ret = 3; + } + + if (!ret) { + LONG ubound; + ret = SafeArrayGetUBound(sa, 1, &ubound); + if (!ret && ubound != 4) + ret = 4; + } + + if (!ret) { + ret = SafeArrayLock(sa); + if (!ret) { + int i; + for (i=0; i<5; i++) + if (((INT*)sa->pvData)[i] != i+1) + ret = 5 + i; + + SafeArrayUnlock(sa); + } + } + + SafeArrayDestroy(sa); + } + + return ret; +} + LIBTEST_API int STDCALL mono_test_marshal_lparray_out_ccw(MonoComObject *pUnk) { From fbf9c54368d78f5c15d506164b0ecbc3173f2e8b Mon Sep 17 00:00:00 2001 From: Esme Povirk Date: Mon, 14 Aug 2023 13:06:45 -0500 Subject: [PATCH 2/4] Move mono_cominterop_emit_marshal_safearray calls into mono_emit_marshal. --- mono/metadata/cominterop.c | 5 +++++ mono/metadata/marshal-ilgen.c | 4 ---- mono/metadata/marshal.c | 13 ++++++++++--- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/mono/metadata/cominterop.c b/mono/metadata/cominterop.c index f1a508c2646e..95443f069cbd 100644 --- a/mono/metadata/cominterop.c +++ b/mono/metadata/cominterop.c @@ -4252,6 +4252,11 @@ mono_cominterop_emit_marshal_safearray (EmitMarshalContext *m, int argnum, MonoT if ((t->attrs & (PARAM_ATTRIBUTE_IN | PARAM_ATTRIBUTE_OUT)) == PARAM_ATTRIBUTE_OUT) break; + MonoType *object_type = mono_get_object_type (); + MonoType *int_type = mono_get_int_type (); + *conv_arg_type = int_type; + conv_arg = mono_mb_add_local (mb, object_type); + /* conv = mono_marshal_safearray_to_array (safearray, typeof(conv), elem_type, NULL); */ mono_mb_emit_ldarg (mb, argnum); mono_mb_emit_ptr (mb, mono_class_from_mono_type_internal (t)); diff --git a/mono/metadata/marshal-ilgen.c b/mono/metadata/marshal-ilgen.c index 790ae5cc451d..068a60f0a502 100644 --- a/mono/metadata/marshal-ilgen.c +++ b/mono/metadata/marshal-ilgen.c @@ -2775,10 +2775,6 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, switch (spec->native) { case MONO_NATIVE_LPARRAY: break; - case MONO_NATIVE_SAFEARRAY: -#ifndef DISABLE_COM - return mono_cominterop_emit_marshal_safearray (m, argnum, t, spec, conv_arg, conv_arg_type, action); -#endif default: { char *msg = g_strdup ("Unsupported array type marshalling to managed code."); mono_mb_emit_exception_marshal_directive (mb, msg); diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c index 4f73bef6aa09..5f3e5515249f 100644 --- a/mono/metadata/marshal.c +++ b/mono/metadata/marshal.c @@ -3419,9 +3419,16 @@ mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, return get_marshal_cb ()->emit_marshal_object (m, argnum, t, spec, conv_arg, conv_arg_type, action); case MONO_TYPE_ARRAY: case MONO_TYPE_SZARRAY: - if (spec && (spec->native == MONO_NATIVE_SAFEARRAY) && - ((action == MARSHAL_ACTION_CONV_OUT) || (action == MARSHAL_ACTION_CONV_IN) || (action == MARSHAL_ACTION_PUSH))) - return mono_cominterop_emit_marshal_safearray (m, argnum, t, spec, conv_arg, conv_arg_type, action); + if (spec && (spec->native == MONO_NATIVE_SAFEARRAY)) + { + switch (action) { + case MARSHAL_ACTION_CONV_OUT: + case MARSHAL_ACTION_CONV_IN: + case MARSHAL_ACTION_PUSH: + case MARSHAL_ACTION_MANAGED_CONV_IN: + return mono_cominterop_emit_marshal_safearray (m, argnum, t, spec, conv_arg, conv_arg_type, action); + } + } return get_marshal_cb ()->emit_marshal_array (m, argnum, t, spec, conv_arg, conv_arg_type, action); case MONO_TYPE_BOOLEAN: return get_marshal_cb ()->emit_marshal_boolean (m, argnum, t, spec, conv_arg, conv_arg_type, action); From 48f5e72c5ea793ffac8246277f455782fe48d702 Mon Sep 17 00:00:00 2001 From: Esme Povirk Date: Mon, 14 Aug 2023 14:09:24 -0500 Subject: [PATCH 3/4] Document the MarshalAction constants. --- mono/metadata/marshal-action.txt | 66 ++++++++++++++++++++++++++++++++ mono/metadata/marshal.c | 1 + 2 files changed, 67 insertions(+) create mode 100644 mono/metadata/marshal-action.txt diff --git a/mono/metadata/marshal-action.txt b/mono/metadata/marshal-action.txt new file mode 100644 index 000000000000..76eb4e100c0c --- /dev/null +++ b/mono/metadata/marshal-action.txt @@ -0,0 +1,66 @@ +Because this takes me forever to figure out every time I have to work with marshaling, here is the meaning of every MarshalAction constant. + +MARSHAL_ACTION_CONV_IN: + +Converts a managed value to an unmanaged value that can later be pushed on the stack. May create a local variable and return its index, which will be passed to other actions via conv_arg. + +argnum = index of an argument containing the managed value +conv_arg = 0 +conv_arg_type = out parameter, used to return the type of this argument in the unmanaged function signature + +MARSHAL_ACTION_PUSH: + +Push an unmanaged value from MARSHAL_ACTION_CONV_IN onto the stack. + +argnum = index of an argument containing the managed value +conv_arg = return value from MARSHAL_ACTION_CONV_IN +conv_arg_type = NULL + +MARSHAL_ACTION_CONV_RESULT: + +Convert the return value of an unmanaged function, currently on the stack, to a managed value. Store the managed value in local with index 3. + +NOTE: emit_native_wrapper_ilgen may not call mono_emit_marshal depending on the type. + +argnum = 0 +conv_arg = 0 +conv_arg_type = NULL + +MARSHAL_ACTION_CONV_OUT: + +Convert an unmanaged output argument value to a managed value. + +NOTE: emit_native_wrapper_ilgen may not call mono_emit_marshal depending on the type. + +argnum = index of an argument that will accept the managed value +conv_arg = return value from MARSHAL_ACTION_CONV_IN +conv_arg_type = NULL + +MARSHAL_ACTION_MANAGED_CONV_IN: + +Convert an unmanaged input value to a managed input value. Returns the index of a local that contains the managed input value. The contents of this local (or the address in the case of a byref argument) will be passed directly to the managed method. + +NOTE: emit_managed_wrapper_ilgen may not call mono_emit_marshal depending on the type. + +argnum = index of an argument containing the unmanaged value +conv_arg = 0 +conv_arg_type = out parameter, accepts a type for this argument in the unmanaged signature + +MARSHAL_ACTION_MANAGED_CONV_RESULT: + +Convert the return value of a managed function, currently on the stack, to an unmanaged value. Store the unmanaged value in local with index 3. + +NOTE: emit_managed_wrapper_ilgen may not call mono_emit_marshal depending on the type. + +argnum = 0 +conv_arg = 0 +conv_art_type = NULL + +MARSHAL_ACTION_MANAGED_CONV_OUT: + +Convert a managed output argument value to an unmanaged value. + +argnum = index of an argument that will accept the unmanaged value +conv_arg = return value from MARSHAL_ACTION_MANAGED_CONV_IN, which is the index of a local that now contains the managed value +conv_arg_type = NULL + diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c index 5f3e5515249f..ce947e90cccd 100644 --- a/mono/metadata/marshal.c +++ b/mono/metadata/marshal.c @@ -3371,6 +3371,7 @@ ptr_type_is_copy_constructed (MonoType *type) return FALSE; } +/* See marshal-action.txt for an explanation of the MarshalAction constants */ int mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, MonoMarshalSpec *spec, int conv_arg, From 2dbfbd4a8dce1847555db12ab8e44005c3f3a458 Mon Sep 17 00:00:00 2001 From: Esme Povirk Date: Mon, 14 Aug 2023 14:56:42 -0500 Subject: [PATCH 4/4] [cominterop] Implement safearray managed-out marshaling. --- mono/metadata/cominterop.c | 27 ++++++++++++++++++++++++--- mono/metadata/marshal-ilgen.c | 4 ++-- mono/metadata/marshal.c | 1 + 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/mono/metadata/cominterop.c b/mono/metadata/cominterop.c index 95443f069cbd..227045d73671 100644 --- a/mono/metadata/cominterop.c +++ b/mono/metadata/cominterop.c @@ -4249,14 +4249,20 @@ mono_cominterop_emit_marshal_safearray (EmitMarshalContext *m, int argnum, MonoT break; } case MARSHAL_ACTION_MANAGED_CONV_IN: { - if ((t->attrs & (PARAM_ATTRIBUTE_IN | PARAM_ATTRIBUTE_OUT)) == PARAM_ATTRIBUTE_OUT) - break; - MonoType *object_type = mono_get_object_type (); MonoType *int_type = mono_get_int_type (); *conv_arg_type = int_type; conv_arg = mono_mb_add_local (mb, object_type); + if ((t->attrs & (PARAM_ATTRIBUTE_IN | PARAM_ATTRIBUTE_OUT)) == PARAM_ATTRIBUTE_OUT) + break; + + if (t->byref) { + char *msg = g_strdup ("Byref input safearray marshaling not implemented"); + mono_mb_emit_exception_full (mb, "System.Runtime.InteropServices", "MarshalDirectiveException", msg); + return conv_arg; + } + /* conv = mono_marshal_safearray_to_array (safearray, typeof(conv), elem_type, NULL); */ mono_mb_emit_ldarg (mb, argnum); mono_mb_emit_ptr (mb, mono_class_from_mono_type_internal (t)); @@ -4267,6 +4273,21 @@ mono_cominterop_emit_marshal_safearray (EmitMarshalContext *m, int argnum, MonoT break; } + case MARSHAL_ACTION_MANAGED_CONV_OUT: { + if (t->attrs & PARAM_ATTRIBUTE_OUT) { + g_assert (t->byref); + + /* *argnum = mono_marshal_safearray_from_array(conv_arg, elem_type) */ + mono_mb_emit_ldarg (mb, argnum); + mono_mb_emit_ldloc (mb, conv_arg); + mono_mb_emit_icon (mb, elem_type); + mono_mb_emit_icall (mb, mono_marshal_safearray_from_array); + mono_mb_emit_byte (mb, CEE_STIND_I); + } + + break; + } + default: g_assert_not_reached (); } diff --git a/mono/metadata/marshal-ilgen.c b/mono/metadata/marshal-ilgen.c index 068a60f0a502..3a3d7be7bcf2 100644 --- a/mono/metadata/marshal-ilgen.c +++ b/mono/metadata/marshal-ilgen.c @@ -2950,12 +2950,11 @@ emit_marshal_array_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, MonoMarshalConv conv; gboolean is_string = FALSE; - if (!spec) + if (!spec || t->byref) /* Already handled in CONV_IN */ break; /* These are already checked in CONV_IN */ - g_assert (!t->byref); g_assert (spec->native == MONO_NATIVE_LPARRAY); g_assert (t->attrs & PARAM_ATTRIBUTE_OUT); @@ -6564,6 +6563,7 @@ emit_managed_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *invoke_s case MONO_TYPE_OBJECT: case MONO_TYPE_STRING: case MONO_TYPE_BOOLEAN: + case MONO_TYPE_SZARRAY: mono_emit_marshal (m, i, t, mspecs [i + 1], tmp_locals [i], NULL, MARSHAL_ACTION_MANAGED_CONV_OUT); break; default: diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c index ce947e90cccd..4cec7cdb93e9 100644 --- a/mono/metadata/marshal.c +++ b/mono/metadata/marshal.c @@ -3427,6 +3427,7 @@ mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, case MARSHAL_ACTION_CONV_IN: case MARSHAL_ACTION_PUSH: case MARSHAL_ACTION_MANAGED_CONV_IN: + case MARSHAL_ACTION_MANAGED_CONV_OUT: return mono_cominterop_emit_marshal_safearray (m, argnum, t, spec, conv_arg, conv_arg_type, action); } }