[Java.Interop.Export] Begin supporting methods with 14+ params (#635)

Context: dotnet/android#4631
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3685184&view=logs&j=b1314ebb-4483-559c-0367-730fc62c4440&t=5022c07e-db44-5ce4-1a38-092215b61c50
Context: 70fc4c0

Commit 70fc4c0 broke use of `jnimarshalmethod-gen.exe` within
xamarin-android and `Mono.Android.dll`, as it allows methods taking
more than 14 parameters to be bound, which results in
`jnimarshalmethod-gen.exe` trying to use an `Action<…>` or
`Func<…>` that takes more than 16 parameters, which isn't allowed:

	EXEC : error : jnimarshalmethod-gen: Unable to process assembly '…/xamarin-android/bin/Release/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android.dll'
	  An incorrect number of type arguments were specified for the declaration of an Action type.
	  Parameter name: typeArgs
	  System.ArgumentException: An incorrect number of type arguments were specified for the declaration of an Action type.
	  Parameter name: typeArgs
	    at System.Linq.Expressions.Expression.GetActionType (System.Type[] typeArgs)
	    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method, Java.Interop.JavaCallableAttribute callable, System.Type type)
	    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method)
	    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.CreateMarshalMethodAssembly (System.String path)
	    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.ProcessAssemblies (System.Collections.Generic.List`1[T] assemblies)

Update `jnimarshalmethod-gen.exe` so that it *skips* methods which
take more than 14 parameters.  This is to unblock xamarin-android
integrations.

Next, *begin* plumbing support for this scenario.

To remove the workaround and get things fully working (failing),
rebuild `jnimarshalmethod-gen.exe` with:

	msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_AllTheArguments=True

Also update `jnimarshalmethod-gen.exe` so that it *optionally* uses
`Mono.Linq.Expressions`, for additional debugging output:

	msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_DumpRegisterNativeMembers=True

(Use `/p:_AllTheArguments=True /p:_DumpRegisterNativeMembers=True`
for both!)

Add a method to `ExportTest.cs` / `ExportType.java` which takes more
than 14 parameters, and update `MarshalMemberBuilder` so that it can
generate the appropriate JNI Marshal Method for such a method.
(This should help avoid the need for xamarin-android integrations to
see what's broken.)

The complexity of supporting more than 14 parameters doesn't lie in
`MarshalMemberBuilder`, per-se; `Java.Interop.Export-Tests.dll` runs
just fine when just using `Expression.Lambda(body, bodyParams)`.
Things get much more complicated when `jnimarshalmethod-gen.exe`
enters the picture, and interacts with `MarshalMemberBuilder`.
`jnimarshalmethod-gen.exe` *requires* an "appropriate" delegate type.

Update `MarshalMemberBuilder` to "know about" the `_JniMarshal*`
delegates added in cbb50af, and attempt to use them from the assembly
that is being processed, if they exist.

This got further, resulting in an error that
`_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z` can't be used as a constant (?!).

Address *that* error by updating `App.cs` so that it emits a
`Type.GetType("_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z", true)`.  This allows
`jnimarshalmethod-gen.exe` to execute successfully.

Then we try *running* it, and it fails:

	$ make run-test-jnimarshal
	…
	EXEC : 1) error : Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods […/Java.Interop/build-tools/scripts/RunNUnitTests.targets]
	  Java.Interop.JavaException : com.xamarin.interop.export.ExportType.staticAction()V
	    at Java.Interop.JniEnvironment+StaticMethods.CallStaticVoidMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method)
	    at Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods ()
	    at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
	    at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
	    --- End of managed Java.Interop.JavaException stack trace ---
	  java.lang.UnsatisfiedLinkError: com.xamarin.interop.export.ExportType.staticAction()V
	  	at com.xamarin.interop.export.ExportType.staticAction(Native Method)
	  	at com.xamarin.interop.export.ExportType.testStaticMethods(ExportType.java:26)>

Add more printfs, and we get the problem:

	System.InvalidProgramException: Invalid IL code in Java.InteropTests.ExportTest/__<$>_jni_marshal_methods:__RegisterNativeMembers (Java.Interop.JniNativeMethodRegistrationArguments): IL_0176: ldloc.0

At this point things don't make sense to me.
The `__RegisterNativeMembers()` method is:

	## Dumping contents of `Java.InteropTests.ExportTest::__RegisterNativeMembers`:
	void (JniNativeMethodRegistrationArguments args)
	{
	    Type targetType;

	    targetType = Type.GetType("Java.InteropTests.ExportTest");
	    args.AddRegistrations(new JniNativeMethodRegistration[] {
	        new JniNativeMethodRegistration("funcIJavaObject", "()Ljava/lang/Object;",
	            Delegate.CreateDelegate(System.Func`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "FuncIJavaObject")),
	        new JniNativeMethodRegistration("funcInt64", "()J",
	            Delegate.CreateDelegate(System.Func`3[System.IntPtr,System.IntPtr,System.Int64], targetType, "FuncInt64")),
	        new JniNativeMethodRegistration("action", "()V",
	            Delegate.CreateDelegate(System.Action`2[System.IntPtr,System.IntPtr], targetType, "InstanceAction")),
	        new JniNativeMethodRegistration("actionIJavaObject", "(Ljava/lang/Object;)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "InstanceActionIJavaObject")),
	        new JniNativeMethodRegistration("staticAction", "()V",
	            Delegate.CreateDelegate(System.Action`2[System.IntPtr,System.IntPtr], targetType, "StaticAction")),
	        new JniNativeMethodRegistration("staticActionFloat", "(F)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.Single], targetType, "StaticActionFloat")),
	        new JniNativeMethodRegistration("staticActionIJavaObject", "(Ljava/lang/Object;)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "StaticActionIJavaObject")),
	        new JniNativeMethodRegistration("staticActionInt", "(I)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.Int32], targetType, "StaticActionInt")),
	        new JniNativeMethodRegistration("staticActionInt32String", "(ILjava/lang/String;)V",
	            Delegate.CreateDelegate(System.Action`4[System.IntPtr,System.IntPtr,System.Int32,System.IntPtr], targetType, "StaticActionInt32String")),
	        new JniNativeMethodRegistration("staticFuncMyLegacyColorMyColor_MyColor", "(II)I",
	            Delegate.CreateDelegate(System.Func`5[System.IntPtr,System.IntPtr,System.Int32,System.Int32,System.Int32], targetType, "StaticFuncMyLegacyColorMyColor_MyColor")),
	        new JniNativeMethodRegistration("staticFuncThisMethodTakesLotsOfParameters", "(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z",
	            Delegate.CreateDelegate((Type)Type.GetType("_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z", true), targetType, "StaticFuncThisMethodTakesLotsOfParameters"))
	    });
	}

The offending IL fragment:

	…
	IL_0161:  ldstr      "staticFuncThisMethodTakesLotsOfParameters"
	IL_0166:  ldstr      "(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z"
	IL_016b:  ldstr      "_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z"
	IL_0170:  ldc.i4.1
	IL_0171:  call       [mscorlib]System.Type [mscorlib]System.Type::GetType(string, bool)
	IL_0176:  ldloc.0
	IL_0177:  ldstr      "StaticFuncThisMethodTakesLotsOfParameters"
	IL_017c:  call       [mscorlib]System.Delegate [mscorlib]System.Delegate::CreateDelegate([mscorlib]System.Type, [mscorlib]System.Type, string)
	IL_0181:  newobj     instance void [Java.Interop]Java.Interop.JniNativeMethodRegistration::.ctor(string, string, [mscorlib]System.Delegate)

I don't understand where the `ldloc.0` is coming from, or why it's
there.

Punting a complete fix for now.

To repro the crash:

	$ rm -Rf bin/TestDebug
	$ msbuild
	$ msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_AllTheArguments=True /p:_DumpRegisterNativeMembers=True
	$ make run-test-jnimarshal

src/Java.Interop.Export/Java.Interop.Export.csproj

@@ -2,6 +2,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFramework>netstandard2.0</TargetFramework>
+    <LangVersion>8.0</LangVersion>
     <ProjectGuid>{B501D075-6183-4E1D-92C9-F7B5002475B1}</ProjectGuid>
     <SignAssembly>true</SignAssembly>
     <AssemblyOriginatorKeyFile>..\..\product.snk</AssemblyOriginatorKeyFile>

src/Java.Interop.Export/Java.Interop/MarshalMemberBuilder.cs

@@ -229,15 +229,64 @@ public LambdaExpression CreateMarshalToManagedExpression (MethodInfo method, Jav
 			}
 			foreach (var p in marshalParameters)
 				funcTypeParams.Add (p.Type);
-			if (ret != null)
-				funcTypeParams.Add (ret.Type);
-			var marshalerType = ret == null
-				? Expression.GetActionType (funcTypeParams.ToArray ())
-				: Expression.GetFuncType (funcTypeParams.ToArray ());
+			var marshalerType = GetMarshalerType (ret?.Type, funcTypeParams, method.DeclaringType);
 
 			bodyParams.AddRange (marshalParameters);
 			var body = Expression.Block (envpVars, envpBody);
-			return Expression.Lambda (marshalerType, body, bodyParams);
+
+			return marshalerType == null
+				? Expression.Lambda (body, bodyParams)
+				: Expression.Lambda (marshalerType, body, bodyParams);
+		}
+
+		static Type GetMarshalerType (Type returnType, List<Type> funcTypeParams, Type declaringType)
+		{
+			// `mscorlib.dll` & `System.Core.dll` only provide Action<…>/Func<…> types for up to 16 parameters
+			if (funcTypeParams.Count <= 16) {
+				if (returnType != null)
+					funcTypeParams.Add (returnType);
+				return returnType == null
+					? Expression.GetActionType (funcTypeParams.ToArray ())
+					: Expression.GetFuncType (funcTypeParams.ToArray ());
+			}
+
+			// Too many parameters; does a `_JniMarshal_*` type exist in the type's declaring assembly?
+			funcTypeParams.RemoveRange (0, 2);
+			var marshalDelegateName = new StringBuilder ();
+			marshalDelegateName.Append ("_JniMarshal_PP");
+			foreach (var paramType in funcTypeParams) {
+				marshalDelegateName.Append (GetJniMarshalDelegateParameterIdentifier (paramType));
+			}
+			marshalDelegateName.Append ("_");
+			if (returnType == null) {
+				marshalDelegateName.Append ("V");
+			} else {
+				marshalDelegateName.Append (GetJniMarshalDelegateParameterIdentifier (returnType));
+			}
+
+			Type marshalDelegateType = declaringType.Assembly.GetType (marshalDelegateName.ToString (), throwOnError: false);
+
+			// Punt?; System.Linq.Expressions will automagically produce the needed delegate type.
+			// Unfortunately, this won't work with jnimarshalmethod-gen.exe.
+			return marshalDelegateType;
+		}
+
+		static char GetJniMarshalDelegateParameterIdentifier (Type type)
+		{
+			if (type == typeof (bool))      return 'Z';
+			if (type == typeof (byte))      return 'B';
+			if (type == typeof (sbyte))     return 'B';
+			if (type == typeof (char))      return 'C';
+			if (type == typeof (short))     return 'S';
+			if (type == typeof (ushort))	return 's';
+			if (type == typeof (int))       return 'I';
+			if (type == typeof (uint))      return 'i';
+			if (type == typeof (long))      return 'J';
+			if (type == typeof (ulong))     return 'j';
+			if (type == typeof (float))     return 'F';
+			if (type == typeof (double))    return 'D';
+			if (type == typeof (void))      return 'V';
+			return 'L';
 		}
 
 		void CheckMarshalTypesMatch (MethodInfo method, string signature, ParameterInfo[] methodParameters)
@@ -258,7 +307,7 @@ void CheckMarshalTypesMatch (MethodInfo method, string signature, ParameterInfo[
 				var jni = vm.MarshalType;
 				if (mptypes [i] != jni)
 					throw new ArgumentException (
-							string.Format ("JNI parameter type mismatch. Type '{0}' != '{1}.", jni, mptypes [i]),
+							$"JNI parameter type mismatch. Type `{jni}` != `{mptypes [i]}` at index {i} in `{signature}`.",
 							"signature");
 			}
 

tests/Java.Interop.Export-Tests/Java.Interop/ExportTest.cs

@@ -6,6 +6,28 @@
 using Java.Interop;
 using Java.Interop.Expressions;
 
+// For use by `jnimarshalmethod-gen.exe` & `make run-test-jnimarshal`
+delegate bool _JniMarshal_PPZBCSIJFDLLLLLDFJ_Z (
+		IntPtr              jnienv,
+		IntPtr              klass,
+		bool                a,
+		sbyte               b,
+		char                c,
+		short               d,
+		int                 e,
+		long                f,
+		float               g,
+		double              h,
+		IntPtr              i,  // java.lang.Object
+		IntPtr              j,  // java.lang.String
+		IntPtr              k,  // java.util.ArrayList<String>
+		IntPtr              l,  // java.lang.String
+		IntPtr              m,  // java.lang.Object
+		double              n,
+		float               o,
+		long                p
+);
+
 namespace Java.InteropTests
 {
 	[JniTypeSignature ("com/xamarin/interop/export/ExportType")]
@@ -83,6 +105,60 @@ public static void StaticActionInt (int i)
 		public static void StaticActionFloat (float f)
 		{
 		}
+
+		[JavaCallable ("staticFuncThisMethodTakesLotsOfParameters", Signature="(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z")]
+		public static bool StaticFuncThisMethodTakesLotsOfParameters (
+				bool                a,
+				sbyte               b,
+				char                c,
+				short               d,
+				int                 e,
+				long                f,
+				float               g,
+				double              h,
+				IntPtr              i,  // java.lang.Object
+				IntPtr              j,  // java.lang.String
+				IntPtr              k,  // java.util.ArrayList<String>
+				IntPtr              l,  // java.lang.String
+				IntPtr              m,  // java.lang.Object
+				double              n,
+				float               o,
+				long                p)
+		{
+			if (a != false)
+				return false;
+			if (b != (byte) 0xb)
+				return false;
+			if (c != 'c')
+				return false;
+			if (d != (short) 0xd)
+				return false;
+			if (e != 0xe)
+				return false;
+			if (f != 0xf)
+				return false;
+			if (g != 1.0f)
+				return false;
+			if (h != 2.0)
+				return false;
+			if (i == IntPtr.Zero)
+				return false;
+			if (j == IntPtr.Zero)
+				return false;
+			if (k == IntPtr.Zero)
+				return false;
+			if (l == IntPtr.Zero)
+				return false;
+			if (m == IntPtr.Zero)
+				return false;
+			if (n != 3.0)
+				return false;
+			if (o != 4.0f)
+				return false;
+			if (p != 0x70)
+				return false;
+			return true;
+		}
 	}
 
 	[JniValueMarshaler (typeof (MyColorValueMarshaler))]

tests/Java.Interop.Export-Tests/Java.Interop/MarshalMemberBuilderTest.cs

@@ -22,7 +22,7 @@ public void AddExportMethods ()
 				var methods = CreateBuilder ()
 					.GetExportedMemberRegistrations (typeof (ExportTest))
 					.ToList ();
-				Assert.AreEqual (10, methods.Count);
+				Assert.AreEqual (11, methods.Count);
 
 				Assert.AreEqual ("action",  methods [0].Name);
 				Assert.AreEqual ("()V",     methods [0].Signature);

tests/Java.Interop.Export-Tests/java/com/xamarin/interop/export/ExportType.java

@@ -30,21 +30,62 @@ public static void testStaticMethods () {
 	public static native void staticActionInt32String (int i, String s);
 	public static native int  staticFuncMyLegacyColorMyColor_MyColor (int color1, int color2);
 
-	public void testMethods () {
-	    action ();
-
-	    actionIJavaObject (this);
-
-	    long j = funcInt64 ();
-	    if (j != 42)
-	        throw new Error ("funcInt64() should return 42!");
+	public static native boolean staticFuncThisMethodTakesLotsOfParameters (
+			boolean             a,
+			byte                b,
+			char                c,
+			short               d,
+			int                 e,
+			long                f,
+			float               g,
+			double              h,
+			Object              i,
+			String              j,
+			ArrayList<String>   k,
+			String              l,
+			Object              m,
+			double              n,
+			float               o,
+			long                p);
 
-	    Object o = funcIJavaObject ();
-	    if (o != this)
-	        throw new Error ("funcIJavaObject() should return `this`!");
-
-	    staticActionInt (1);
-	    staticActionFloat (2.0f);
+	public void testMethods () {
+		action ();
+
+		actionIJavaObject (this);
+
+		long j = funcInt64 ();
+		if (j != 42)
+			throw new Error ("funcInt64() should return 42!");
+
+		Object o = funcIJavaObject ();
+		if (o != this)
+			throw new Error ("funcIJavaObject() should return `this`!");
+
+		staticActionInt (1);
+		staticActionFloat (2.0f);
+
+		/*
+		boolean r = staticFuncThisMethodTakesLotsOfParameters (
+				false,
+				(byte) 0xb,
+				'c',
+				(short) 0xd,
+				0xe,
+				0xf,
+				1.0f,
+				2.0,
+				new Object (),
+				"j",
+				new ArrayList<String>(),
+				"l",
+				new Object (),
+				3.0,
+				4.0f,
+				0x70
+		);
+		if (r != true)
+			throw new Error ("staticFuncThisMethodTakesLotsOfParameters should return true!");
+		 */
 	}
 
 	public native void action ();

tools/jnimarshalmethod-gen/App.cs

@@ -13,6 +13,10 @@
 using Mono.Collections.Generic;
 using Java.Interop.Tools.Cecil;
 
+#if _DUMP_REGISTER_NATIVE_MEMBERS
+using Mono.Linq.Expressions;
+#endif  // _DUMP_REGISTER_NATIVE_MEMBERS
+
 namespace Xamarin.Android.Tools.JniMarshalMethodGenerator {
 
 	class App : MarshalByRefObject
@@ -393,6 +397,13 @@ void CreateMarshalMethodAssembly (string path)
 							continue;
 					}
 
+#if !_ALL_THE_ARGUMENTS
+					if (method.GetParameters ().Length > 14) {
+						Warning ($"Methods taking more than 14 parameters is not supported.");
+						continue;
+					}
+#endif	// !_ALL_THE_ARGUMENTS
+
 					if (dt == null)
 						dt = GetTypeBuilder (dm, type);
 
@@ -467,7 +478,20 @@ void CreateMarshalMethodAssembly (string path)
 
 		static Expression CreateRegistration (string method, string signature, LambdaExpression lambda, ParameterExpression targetType, string methodName)
 		{
-			var d = Expression.Call (Delegate_CreateDelegate, Expression.Constant (lambda.Type, typeof (Type)), targetType, Expression.Constant (methodName));
+			Expression registrationDelegateType = null;
+			if (lambda.Type.Assembly == typeof (object).Assembly ||
+					lambda.Type.Assembly == typeof (System.Linq.Enumerable).Assembly) {
+				registrationDelegateType = Expression.Constant (lambda.Type, typeof (Type));
+			}
+			else {
+				Func<string, bool, Type> getType = Type.GetType;
+				registrationDelegateType = Expression.Call (getType.GetMethodInfo (),
+						Expression.Constant (lambda.Type.FullName, typeof (string)),
+						Expression.Constant (true, typeof (bool)));
+				registrationDelegateType = Expression.Convert (registrationDelegateType, typeof (Type));
+			}
+
+			var d = Expression.Call (Delegate_CreateDelegate, registrationDelegateType, targetType, Expression.Constant (methodName));
 			return Expression.New (JniNativeMethodRegistration_ctor,
 					Expression.Constant (method),
 					Expression.Constant (signature),
@@ -488,6 +512,10 @@ static void AddRegisterNativeMembers (TypeBuilder dt, ParameterExpression target
 			var rb = dt.DefineMethod ("__RegisterNativeMembers",
 					System.Reflection.MethodAttributes.Public | System.Reflection.MethodAttributes.Static);
 			rb.SetCustomAttribute (new CustomAttributeBuilder (typeof (JniAddNativeMethodRegistrationAttribute).GetConstructor (Type.EmptyTypes), new object[0]));
+#if _DUMP_REGISTER_NATIVE_MEMBERS
+			Console.WriteLine ($"## Dumping contents of `{dt.FullName}::__RegisterNativeMembers`: ");
+			Console.WriteLine (lambda.ToCSharpCode ());
+#endif  // _DUMP_REGISTER_NATIVE_MEMBERS
 			lambda.CompileToMethod (rb);
 		}
 

tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj

@@ -12,11 +12,23 @@
   <PropertyGroup>
     <OutputPath>$(UtilityOutputFullPath)</OutputPath>
   </PropertyGroup>
+
+  <PropertyGroup Condition=" '$(_DumpRegisterNativeMembers)' == 'True' ">
+    <DefineConstants>_DUMP_REGISTER_NATIVE_MEMBERS;$(DefineConstants)</DefineConstants>
+  </PropertyGroup>
+
+  <PropertyGroup Condition=" '$(_AllTheArguments)' == 'True' ">
+    <DefineConstants>_ALL_THE_ARGUMENTS;$(DefineConstants)</DefineConstants>
+  </PropertyGroup>
 
   <ItemGroup>
     <PackageReference Include="Mono.Options" Version="5.3.0.1" />
   </ItemGroup>
 
+  <ItemGroup Condition=" '$(_DumpRegisterNativeMembers)' == 'True' ">
+    <PackageReference Include="Mono.Linq.Expressions" Version="2.0.0" />
+  </ItemGroup>
+
   <ItemGroup>
     <ProjectReference Include="..\..\src\Java.Interop.Export\Java.Interop.Export.csproj" />
     <ProjectReference Include="..\..\src\Java.Interop\Java.Interop.csproj" />