From 8074e038d2766a0b2887bca46ede9d66050f15c8 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 29 Apr 2020 21:03:04 -0400 Subject: [PATCH] [Java.Interop.Export] Begin supporting methods with 14+ params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/xamarin-android/pull/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: 70fc4c0771ca7f7ea346f221eca83fb109732a5d Commit 70fc4c07 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. (Hopefully this in itself will be enough for xamarin-android to build.) 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 like 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 cbb50af2, 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 --- .../Java.Interop.Export.csproj | 1 + .../Java.Interop/MarshalMemberBuilder.cs | 63 +++++++++++++-- .../Java.Interop/ExportTest.cs | 76 +++++++++++++++++++ .../Java.Interop/MarshalMemberBuilderTest.cs | 2 +- .../xamarin/interop/export/ExportType.java | 69 +++++++++++++---- tools/jnimarshalmethod-gen/App.cs | 30 +++++++- ...oid.Tools.JniMarshalMethodGenerator.csproj | 12 +++ 7 files changed, 230 insertions(+), 23 deletions(-) diff --git a/src/Java.Interop.Export/Java.Interop.Export.csproj b/src/Java.Interop.Export/Java.Interop.Export.csproj index 84f23fba6..2ed63dad0 100644 --- a/src/Java.Interop.Export/Java.Interop.Export.csproj +++ b/src/Java.Interop.Export/Java.Interop.Export.csproj @@ -2,6 +2,7 @@ netstandard2.0 + 8.0 {B501D075-6183-4E1D-92C9-F7B5002475B1} true ..\..\product.snk diff --git a/src/Java.Interop.Export/Java.Interop/MarshalMemberBuilder.cs b/src/Java.Interop.Export/Java.Interop/MarshalMemberBuilder.cs index 0cacc6f4f..69919d646 100644 --- a/src/Java.Interop.Export/Java.Interop/MarshalMemberBuilder.cs +++ b/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 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"); } diff --git a/tests/Java.Interop.Export-Tests/Java.Interop/ExportTest.cs b/tests/Java.Interop.Export-Tests/Java.Interop/ExportTest.cs index dd7459f6e..877448bde 100644 --- a/tests/Java.Interop.Export-Tests/Java.Interop/ExportTest.cs +++ b/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 + 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 + 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))] diff --git a/tests/Java.Interop.Export-Tests/Java.Interop/MarshalMemberBuilderTest.cs b/tests/Java.Interop.Export-Tests/Java.Interop/MarshalMemberBuilderTest.cs index 41c3e658c..4aa607aa4 100644 --- a/tests/Java.Interop.Export-Tests/Java.Interop/MarshalMemberBuilderTest.cs +++ b/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); diff --git a/tests/Java.Interop.Export-Tests/java/com/xamarin/interop/export/ExportType.java b/tests/Java.Interop.Export-Tests/java/com/xamarin/interop/export/ExportType.java index 482ba455c..01108ae14 100644 --- a/tests/Java.Interop.Export-Tests/java/com/xamarin/interop/export/ExportType.java +++ b/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 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(), + "l", + new Object (), + 3.0, + 4.0f, + 0x70 + ); + if (r != true) + throw new Error ("staticFuncThisMethodTakesLotsOfParameters should return true!"); + */ } public native void action (); diff --git a/tools/jnimarshalmethod-gen/App.cs b/tools/jnimarshalmethod-gen/App.cs index eaebb9903..d752a1c04 100644 --- a/tools/jnimarshalmethod-gen/App.cs +++ b/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 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); } diff --git a/tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj b/tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj index 17ea1539e..97ffb58df 100644 --- a/tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj +++ b/tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj @@ -12,11 +12,23 @@ $(UtilityOutputFullPath) + + + _DUMP_REGISTER_NATIVE_MEMBERS;$(DefineConstants) + + + + _ALL_THE_ARGUMENTS;$(DefineConstants) + + + + +