From 6c3481cbba4fcb06a511580227401312712ea2a9 Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Thu, 31 Dec 2020 17:40:46 -0500
Subject: [PATCH 01/11] [metadata] Treat CallConv bit 0x09 as
 MONO_CALL_UNMANAGED_MD

This is the C#9 function pointers "unmanaged"+ext calling convention.
Additional calling convention details are encoded in the modopts of the return
type.

This PR doesn't handle the modopts yet
---
 src/mono/mono/metadata/metadata.c | 1 +
 src/mono/mono/metadata/metadata.h | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c
index d7a792bdf57fe..73a6978c988e9 100644
--- a/src/mono/mono/metadata/metadata.c
+++ b/src/mono/mono/metadata/metadata.c
@@ -2355,6 +2355,7 @@ mono_metadata_parse_method_signature_full (MonoImage *m, MonoGenericContainer *c
 	case MONO_CALL_STDCALL:
 	case MONO_CALL_THISCALL:
 	case MONO_CALL_FASTCALL:
+	case MONO_CALL_UNMANAGED_MD:
 		method->pinvoke = 1;
 		break;
 	}
diff --git a/src/mono/mono/metadata/metadata.h b/src/mono/mono/metadata/metadata.h
index 965b06a86c195..c44931d7328d8 100644
--- a/src/mono/mono/metadata/metadata.h
+++ b/src/mono/mono/metadata/metadata.h
@@ -37,7 +37,11 @@ typedef enum {
 	MONO_CALL_STDCALL,
 	MONO_CALL_THISCALL,
 	MONO_CALL_FASTCALL,
-	MONO_CALL_VARARG
+	MONO_CALL_VARARG = 0x05,
+	/* unused, */
+	/* unused, */
+	/* unused, */
+	MONO_CALL_UNMANAGED_MD = 0x09, /* default unmanaged calling convention, with additional attributed encoded in modopts */
 } MonoCallConvention;
 
 /* ECMA lamespec: the old spec had more info... */

From fca0786402c073428ad1986ad7adf6a4d16a2c5b Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Thu, 31 Dec 2020 17:42:10 -0500
Subject: [PATCH 02/11] [marshal] Add
 mono_marshal_get_native_func_wrapper_indirect

This will be used to add a wrapper around "calli sig" indirect calls where "sig" has
an unmanaged calling convention.

We need to do a GC transition before calling an unmanaged function from
managed. So this wrapper does it.

This is reusing much of the code implemented for
mono_marshal_get_native_func_wrapper_aot which is used for delegates.
Unfortunately that means that the function pointer is (pointlessly) boxed when
it is passed as the first argument.
---
 src/mono/mono/metadata/image.c              |  1 +
 src/mono/mono/metadata/marshal-ilgen.c      |  2 +-
 src/mono/mono/metadata/marshal.c            | 66 +++++++++++++++++++++
 src/mono/mono/metadata/marshal.h            |  5 ++
 src/mono/mono/metadata/metadata-internals.h |  1 +
 5 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/src/mono/mono/metadata/image.c b/src/mono/mono/metadata/image.c
index 79d3921b03439..fe5ec7d9eb103 100644
--- a/src/mono/mono/metadata/image.c
+++ b/src/mono/mono/metadata/image.c
@@ -2493,6 +2493,7 @@ mono_wrapper_caches_free (MonoWrapperCaches *cache)
 	free_hash (cache->native_wrapper_aot_check_cache);
 
 	free_hash (cache->native_func_wrapper_aot_cache);
+	free_hash (cache->native_func_wrapper_indirect_cache);
 	free_hash (cache->remoting_invoke_cache);
 	free_hash (cache->synchronized_cache);
 	free_hash (cache->unbox_wrapper_cache);
diff --git a/src/mono/mono/metadata/marshal-ilgen.c b/src/mono/mono/metadata/marshal-ilgen.c
index ea580d33e0450..1f9fe54b85463 100644
--- a/src/mono/mono/metadata/marshal-ilgen.c
+++ b/src/mono/mono/metadata/marshal-ilgen.c
@@ -2141,7 +2141,7 @@ emit_native_wrapper_ilgen (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSi
 		mono_mb_emit_byte (mb, CEE_LDARG_0);
 		mono_mb_emit_op (mb, CEE_UNBOX, mono_defaults.int_class);
 		mono_mb_emit_byte (mb, CEE_LDIND_I);
-		if (piinfo->piflags & PINVOKE_ATTRIBUTE_SUPPORTS_LAST_ERROR) {
+		if (piinfo && (piinfo->piflags & PINVOKE_ATTRIBUTE_SUPPORTS_LAST_ERROR) != 0) {
 			mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
 			mono_mb_emit_byte (mb, CEE_MONO_SAVE_LAST_ERROR);
 		}
diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c
index 17d0430d740c8..213d94af1cef1 100644
--- a/src/mono/mono/metadata/marshal.c
+++ b/src/mono/mono/metadata/marshal.c
@@ -3658,6 +3658,72 @@ mono_marshal_get_native_func_wrapper_aot (MonoClass *klass)
 	return res;
 }
 
+/*
+ * Gets a wrapper for an indirect call to a function with the given signature.
+ * The actual function is passed as the first argument to the wrapper.
+ *
+ * The wrapper is
+ *
+ * retType wrapper (fnPtrBoxed, arg1... argN) {
+ *   enter_gc_safe;
+ *   fnPtr = unbox (fnPtrBoxed)
+ *   ret = fnPtr (arg1, ... argN);
+ *   exit_gc_safe;
+ *   return ret;
+ * }
+ *
+ */
+MonoMethod*
+mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMethodSignature *sig,
+					       gboolean aot)
+{
+	caller_class = mono_class_get_generic_type_definition (caller_class);
+	MonoImage *image = m_class_get_image (caller_class);
+	g_assert (sig->pinvoke);
+	g_assert (!sig->hasthis && ! sig->explicit_this);
+	g_assert (!sig->is_inflated && !sig->has_type_parameters);
+	/* g_assert (every param and return type is blittable) */
+
+	GHashTable *cache = get_cache (&image->wrapper_caches.native_func_wrapper_indirect_cache,
+				       (GHashFunc)mono_signature_hash, 
+				       (GCompareFunc)mono_metadata_signature_equal);
+	
+	MonoMethod *res;
+	if ((res = mono_marshal_find_in_cache (cache, sig)))
+	    return res;
+	
+	fprintf (stderr, "generating wrapper for signature %s\n", mono_signature_full_name (sig));
+	
+	/* FIXME: better wrapper name */
+	char * name = g_strdup_printf ("wrapper_native_indirect_%p", sig);
+	MonoMethodBuilder *mb = mono_mb_new (caller_class, name, MONO_WRAPPER_MANAGED_TO_NATIVE);
+	mb->method->save_lmf = 1;
+
+	WrapperInfo *info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_NATIVE_FUNC_INDIRECT);
+	info->d.managed_to_native.method = NULL;
+
+	MonoMethodPInvoke *piinfo = NULL;
+	MonoMarshalSpec **mspecs = g_new0 (MonoMarshalSpec *, 1 + sig->param_count);
+        gpointer func = NULL;
+	gboolean check_exceptions = FALSE; /* FIXME: don't expect native to throw */
+	gboolean func_param = TRUE;
+	gboolean skip_gc_trans = FALSE;
+	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, aot, check_exceptions, func_param, skip_gc_trans);
+	g_free (mspecs);
+
+	MonoMethodSignature *csig = mono_metadata_signature_dup_add_this (image, sig, mono_defaults.int_class);
+	csig->pinvoke = 0;
+
+	MonoMethodSignature *key_sig = mono_metadata_signature_dup_full (image, sig);
+
+	gboolean found;
+	res = mono_mb_create_and_cache_full (cache, key_sig, mb, csig, csig->param_count + 16, info, &found);
+
+	mono_mb_free (mb);
+
+	return res;
+}
+
 /*
  * mono_marshal_emit_managed_wrapper:
  *
diff --git a/src/mono/mono/metadata/marshal.h b/src/mono/mono/metadata/marshal.h
index 13b3ec94da513..1d75e2d6ea02a 100644
--- a/src/mono/mono/metadata/marshal.h
+++ b/src/mono/mono/metadata/marshal.h
@@ -115,6 +115,7 @@ typedef enum {
 	/* Subtypes of MONO_WRAPPER_MANAGED_TO_NATIVE */
 	WRAPPER_SUBTYPE_ICALL_WRAPPER, // specifically JIT icalls
 	WRAPPER_SUBTYPE_NATIVE_FUNC_AOT,
+	WRAPPER_SUBTYPE_NATIVE_FUNC_INDIRECT,
 	WRAPPER_SUBTYPE_PINVOKE,
 	/* Subtypes of MONO_WRAPPER_OTHER */
 	WRAPPER_SUBTYPE_SYNCHRONIZED_INNER,
@@ -473,6 +474,10 @@ mono_marshal_get_native_func_wrapper (MonoImage *image, MonoMethodSignature *sig
 MonoMethod*
 mono_marshal_get_native_func_wrapper_aot (MonoClass *klass);
 
+MonoMethod*
+mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMethodSignature *sig,
+					       gboolean aot);
+
 MonoMethod *
 mono_marshal_get_struct_to_ptr (MonoClass *klass);
 
diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h
index 3dbf9b85f3f3a..329f839f7fc33 100644
--- a/src/mono/mono/metadata/metadata-internals.h
+++ b/src/mono/mono/metadata/metadata-internals.h
@@ -268,6 +268,7 @@ typedef struct {
 	GHashTable *native_wrapper_aot_check_cache;
 
 	GHashTable *native_func_wrapper_aot_cache;
+	GHashTable *native_func_wrapper_indirect_cache; /* Indexed by MonoMethodSignature. Protected by the marshal lock */
 	GHashTable *remoting_invoke_cache;
 	GHashTable *synchronized_cache;
 	GHashTable *unbox_wrapper_cache;

From 05302a384168b7eb12ad70d6d6fb1b38a71ea5aa Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Thu, 31 Dec 2020 17:44:18 -0500
Subject: [PATCH 03/11] [jit] Use a wrapper for "calli unmanaged" instructions

If there's a calli with an unmanaged signature, invoke a wrapper that
does a transition to GC Safe mode and then do the call.

The wrapper is always inlined into the callee.

Because we're reusing much of the code of
mono_marshal_get_native_func_wrapper_aot, the function pointer first has to be
boxed before it's passed to the wrapper.  In theory we should be able to just
pass it directly and get much simpler code.  But that will require changing the
code in emit_native_wrapper_ilgen to get an unboxed function pointer arg
---
 src/mono/mono/mini/method-to-ir.c | 47 +++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index 3e383722bcf72..f1abc6a529621 100644
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -7065,6 +7065,53 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 				addr = mono_emit_jit_icall (cfg, mono_get_native_calli_wrapper, args);
 			}
 
+			if (fsig->pinvoke && method->wrapper_type != MONO_WRAPPER_MANAGED_TO_NATIVE) {
+				gboolean skip_gc_trans = FALSE; /* TODO: unmanaged[SuppressGCTransition] call conv */
+				if (!skip_gc_trans) {
+					/* Call the wrapper that will do the GC transition instead */
+					MonoMethod *wrapper = mono_marshal_get_native_func_wrapper_indirect (method->klass, fsig, cfg->compile_aot);
+					/* box the indirect function ptr */
+					/* FIXME: it would be nice if the wrapper didn't require a boxed argument */
+
+					MONO_INST_NEW (cfg, ins, OP_BOX);
+					ins->type = STACK_OBJ;
+					ins->klass = mono_defaults.int_class;
+					ins->sreg1 = addr->dreg;
+					ins->dreg = alloc_dreg (cfg, STACK_PTR);
+					MONO_ADD_INS (cfg->cbb, ins);
+
+					fsig = mono_method_signature_internal (wrapper);
+
+					n = fsig->param_count - 1; /* wrapper has extra fnptr param */
+
+					CHECK_STACK (n);
+
+					/* move the args to allow room for 'this' in the first position */
+					while (n--) {
+						--sp;
+						sp [1] = sp [0];
+					}
+
+					sp[0] = ins; /* n+1 args, first arg is the boxed address of the indirect method to call */
+
+					g_assert (!fsig->hasthis && !fsig->pinvoke);
+
+					gboolean inline_wrapper = cfg->opt & MONO_OPT_INLINE || cfg->compile_aot;
+					if (inline_wrapper) {
+						int costs = inline_method (cfg, wrapper, fsig, sp, ip, cfg->real_offset, TRUE);
+						CHECK_CFG_EXCEPTION;
+						g_assert (costs > 0);
+						cfg->real_offset += 5; /* FIXME: why? */
+						inline_costs += costs;
+						/* FIXME: calli_end expects ins to be the result of the call. */
+						ins = sp[0]; /*FIXME: is that the right instruction?*/
+					} else {
+						ins = mono_emit_method_call (cfg, wrapper, /*args*/sp, NULL);
+					}
+					goto calli_end; /* FIXME? yes? */
+				}
+			}
+
 			n = fsig->param_count + fsig->hasthis;
 
 			CHECK_STACK (n);

From 89fd579643e036d75c8c8d76e2a0da541702ff02 Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Thu, 31 Dec 2020 20:33:33 -0500
Subject: [PATCH 04/11] fixup don't emit GC transitions in runtime invoke
 wrapper

---
 src/mono/mono/mini/method-to-ir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index f1abc6a529621..a7a3253346119 100644
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -7065,7 +7065,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 				addr = mono_emit_jit_icall (cfg, mono_get_native_calli_wrapper, args);
 			}
 
-			if (fsig->pinvoke && method->wrapper_type != MONO_WRAPPER_MANAGED_TO_NATIVE) {
+			if (fsig->pinvoke &&
+			    method->wrapper_type != MONO_WRAPPER_MANAGED_TO_NATIVE &&
+			    method->wrapper_type != MONO_WRAPPER_RUNTIME_INVOKE) {
 				gboolean skip_gc_trans = FALSE; /* TODO: unmanaged[SuppressGCTransition] call conv */
 				if (!skip_gc_trans) {
 					/* Call the wrapper that will do the GC transition instead */

From b9e825a9ddca537395e3caa490db79a79cae002c Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Thu, 31 Dec 2020 21:23:20 -0500
Subject: [PATCH 05/11] fixup don't emit two wrappers on dynamic methods

---
 src/mono/mono/mini/method-to-ir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index a7a3253346119..eb738ce965b98 100644
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -7065,7 +7065,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 				addr = mono_emit_jit_icall (cfg, mono_get_native_calli_wrapper, args);
 			}
 
-			if (fsig->pinvoke &&
+			if (!method->dynamic && fsig->pinvoke &&
 			    method->wrapper_type != MONO_WRAPPER_MANAGED_TO_NATIVE &&
 			    method->wrapper_type != MONO_WRAPPER_RUNTIME_INVOKE) {
 				gboolean skip_gc_trans = FALSE; /* TODO: unmanaged[SuppressGCTransition] call conv */

From f6e52ad9d7f43f3ce68ede22b7782ff81faa0d56 Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Fri, 1 Jan 2021 11:19:04 -0500
Subject: [PATCH 06/11] check that calli wrapper only gets blittable args

---
 src/mono/mono/metadata/marshal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c
index 213d94af1cef1..13dcbd3ffd88d 100644
--- a/src/mono/mono/metadata/marshal.c
+++ b/src/mono/mono/metadata/marshal.c
@@ -116,6 +116,8 @@ static GENERATE_TRY_GET_CLASS_WITH_CACHE (suppress_gc_transition_attribute, "Sys
 static GENERATE_TRY_GET_CLASS_WITH_CACHE (unmanaged_callers_only_attribute, "System.Runtime.InteropServices", "UnmanagedCallersOnlyAttribute")
 #endif
 
+static gboolean type_is_blittable (MonoType *type);
+
 static MonoImage*
 get_method_image (MonoMethod *method)
 {
@@ -3682,6 +3684,17 @@ mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMeth
 	g_assert (sig->pinvoke);
 	g_assert (!sig->hasthis && ! sig->explicit_this);
 	g_assert (!sig->is_inflated && !sig->has_type_parameters);
+
+	if (!type_is_blittable (sig->ret)) {
+		g_assertf (type_is_blittable (sig->ret), "sig return type %s is not blittable\n", mono_type_full_name (sig->ret));
+	}
+	for (int i = 0; i < sig->param_count; ++i)
+	{
+		MonoType *ty = sig->params[i];
+		if (!type_is_blittable (ty)) {
+			g_assertf (type_is_blittable (ty), "sig param %d (type %s) is not blittable\n", i, mono_type_full_name (ty));
+		}
+	}
 	/* g_assert (every param and return type is blittable) */
 
 	GHashTable *cache = get_cache (&image->wrapper_caches.native_func_wrapper_indirect_cache,

From 72682b25cc57f719cd2a1603d152063786893188 Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Mon, 4 Jan 2021 13:28:16 -0500
Subject: [PATCH 07/11] negate logic for when to add an indirection wrapper

assume that if the callee is a wrapper, it doesn't need the transition, and
only add it to normal managed methods.
---
 src/mono/mono/mini/method-to-ir.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index eb738ce965b98..d5e6d15ec772d 100644
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -7066,9 +7066,14 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 			}
 
 			if (!method->dynamic && fsig->pinvoke &&
-			    method->wrapper_type != MONO_WRAPPER_MANAGED_TO_NATIVE &&
-			    method->wrapper_type != MONO_WRAPPER_RUNTIME_INVOKE) {
-				gboolean skip_gc_trans = FALSE; /* TODO: unmanaged[SuppressGCTransition] call conv */
+			    !method->wrapper_type) {
+				/* MONO_WRAPPER_DYNAMIC_METHOD dynamic method handled above in the
+				method->dynamic case; for other wrapper types assume the code knows
+				what its doing and added its own GC transitions */
+
+				/* TODO: unmanaged[SuppressGCTransition] call conv will set
+				 * skip_gc_trans to TRUE*/
+				gboolean skip_gc_trans = FALSE;
 				if (!skip_gc_trans) {
 					/* Call the wrapper that will do the GC transition instead */
 					MonoMethod *wrapper = mono_marshal_get_native_func_wrapper_indirect (method->klass, fsig, cfg->compile_aot);

From 675e7527bf5f8eec96d0b0d27956133a71c4e1ae Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Fri, 1 Jan 2021 11:19:23 -0500
Subject: [PATCH 08/11] add disabled debug printf

---
 src/mono/mono/mini/method-to-ir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index d5e6d15ec772d..cd3619e89375d 100644
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -7075,6 +7075,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 				 * skip_gc_trans to TRUE*/
 				gboolean skip_gc_trans = FALSE;
 				if (!skip_gc_trans) {
+#if 0
+					fprintf (stderr, "generating wrapper for calli in method %s with wrapper type %s\n", method->name, mono_wrapper_type_to_str (method->wrapper_type));
+#endif
 					/* Call the wrapper that will do the GC transition instead */
 					MonoMethod *wrapper = mono_marshal_get_native_func_wrapper_indirect (method->klass, fsig, cfg->compile_aot);
 					/* box the indirect function ptr */

From 4cccd110cec0680a3c52c608eab23adf577fdecb Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Tue, 5 Jan 2021 12:27:02 -0500
Subject: [PATCH 09/11] Add MonoNativeWrapperFlags arg to emit_native_wrapper

Replace the 4 boolean args by a flags arg.

Also add a new EMIT_NATIVE_WRAPPER_FUNC_PARAM_UNBOXED and use it in
mono_marshal_get_native_func_wrapper_indirect.  The new flag has no effect yet.
---
 src/mono/mono/metadata/cominterop.c      |  2 +-
 src/mono/mono/metadata/marshal-ilgen.c   |  9 ++++++++-
 src/mono/mono/metadata/marshal-noilgen.c |  2 +-
 src/mono/mono/metadata/marshal.c         | 18 ++++++++++++------
 src/mono/mono/metadata/marshal.h         | 16 +++++++++++++---
 5 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/src/mono/mono/metadata/cominterop.c b/src/mono/mono/metadata/cominterop.c
index 4c46107221a44..e80cca9f292d7 100644
--- a/src/mono/mono/metadata/cominterop.c
+++ b/src/mono/mono/metadata/cominterop.c
@@ -1023,7 +1023,7 @@ cominterop_get_native_wrapper_adjusted (MonoMethod *method)
 		}
 	}
 
-	mono_marshal_emit_native_wrapper (m_class_get_image (method->klass), mb_native, sig_native, piinfo, mspecs, piinfo->addr, FALSE, TRUE, FALSE, FALSE);
+	mono_marshal_emit_native_wrapper (m_class_get_image (method->klass), mb_native, sig_native, piinfo, mspecs, piinfo->addr, EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS);
 
 	res = mono_mb_create_method (mb_native, sig_native, sig_native->param_count + 16);	
 
diff --git a/src/mono/mono/metadata/marshal-ilgen.c b/src/mono/mono/metadata/marshal-ilgen.c
index 1f9fe54b85463..f7a7e0742e768 100644
--- a/src/mono/mono/metadata/marshal-ilgen.c
+++ b/src/mono/mono/metadata/marshal-ilgen.c
@@ -1995,13 +1995,20 @@ gc_safe_transition_builder_cleanup (GCSafeTransitionBuilder *builder)
  * \param method if non-NULL, the pinvoke method to call
  * \param check_exceptions Whenever to check for pending exceptions after the native call
  * \param func_param the function to call is passed as a boxed IntPtr as the first parameter
+ * \param func_param_unboxed combined with \p func_param, expect the function to call as an unboxed IntPtr as the first parameter
  * \param skip_gc_trans Whenever to skip GC transitions
  *
  * generates IL code for the pinvoke wrapper, the generated code calls \p func .
  */
 static void
-emit_native_wrapper_ilgen (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, gboolean aot, gboolean check_exceptions, gboolean func_param, gboolean skip_gc_trans)
+emit_native_wrapper_ilgen (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, MonoNativeWrapperFlags flags)
 {
+	gboolean aot = (flags & EMIT_NATIVE_WRAPPER_AOT) != 0;
+	gboolean check_exceptions = (flags & EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS) != 0;
+	gboolean func_param = (flags & EMIT_NATIVE_WRAPPER_FUNC_PARAM) != 0;
+	/* TODO: make use of func_param_unboxed */
+	gboolean func_param_unboxed = (flags & EMIT_NATIVE_WRAPPER_FUNC_PARAM_UNBOXED) != 0;
+	gboolean skip_gc_trans = (flags & EMIT_NATIVE_WRAPPER_SKIP_GC_TRANS) != 0;
 	EmitMarshalContext m;
 	MonoMethodSignature *csig;
 	MonoClass *klass;
diff --git a/src/mono/mono/metadata/marshal-noilgen.c b/src/mono/mono/metadata/marshal-noilgen.c
index 6555604810baf..cb79e2f504a73 100644
--- a/src/mono/mono/metadata/marshal-noilgen.c
+++ b/src/mono/mono/metadata/marshal-noilgen.c
@@ -350,7 +350,7 @@ emit_thunk_invoke_wrapper_noilgen (MonoMethodBuilder *mb, MonoMethod *method, Mo
 }
 
 static void
-emit_native_wrapper_noilgen (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, gboolean aot, gboolean check_exceptions, gboolean func_param, gboolean skip_gc_trans)
+emit_native_wrapper_noilgen (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, MonoNativeWrapperFlags flags)
 {
 }
 
diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c
index 13dcbd3ffd88d..df606de1786ba 100644
--- a/src/mono/mono/metadata/marshal.c
+++ b/src/mono/mono/metadata/marshal.c
@@ -3520,7 +3520,11 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
 	}
 #endif
 
-	mono_marshal_emit_native_wrapper (get_method_image (mb->method), mb, csig, piinfo, mspecs, piinfo->addr, aot, check_exceptions, FALSE, skip_gc_trans);
+	MonoNativeWrapperFlags flags = aot ? EMIT_NATIVE_WRAPPER_AOT : (MonoNativeWrapperFlags)0;
+	flags |= check_exceptions ? EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS : (MonoNativeWrapperFlags)0;
+	flags |= skip_gc_trans ? EMIT_NATIVE_WRAPPER_SKIP_GC_TRANS : (MonoNativeWrapperFlags)0;
+
+	mono_marshal_emit_native_wrapper (get_method_image (mb->method), mb, csig, piinfo, mspecs, piinfo->addr, flags);
 	info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_PINVOKE);
 	info->d.managed_to_native.method = method;
 
@@ -3575,7 +3579,7 @@ mono_marshal_get_native_func_wrapper (MonoImage *image, MonoMethodSignature *sig
 	mb = mono_mb_new (mono_defaults.object_class, name, MONO_WRAPPER_MANAGED_TO_NATIVE);
 	mb->method->save_lmf = 1;
 
-	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, FALSE, TRUE, FALSE, FALSE);
+	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS);
 
 	csig = mono_metadata_signature_dup_full (image, sig);
 	csig->pinvoke = 0;
@@ -3638,7 +3642,7 @@ mono_marshal_get_native_func_wrapper_aot (MonoClass *klass)
 	mb = mono_mb_new (invoke->klass, name, MONO_WRAPPER_MANAGED_TO_NATIVE);
 	mb->method->save_lmf = 1;
 
-	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, NULL, FALSE, TRUE, TRUE, FALSE);
+	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, NULL, EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS | EMIT_NATIVE_WRAPPER_FUNC_PARAM);
 
 	info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_NATIVE_FUNC_AOT);
 	info->d.managed_to_native.method = invoke;
@@ -3721,7 +3725,9 @@ mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMeth
 	gboolean check_exceptions = FALSE; /* FIXME: don't expect native to throw */
 	gboolean func_param = TRUE;
 	gboolean skip_gc_trans = FALSE;
-	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, aot, check_exceptions, func_param, skip_gc_trans);
+	MonoNativeWrapperFlags flags = aot ? EMIT_NATIVE_WRAPPER_AOT : (MonoNativeWrapperFlags)0;
+	flags |= EMIT_NATIVE_WRAPPER_FUNC_PARAM | EMIT_NATIVE_WRAPPER_FUNC_PARAM_UNBOXED;
+	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, flags);
 	g_free (mspecs);
 
 	MonoMethodSignature *csig = mono_metadata_signature_dup_add_this (image, sig, mono_defaults.int_class);
@@ -6462,9 +6468,9 @@ mono_marshal_lookup_pinvoke (MonoMethod *method)
 }
 
 void
-mono_marshal_emit_native_wrapper (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, gboolean aot, gboolean check_exceptions, gboolean func_param, gboolean skip_gc_trans)
+mono_marshal_emit_native_wrapper (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, MonoNativeWrapperFlags flags)
 {
-	get_marshal_cb ()->emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, aot, check_exceptions, func_param, skip_gc_trans);
+	get_marshal_cb ()->emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, flags);
 }
 
 static MonoMarshalCallbacks marshal_cb;
diff --git a/src/mono/mono/metadata/marshal.h b/src/mono/mono/metadata/marshal.h
index 1d75e2d6ea02a..2ec6326acdb87 100644
--- a/src/mono/mono/metadata/marshal.h
+++ b/src/mono/mono/metadata/marshal.h
@@ -301,7 +301,17 @@ typedef enum {
 } MonoStelemrefKind;
 
 
-#define MONO_MARSHAL_CALLBACKS_VERSION 4
+typedef enum {
+	EMIT_NATIVE_WRAPPER_AOT = 0x01, /* FIXME: what does "aot" mean here */
+	EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS = 0x02,
+	EMIT_NATIVE_WRAPPER_FUNC_PARAM = 0x04,
+	EMIT_NATIVE_WRAPPER_FUNC_PARAM_UNBOXED = 0x08,
+	EMIT_NATIVE_WRAPPER_SKIP_GC_TRANS=0x10,
+} MonoNativeWrapperFlags;
+
+G_ENUM_FUNCTIONS(MonoNativeWrapperFlags);
+
+#define MONO_MARSHAL_CALLBACKS_VERSION 5
 
 typedef struct {
 	int version;
@@ -326,7 +336,7 @@ typedef struct {
 	void (*emit_virtual_stelemref) (MonoMethodBuilder *mb, const char **param_names, MonoStelemrefKind kind);
 	void (*emit_stelemref) (MonoMethodBuilder *mb);
 	void (*emit_array_address) (MonoMethodBuilder *mb, int rank, int elem_size);
-	void (*emit_native_wrapper) (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, gboolean aot, gboolean check_exceptions, gboolean func_param, gboolean skip_gc_trans);
+	void (*emit_native_wrapper) (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, MonoNativeWrapperFlags flags);
 	void (*emit_managed_wrapper) (MonoMethodBuilder *mb, MonoMethodSignature *invoke_sig, MonoMarshalSpec **mspecs, EmitMarshalContext* m, MonoMethod *method, MonoGCHandle target_handle);
 	void (*emit_runtime_invoke_body) (MonoMethodBuilder *mb, const char **param_names, MonoImage *image, MonoMethod *method, MonoMethodSignature *sig, MonoMethodSignature *callsig, gboolean virtual_, gboolean need_direct_wrapper);
 	void (*emit_runtime_invoke_dynamic) (MonoMethodBuilder *mb);
@@ -673,7 +683,7 @@ mono_signature_no_pinvoke (MonoMethod *method);
 /* Called from cominterop.c/remoting.c */
 
 void
-mono_marshal_emit_native_wrapper (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, gboolean aot, gboolean check_exceptions, gboolean func_param, gboolean skip_gc_trans);
+mono_marshal_emit_native_wrapper (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func, MonoNativeWrapperFlags flags);
 
 void
 mono_marshal_emit_managed_wrapper (MonoMethodBuilder *mb, MonoMethodSignature *invoke_sig, MonoMarshalSpec **mspecs, EmitMarshalContext* m, MonoMethod *method, MonoGCHandle target_handle);

From b93e9323eb5d2295b0dcc8ab1bdfc01f9d958359 Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Tue, 5 Jan 2021 12:34:27 -0500
Subject: [PATCH 10/11] Address review feedback

---
 src/mono/mono/metadata/marshal.c  | 22 ++++++++--------------
 src/mono/mono/mini/method-to-ir.c |  5 ++---
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c
index df606de1786ba..5c2bc0631120c 100644
--- a/src/mono/mono/metadata/marshal.c
+++ b/src/mono/mono/metadata/marshal.c
@@ -3689,15 +3689,11 @@ mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMeth
 	g_assert (!sig->hasthis && ! sig->explicit_this);
 	g_assert (!sig->is_inflated && !sig->has_type_parameters);
 
-	if (!type_is_blittable (sig->ret)) {
-		g_assertf (type_is_blittable (sig->ret), "sig return type %s is not blittable\n", mono_type_full_name (sig->ret));
-	}
-	for (int i = 0; i < sig->param_count; ++i)
-	{
-		MonoType *ty = sig->params[i];
-		if (!type_is_blittable (ty)) {
-			g_assertf (type_is_blittable (ty), "sig param %d (type %s) is not blittable\n", i, mono_type_full_name (ty));
-		}
+	g_assertf (type_is_blittable (sig->ret), "sig return type %s is not blittable\n", mono_type_full_name (sig->ret));
+
+	for (int i = 0; i < sig->param_count; ++i) {
+		MonoType *ty = sig->params [i];
+		g_assertf (type_is_blittable (ty), "sig param %d (type %s) is not blittable\n", i, mono_type_full_name (ty));
 	}
 	/* g_assert (every param and return type is blittable) */
 
@@ -3709,7 +3705,9 @@ mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMeth
 	if ((res = mono_marshal_find_in_cache (cache, sig)))
 	    return res;
 	
+#if 0
 	fprintf (stderr, "generating wrapper for signature %s\n", mono_signature_full_name (sig));
+#endif
 	
 	/* FIXME: better wrapper name */
 	char * name = g_strdup_printf ("wrapper_native_indirect_%p", sig);
@@ -3721,13 +3719,9 @@ mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMeth
 
 	MonoMethodPInvoke *piinfo = NULL;
 	MonoMarshalSpec **mspecs = g_new0 (MonoMarshalSpec *, 1 + sig->param_count);
-        gpointer func = NULL;
-	gboolean check_exceptions = FALSE; /* FIXME: don't expect native to throw */
-	gboolean func_param = TRUE;
-	gboolean skip_gc_trans = FALSE;
 	MonoNativeWrapperFlags flags = aot ? EMIT_NATIVE_WRAPPER_AOT : (MonoNativeWrapperFlags)0;
 	flags |= EMIT_NATIVE_WRAPPER_FUNC_PARAM | EMIT_NATIVE_WRAPPER_FUNC_PARAM_UNBOXED;
-	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, flags);
+	mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, /*func*/NULL, flags);
 	g_free (mspecs);
 
 	MonoMethodSignature *csig = mono_metadata_signature_dup_add_this (image, sig, mono_defaults.int_class);
diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index cd3619e89375d..01525af19f311 100644
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -7113,12 +7113,11 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 						g_assert (costs > 0);
 						cfg->real_offset += 5; /* FIXME: why? */
 						inline_costs += costs;
-						/* FIXME: calli_end expects ins to be the result of the call. */
-						ins = sp[0]; /*FIXME: is that the right instruction?*/
+						ins = sp[0];
 					} else {
 						ins = mono_emit_method_call (cfg, wrapper, /*args*/sp, NULL);
 					}
-					goto calli_end; /* FIXME? yes? */
+					goto calli_end;
 				}
 			}
 

From 25b94ac953efd74a61152801661be45e619a6faa Mon Sep 17 00:00:00 2001
From: Aleksey Kliger <alklig@microsoft.com>
Date: Tue, 5 Jan 2021 12:39:16 -0500
Subject: [PATCH 11/11] Use an unboxed func ptr
 mono_marshal_get_native_func_wrapper_indirect

Add support for the EMIT_NATIVE_WRAPPER_FUNC_PARAM_UNBOXED flag to mono_marshal_emit_native_wrapper
---
 src/mono/mono/metadata/marshal-ilgen.c |  7 ++++---
 src/mono/mono/metadata/marshal.c       |  3 +--
 src/mono/mono/mini/method-to-ir.c      | 13 ++-----------
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/mono/mono/metadata/marshal-ilgen.c b/src/mono/mono/metadata/marshal-ilgen.c
index f7a7e0742e768..17337b71bbb3d 100644
--- a/src/mono/mono/metadata/marshal-ilgen.c
+++ b/src/mono/mono/metadata/marshal-ilgen.c
@@ -2006,7 +2006,6 @@ emit_native_wrapper_ilgen (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSi
 	gboolean aot = (flags & EMIT_NATIVE_WRAPPER_AOT) != 0;
 	gboolean check_exceptions = (flags & EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS) != 0;
 	gboolean func_param = (flags & EMIT_NATIVE_WRAPPER_FUNC_PARAM) != 0;
-	/* TODO: make use of func_param_unboxed */
 	gboolean func_param_unboxed = (flags & EMIT_NATIVE_WRAPPER_FUNC_PARAM_UNBOXED) != 0;
 	gboolean skip_gc_trans = (flags & EMIT_NATIVE_WRAPPER_SKIP_GC_TRANS) != 0;
 	EmitMarshalContext m;
@@ -2146,8 +2145,10 @@ emit_native_wrapper_ilgen (MonoImage *image, MonoMethodBuilder *mb, MonoMethodSi
 	/* call the native method */
 	if (func_param) {
 		mono_mb_emit_byte (mb, CEE_LDARG_0);
-		mono_mb_emit_op (mb, CEE_UNBOX, mono_defaults.int_class);
-		mono_mb_emit_byte (mb, CEE_LDIND_I);
+		if (!func_param_unboxed) {
+			mono_mb_emit_op (mb, CEE_UNBOX, mono_defaults.int_class);
+			mono_mb_emit_byte (mb, CEE_LDIND_I);
+		}
 		if (piinfo && (piinfo->piflags & PINVOKE_ATTRIBUTE_SUPPORTS_LAST_ERROR) != 0) {
 			mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
 			mono_mb_emit_byte (mb, CEE_MONO_SAVE_LAST_ERROR);
diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c
index 5c2bc0631120c..44cf4ad27ee7a 100644
--- a/src/mono/mono/metadata/marshal.c
+++ b/src/mono/mono/metadata/marshal.c
@@ -3670,9 +3670,8 @@ mono_marshal_get_native_func_wrapper_aot (MonoClass *klass)
  *
  * The wrapper is
  *
- * retType wrapper (fnPtrBoxed, arg1... argN) {
+ * retType wrapper (fnPtr, arg1... argN) {
  *   enter_gc_safe;
- *   fnPtr = unbox (fnPtrBoxed)
  *   ret = fnPtr (arg1, ... argN);
  *   exit_gc_safe;
  *   return ret;
diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
index 01525af19f311..530320f0d07ee 100644
--- a/src/mono/mono/mini/method-to-ir.c
+++ b/src/mono/mono/mini/method-to-ir.c
@@ -7080,15 +7080,6 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 #endif
 					/* Call the wrapper that will do the GC transition instead */
 					MonoMethod *wrapper = mono_marshal_get_native_func_wrapper_indirect (method->klass, fsig, cfg->compile_aot);
-					/* box the indirect function ptr */
-					/* FIXME: it would be nice if the wrapper didn't require a boxed argument */
-
-					MONO_INST_NEW (cfg, ins, OP_BOX);
-					ins->type = STACK_OBJ;
-					ins->klass = mono_defaults.int_class;
-					ins->sreg1 = addr->dreg;
-					ins->dreg = alloc_dreg (cfg, STACK_PTR);
-					MONO_ADD_INS (cfg->cbb, ins);
 
 					fsig = mono_method_signature_internal (wrapper);
 
@@ -7102,7 +7093,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 						sp [1] = sp [0];
 					}
 
-					sp[0] = ins; /* n+1 args, first arg is the boxed address of the indirect method to call */
+					sp[0] = addr; /* n+1 args, first arg is the address of the indirect method to call */
 
 					g_assert (!fsig->hasthis && !fsig->pinvoke);
 
@@ -7111,7 +7102,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 						int costs = inline_method (cfg, wrapper, fsig, sp, ip, cfg->real_offset, TRUE);
 						CHECK_CFG_EXCEPTION;
 						g_assert (costs > 0);
-						cfg->real_offset += 5; /* FIXME: why? */
+						cfg->real_offset += 5;
 						inline_costs += costs;
 						ins = sp[0];
 					} else {