From 3f6fc67948d8809ba81bc61ba1f2332d78dab956 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 29 Mar 2022 22:01:22 +0200 Subject: [PATCH 01/10] [marshal methods] Part 2 of ? Implement a marshal method candidate classifier which is passed to JavaCallableWrapperGenerator in order to identify methods that can be registered "statically" and for which we can generate native code instead of using System.Reflection.Emit. The selected methods are marked with the `[UnmanagedCallersOnly]` attribute and their support code in the form of the connector method (which returns a delegate to the native callback) as well as its backing field which stores the delegate, are removed from the assemblies. --- Java.Interop.diff | 283 ++++++++++++++ .../Tasks/GenerateJavaStubs.cs | 78 +++- .../Tasks/GeneratePackageManagerJava.cs | 4 +- .../MarshalMethodsAssemblyRewriter.cs | 134 +++++++ .../Utilities/MarshalMethodsClassifier.cs | 354 ++++++++++++++++++ .../MarshalMethodsNativeAssemblyGenerator.cs | 17 +- .../Utilities/MarshalMethodsState.cs | 17 + src/monodroid/CMakeLists.txt | 12 +- src/monodroid/jni/embedded-assemblies.cc | 4 + src/monodroid/jni/monodroid-glue.cc | 6 +- src/monodroid/jni/xamarin-app.hh | 2 +- 11 files changed, 881 insertions(+), 30 deletions(-) create mode 100644 Java.Interop.diff create mode 100644 src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs create mode 100644 src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs create mode 100644 src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs diff --git a/Java.Interop.diff b/Java.Interop.diff new file mode 100644 index 00000000000..761d72c255c --- /dev/null +++ b/Java.Interop.diff @@ -0,0 +1,283 @@ +diff --git a/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs b/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs +index ec1ea955..7f8e1a1b 100644 +--- a/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs ++++ b/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs +@@ -1,5 +1,7 @@ + using System; + ++using Mono.Cecil; ++ + namespace Android.Runtime { + + [AttributeUsage (AttributeTargets.Class | AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Interface | AttributeTargets.Method | AttributeTargets.Property)] +@@ -23,7 +25,21 @@ namespace Android.Runtime { + this.connector = connector; + this.signature = signature; + } ++#if HAVE_CECIL ++ public RegisterAttribute (string name, CustomAttribute originAttribute) ++ : this (name) ++ { ++ OriginAttribute = originAttribute; ++ } ++ ++ public RegisterAttribute (string name, string signature, string connector, CustomAttribute originAttribute) ++ : this (name, signature, connector) ++ { ++ OriginAttribute = originAttribute; ++ } + ++ public CustomAttribute OriginAttribute { get; } ++#endif + public string Connector { + get { return connector; } + set { connector = value; } +diff --git a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs +index d30bce4a..afde1e48 100644 +--- a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs ++++ b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs +@@ -25,40 +25,9 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + JavaInterop1, + } + +- public class OverriddenMethodDescriptor ++ public abstract class JavaCallableMethodClassifier + { +- static readonly char[] methodDescSplitChars = new char[] { ':' }; +- +- public string JavaPackageName { get; } +- public string NativeName { get; } +- public string JniSignature { get; } +- public string Connector { get; } +- public string ManagedTypeName { get; } +- public string OriginalDescString { get; } +- +- public OverriddenMethodDescriptor (string javaPackageName, string methodDescription, string fallbackManagedTypeName) +- { +- OriginalDescString = methodDescription; +- JavaPackageName = javaPackageName; +- string[] parts = methodDescription.Split (methodDescSplitChars, 4); +- +- if (parts.Length < 2) { +- throw new InvalidOperationException ($"Unexpected format for method description. Expected at least 2 parts, got {parts.Length} from: '{methodDescription}'"); +- } +- +- NativeName = parts[0]; +- JniSignature = parts[1]; +- if (parts.Length > 2) { +- Connector = parts[2]; +- if (parts.Length > 3) { +- ManagedTypeName = TypeDefinitionRocks.CecilTypeNameToReflectionTypeName (parts[3]); +- } +- } +- +- if (String.IsNullOrEmpty (ManagedTypeName)) { +- ManagedTypeName = fallbackManagedTypeName; +- } +- } ++ public abstract bool ShouldBeDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute); + } + + public class JavaCallableWrapperGenerator { +@@ -95,21 +64,22 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + List methods = new List (); + List ctors = new List (); + List children; +- List overriddenMethodDescriptors; ++ + readonly IMetadataResolver cache; ++ readonly JavaCallableMethodClassifier methodClassifier; + + [Obsolete ("Use the TypeDefinitionCache overload for better performance.")] + public JavaCallableWrapperGenerator (TypeDefinition type, Action log) + : this (type, null, log, resolver: null) + { } + +- public JavaCallableWrapperGenerator (TypeDefinition type, Action log, TypeDefinitionCache cache) +- : this (type, log, (IMetadataResolver) cache) ++ public JavaCallableWrapperGenerator (TypeDefinition type, Action log, TypeDefinitionCache cache, JavaCallableMethodClassifier methodClassifier = null) ++ : this (type, log, (IMetadataResolver) cache, methodClassifier) + { + } + +- public JavaCallableWrapperGenerator (TypeDefinition type, Action log, IMetadataResolver resolver) +- : this (type, null, log, resolver) ++ public JavaCallableWrapperGenerator (TypeDefinition type, Action log, IMetadataResolver resolver, JavaCallableMethodClassifier methodClassifier = null) ++ : this (type, null, log, resolver, methodClassifier) + { + if (type.HasNestedTypes) { + children = new List (); +@@ -117,7 +87,6 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + } + } + +- public IList OverriddenMethodDescriptors => overriddenMethodDescriptors; + public string ApplicationJavaClass { get; set; } + public JavaPeerStyle CodeGenerationTarget { get; set; } + +@@ -125,6 +94,9 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + + public bool HasExport { get; private set; } + ++ // If there are no methods, we need to generate "empty" registration because of backward compatibility ++ public bool HasDynamicallyRegisteredMethods => methods.Count == 0 || methods.Any ((Signature sig) => sig.IsDynamicallyRegistered); ++ + /// + /// The Java source code to be included in Instrumentation.onCreate + /// +@@ -152,8 +124,9 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + HasExport |= children.Any (t => t.HasExport); + } + +- JavaCallableWrapperGenerator (TypeDefinition type, string outerType, Action log, IMetadataResolver resolver) ++ JavaCallableWrapperGenerator (TypeDefinition type, string outerType, Action log, IMetadataResolver resolver, JavaCallableMethodClassifier methodClassifier = null) + { ++ this.methodClassifier = methodClassifier; + this.type = type; + this.log = log; + this.cache = resolver ?? new TypeDefinitionCache (); +@@ -366,12 +339,13 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + // attr.Resolve (); + RegisterAttribute r = null; + if (attr.ConstructorArguments.Count == 1) +- r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value); ++ r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value, attr); + else if (attr.ConstructorArguments.Count == 3) + r = new RegisterAttribute ( + (string) attr.ConstructorArguments [0].Value, + (string) attr.ConstructorArguments [1].Value, +- (string) attr.ConstructorArguments [2].Value); ++ (string) attr.ConstructorArguments [2].Value, ++ attr); + if (r != null) { + var v = attr.Properties.FirstOrDefault (p => p.Name == "DoNotGenerateAcw"); + r.DoNotGenerateAcw = v.Name == null ? false : (bool) v.Argument.Value; +@@ -384,7 +358,7 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + // attr.Resolve (); + RegisterAttribute r = null; + if (attr.ConstructorArguments.Count == 1) +- r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value); ++ r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value, attr); + if (r != null) { + var v = attr.Properties.FirstOrDefault (p => p.Name == "GenerateJavaPeer"); + if (v.Name == null) { +@@ -403,7 +377,8 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + if (attr.ConstructorArguments.Count == 2) + r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value, + (string) attr.ConstructorArguments [1].Value, +- ""); ++ "", ++ attr); + return r; + } + +@@ -467,7 +442,8 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + if (attr.Name.Contains ("-impl") || (attr.Name.Length > 7 && attr.Name[attr.Name.Length - 8] == '-')) + Diagnostic.Error (4217, LookupSource (implementedMethod), Localization.Resources.JavaCallableWrappers_XA4217, attr.Name); + +- var msig = new Signature (implementedMethod, attr); ++ bool shouldBeDynamicallyRegistered = methodClassifier?.ShouldBeDynamicallyRegistered (registeredMethod, implementedMethod, attr.OriginAttribute) ?? true; ++ var msig = new Signature (implementedMethod, attr, shouldBeDynamicallyRegistered); + if (!registeredMethod.IsConstructor && !methods.Any (m => m.Name == msig.Name && m.Params == msig.Params)) + methods.Add (msig); + } +@@ -542,21 +518,38 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + + GenerateHeader (writer); + +- writer.WriteLine ("/** @hide */"); +- writer.WriteLine ("\tpublic static final String __md_methods;"); +- if (children != null) { +- foreach (var i in Enumerable.Range (1, children.Count)) +- writer.WriteLine ("\tstatic final String __md_{0}_methods;", i); ++ bool needCtor = false; ++ if (HasDynamicallyRegisteredMethods) { ++ needCtor = true; ++ writer.WriteLine ("/** @hide */"); ++ writer.WriteLine ("\tpublic static final String __md_methods;"); + } +- writer.WriteLine ("\tstatic {"); +- GenerateRegisterType (writer, this, "__md_methods"); ++ + if (children != null) { +- for (int i = 0; i < children.Count; ++i) { +- string methods = string.Format ("__md_{0}_methods", i + 1); +- GenerateRegisterType (writer, children [i], methods); ++ for (int i = 0; i < children.Count; i++) { ++ if (!children[i].HasDynamicallyRegisteredMethods) { ++ continue; ++ } ++ needCtor = true; ++ writer.WriteLine ("\tstatic final String __md_{0}_methods;", i + 1); ++ } ++ } ++ ++ if (needCtor) { ++ writer.WriteLine ("\tstatic {"); ++ ++ if (HasDynamicallyRegisteredMethods) { ++ GenerateRegisterType (writer, this, "__md_methods"); ++ } ++ ++ if (children != null) { ++ for (int i = 0; i < children.Count; ++i) { ++ string methods = string.Format ("__md_{0}_methods", i + 1); ++ GenerateRegisterType (writer, children [i], methods); ++ } + } ++ writer.WriteLine ("\t}"); + } +- writer.WriteLine ("\t}"); + + GenerateBody (writer); + +@@ -710,16 +703,19 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + + void GenerateRegisterType (TextWriter sw, JavaCallableWrapperGenerator self, string field) + { +- if (overriddenMethodDescriptors == null) { +- overriddenMethodDescriptors = new List (); ++ string managedTypeName = self.type.GetPartialAssemblyQualifiedName (cache); ++ string javaTypeName = $"{package}.{name}"; ++ ++ if (!self.HasDynamicallyRegisteredMethods) { ++ return; + } + + sw.WriteLine ("\t\t{0} = ", field); +- string managedTypeName = self.type.GetPartialAssemblyQualifiedName (cache); +- string javaTypeName = $"{package}.{name}"; ++ + foreach (Signature method in self.methods) { +- sw.WriteLine ("\t\t\t\"{0}\\n\" +", method.Method); +- overriddenMethodDescriptors.Add (new OverriddenMethodDescriptor (javaTypeName, method.Method, managedTypeName)); ++ if (method.IsDynamicallyRegistered) { ++ sw.WriteLine ("\t\t\t\"{0}\\n\" +", method.Method); ++ } + } + sw.WriteLine ("\t\t\t\"\";"); + if (CannotRegisterInStaticConstructor (self.type)) +@@ -772,12 +768,13 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + + class Signature { + +- public Signature (MethodDefinition method, RegisterAttribute register) : this (method, register, null, null) {} ++ public Signature (MethodDefinition method, RegisterAttribute register, bool shouldBeDynamicallyRegistered = true) : this (method, register, null, null, shouldBeDynamicallyRegistered) {} + +- public Signature (MethodDefinition method, RegisterAttribute register, string managedParameters, string outerType) ++ public Signature (MethodDefinition method, RegisterAttribute register, string managedParameters, string outerType, bool shouldBeDynamicallyRegistered = true) + : this (register.Name, register.Signature, register.Connector, managedParameters, outerType, null) + { + Annotations = JavaCallableWrapperGenerator.GetAnnotationsString ("\t", method.CustomAttributes); ++ IsDynamicallyRegistered = shouldBeDynamicallyRegistered; + } + + public Signature (MethodDefinition method, ExportAttribute export, IMetadataResolver cache) +@@ -880,6 +877,7 @@ namespace Java.Interop.Tools.JavaCallableWrappers { + public readonly string Method; + public readonly bool IsExport; + public readonly bool IsStatic; ++ public readonly bool IsDynamicallyRegistered = true; + public readonly string [] ThrownTypeNames; + public readonly string Annotations; + } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index 00e08ff4305..160f25acb78 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -124,7 +124,12 @@ void Run (DirectoryAssemblyResolver res) if (Directory.Exists (dir.ItemSpec)) res.SearchDirectories.Add (dir.ItemSpec); } - +#if ENABLE_MARSHAL_METHODS + Dictionary> marshalMethodsAssemblyPaths = null; + if (!Debug) { + marshalMethodsAssemblyPaths = new Dictionary> (StringComparer.Ordinal); + } +#endif // Put every assembly we'll need in the resolver bool hasExportReference = false; bool haveMonoAndroid = false; @@ -159,6 +164,11 @@ void Run (DirectoryAssemblyResolver res) } res.Load (assembly.ItemSpec); +#if ENABLE_MARSHAL_METHODS + if (!Debug) { + StoreMarshalAssemblyPath (Path.GetFileNameWithoutExtension (assembly.ItemSpec), assembly); + } +#endif } // However we only want to look for JLO types in user code for Java stub code generation @@ -172,6 +182,11 @@ void Run (DirectoryAssemblyResolver res) string name = Path.GetFileNameWithoutExtension (asm.ItemSpec); if (!userAssemblies.ContainsKey (name)) userAssemblies.Add (name, asm.ItemSpec); +#if ENABLE_MARSHAL_METHODS + if (!Debug) { + StoreMarshalAssemblyPath (name, asm); + } +#endif } // Step 1 - Find all the JLO types @@ -182,10 +197,6 @@ void Run (DirectoryAssemblyResolver res) List allJavaTypes = scanner.GetJavaTypes (allTypemapAssemblies, res); - // Step 2 - Generate type maps - // Type mappings need to use all the assemblies, always. - WriteTypeMappings (allJavaTypes, cache); - var javaTypes = new List (); foreach (TypeDefinition td in allJavaTypes) { if (!userAssemblies.ContainsKey (td.Module.Assembly.Name.Name) || JavaTypeScanner.ShouldSkipJavaCallableWrapperGeneration (td, cache)) { @@ -195,11 +206,27 @@ void Run (DirectoryAssemblyResolver res) javaTypes.Add (td); } - // Step 3 - Generate Java stub code - var success = CreateJavaSources (javaTypes, cache); + MarshalMethodsClassifier classifier = null; +#if ENABLE_MARSHAL_METHODS + if (!Debug) { + classifier = new MarshalMethodsClassifier (cache, res, Log); + } +#endif + // Step 2 - Generate Java stub code + var success = CreateJavaSources (javaTypes, cache, res, classifier); if (!success) return; +#if ENABLE_MARSHAL_METHODS + if (!Debug) { + var rewriter = new MarshalMethodsAssemblyRewriter (classifier.MarshalMethods, classifier.Assemblies, marshalMethodsAssemblyPaths); + rewriter.Rewrite (res); + } +#endif + // Step 3 - Generate type maps + // Type mappings need to use all the assemblies, always. + WriteTypeMappings (allJavaTypes, cache); + // We need to save a map of .NET type -> ACW type for resource file fixups var managed = new Dictionary (javaTypes.Count, StringComparer.Ordinal); var java = new Dictionary (javaTypes.Count, StringComparer.Ordinal); @@ -210,7 +237,7 @@ void Run (DirectoryAssemblyResolver res) using (var acw_map = MemoryStreamPool.Shared.CreateStreamWriter ()) { foreach (TypeDefinition type in javaTypes) { string managedKey = type.FullName.Replace ('/', '.'); - string javaKey = JavaNativeTypeManager.ToJniName (type).Replace ('/', '.'); + string javaKey = JavaNativeTypeManager.ToJniName (type, cache).Replace ('/', '.'); acw_map.Write (type.GetPartialAssemblyQualifiedName (cache)); acw_map.Write (';'); @@ -332,17 +359,32 @@ void Run (DirectoryAssemblyResolver res) string applicationTemplateFile = "ApplicationRegistration.java"; SaveResource (applicationTemplateFile, applicationTemplateFile, real_app_dir, template => template.Replace ("// REGISTER_APPLICATION_AND_INSTRUMENTATION_CLASSES_HERE", regCallsWriter.ToString ())); + +#if ENABLE_MARSHAL_METHODS + void StoreMarshalAssemblyPath (string name, ITaskItem asm) + { + if (Debug) { + return; + } + + if (!marshalMethodsAssemblyPaths.TryGetValue (name, out HashSet assemblyPaths)) { + assemblyPaths = new HashSet (); + marshalMethodsAssemblyPaths.Add (name, assemblyPaths); + } + + if (!assemblyPaths.Contains (asm.ItemSpec)) { + assemblyPaths.Add (asm.ItemSpec); + } + } +#endif } - bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCache cache) + bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCache cache, DirectoryAssemblyResolver resolver, MarshalMethodsClassifier classifier) { string outputPath = Path.Combine (OutputDirectory, "src"); string monoInit = GetMonoInitSource (AndroidSdkPlatform); bool hasExportReference = ResolvedAssemblies.Any (assembly => Path.GetFileName (assembly.ItemSpec) == "Mono.Android.Export.dll"); bool generateOnCreateOverrides = int.Parse (AndroidSdkPlatform) <= 10; -#if ENABLE_MARSHAL_METHODS - var overriddenMethodDescriptors = new List (); -#endif bool ok = true; foreach (var t in javaTypes) { @@ -353,7 +395,7 @@ bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCac using (var writer = MemoryStreamPool.Shared.CreateStreamWriter ()) { try { - var jti = new JavaCallableWrapperGenerator (t, Log.LogWarning, cache) { + var jti = new JavaCallableWrapperGenerator (t, Log.LogWarning, cache, classifier) { GenerateOnCreateOverrides = generateOnCreateOverrides, ApplicationJavaClass = ApplicationJavaClass, MonoRuntimeInitialization = monoInit, @@ -361,7 +403,11 @@ bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCac jti.Generate (writer); #if ENABLE_MARSHAL_METHODS - overriddenMethodDescriptors.AddRange (jti.OverriddenMethodDescriptors); + if (!Debug) { + if (classifier.FoundDynamicallyRegisteredMethods) { + Log.LogWarning ($"Type '{t.GetAssemblyQualifiedName ()}' will register some of its Java override methods dynamically. This may adversely affect runtime performance. See preceding warnings for names of dynamically registered methods."); + } + } #endif writer.Flush (); @@ -397,7 +443,9 @@ bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCac } } #if ENABLE_MARSHAL_METHODS - BuildEngine4.RegisterTaskObjectAssemblyLocal (MarshalMethodsRegisterTaskKey, overriddenMethodDescriptors, RegisteredTaskObjectLifetime.Build); + if (!Debug) { + BuildEngine4.RegisterTaskObjectAssemblyLocal (MarshalMethodsRegisterTaskKey, new MarshalMethodsState (classifier.MarshalMethods), RegisteredTaskObjectLifetime.Build); + } #endif return ok; } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs index 5b67610cb6e..b6f6f92b05c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs @@ -442,10 +442,12 @@ void AddEnvironment () }; appConfigAsmGen.Init (); #if ENABLE_MARSHAL_METHODS + var marshalMethodsState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (GenerateJavaStubs.MarshalMethodsRegisterTaskKey, RegisteredTaskObjectLifetime.Build); + var marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator () { NumberOfAssembliesInApk = assemblyCount, UniqueAssemblyNames = uniqueAssemblyNames, - OverriddenMethodDescriptors = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal> (GenerateJavaStubs.MarshalMethodsRegisterTaskKey, RegisteredTaskObjectLifetime.Build) + MarshalMethods = marshalMethodsState?.MarshalMethods, }; marshalMethodsAsmGen.Init (); #endif diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs new file mode 100644 index 00000000000..524411ed9ce --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs @@ -0,0 +1,134 @@ +#if ENABLE_MARSHAL_METHODS +using System; +using System.Collections.Generic; +using System.IO; + +using Java.Interop.Tools.Cecil; +using Mono.Cecil; + +namespace Xamarin.Android.Tasks +{ + class MarshalMethodsAssemblyRewriter + { + IDictionary methods; + ICollection uniqueAssemblies; + IDictionary > assemblyPaths; + + public MarshalMethodsAssemblyRewriter (IDictionary methods, ICollection uniqueAssemblies, IDictionary > assemblyPaths) + { + this.methods = methods ?? throw new ArgumentNullException (nameof (methods)); + this.uniqueAssemblies = uniqueAssemblies ?? throw new ArgumentNullException (nameof (uniqueAssemblies)); + this.assemblyPaths = assemblyPaths ?? throw new ArgumentNullException (nameof (assemblyPaths)); + } + + public void Rewrite (DirectoryAssemblyResolver resolver) + { + var unmanagedCallersOnlyAttributes = new Dictionary (); + foreach (AssemblyDefinition asm in uniqueAssemblies) { + unmanagedCallersOnlyAttributes.Add (asm, GetUnmanagedCallersOnlyAttribute (asm, resolver)); + } + + Console.WriteLine ("Adding the [UnmanagedCallersOnly] attribute to native callback methods and removing unneeded fields+methods"); + foreach (MarshalMethodEntry method in methods.Values) { + Console.WriteLine ($"\t{method.NativeCallback.FullName} (token: 0x{method.NativeCallback.MetadataToken.RID:x})"); + method.NativeCallback.CustomAttributes.Add (unmanagedCallersOnlyAttributes [method.NativeCallback.Module.Assembly]); + method.Connector.DeclaringType.Methods.Remove (method.Connector); + method.CallbackField?.DeclaringType.Fields.Remove (method.CallbackField); + } + + Console.WriteLine (); + Console.WriteLine ("Rewriting assemblies"); + + var newAssemblyPaths = new List (); + foreach (AssemblyDefinition asm in uniqueAssemblies) { + foreach (string path in GetAssemblyPaths (asm)) { + var writerParams = new WriterParameters { + WriteSymbols = (File.Exists (path + ".mdb") || File.Exists (Path.ChangeExtension (path, ".pdb"))), + }; + + string output = $"{path}.new"; + Console.WriteLine ($"\t{asm.Name} => {output}"); + asm.Write (output, writerParams); + newAssemblyPaths.Add (output); + } + } + + // Replace old versions of the assemblies only after we've finished rewriting without issues, otherwise leave the new + // versions around. + foreach (string path in newAssemblyPaths) { + string target = Path.Combine (Path.GetDirectoryName (path), Path.GetFileNameWithoutExtension (path)); + MoveFile (path, target); + + string source = Path.ChangeExtension (path, ".pdb"); + if (File.Exists (source)) { + target = Path.ChangeExtension (Path.Combine (Path.GetDirectoryName (source), Path.GetFileNameWithoutExtension (source)), ".pdb"); + + MoveFile (source, target); + } + + source = $"{path}.mdb"; + if (File.Exists (source)) { + target = Path.ChangeExtension (path, ".mdb"); + MoveFile (source, target); + } + } + + Console.WriteLine (); + Console.WriteLine ("Method tokens:"); + foreach (MarshalMethodEntry method in methods.Values) { + Console.WriteLine ($"\t{method.NativeCallback.FullName} (token: 0x{method.NativeCallback.MetadataToken.RID:x})"); + } + + void MoveFile (string source, string target) + { + Console.WriteLine ($"Moving '{source}' => '{target}'"); + if (File.Exists (target)) { + File.Delete (target); + } + + File.Move (source, target); + } + } + + ICollection GetAssemblyPaths (AssemblyDefinition asm) + { + if (!assemblyPaths.TryGetValue (asm.Name.Name, out HashSet paths)) { + throw new InvalidOperationException ($"Unable to determine file path for assembly '{asm.Name.Name}'"); + } + + return paths; + } + + CustomAttribute GetUnmanagedCallersOnlyAttribute (AssemblyDefinition targetAssembly, DirectoryAssemblyResolver resolver) + { + AssemblyDefinition asm = resolver.Resolve ("System.Runtime.InteropServices"); + TypeDefinition unmanagedCallersOnlyAttribute = null; + foreach (ModuleDefinition md in asm.Modules) { + foreach (ExportedType et in md.ExportedTypes) { + if (!et.IsForwarder) { + continue; + } + + if (String.Compare ("System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute", et.FullName, StringComparison.Ordinal) != 0) { + continue; + } + + unmanagedCallersOnlyAttribute = et.Resolve (); + break; + } + } + + MethodDefinition attrConstructor = null; + foreach (MethodDefinition md in unmanagedCallersOnlyAttribute.Methods) { + if (!md.IsConstructor) { + continue; + } + + attrConstructor = md; + } + + return new CustomAttribute (targetAssembly.MainModule.ImportReference (attrConstructor)); + } + } +} +#endif diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs new file mode 100644 index 00000000000..eea4007bfba --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs @@ -0,0 +1,354 @@ +using System; +using System.Collections.Generic; + +using Java.Interop.Tools.Cecil; +using Java.Interop.Tools.JavaCallableWrappers; +using Microsoft.Android.Build.Tasks; +using Microsoft.Build.Utilities; +using Mono.Cecil; + +namespace Xamarin.Android.Tasks +{ +#if ENABLE_MARSHAL_METHODS + public sealed class MarshalMethodEntry + { + public MethodDefinition NativeCallback { get; } + public MethodDefinition Connector { get; } + public MethodDefinition TargetMethod { get; } + public FieldDefinition CallbackField { get; } + + public MarshalMethodEntry (MethodDefinition nativeCallback, MethodDefinition connector, MethodDefinition targetMethod, FieldDefinition callbackField) + { + NativeCallback = nativeCallback ?? throw new ArgumentNullException (nameof (nativeCallback)); + Connector = connector ?? throw new ArgumentNullException (nameof (connector)); + TargetMethod = targetMethod ?? throw new ArgumentNullException (nameof (targetMethod)); + CallbackField = callbackField; + } + } +#endif + + class MarshalMethodsClassifier : JavaCallableMethodClassifier + { +#if ENABLE_MARSHAL_METHODS + sealed class ConnectorInfo + { + public string MethodName { get; } + public string TypeName { get; } + public AssemblyNameReference AssemblyName { get; } + + public ConnectorInfo (string spec) + { + string[] connectorSpec = spec.Split (':'); + MethodName = connectorSpec[0]; + + if (connectorSpec.Length < 2) { + return; + } + + string fullTypeName = connectorSpec[1]; + int comma = fullTypeName.IndexOf (','); + TypeName = fullTypeName.Substring (0, comma); + AssemblyName = AssemblyNameReference.Parse (fullTypeName.Substring (comma + 1).Trim ()); + } + } + + interface IMethodSignatureMatcher + { + bool Matches (MethodDefinition method); + } + + sealed class NativeCallbackSignature : IMethodSignatureMatcher + { + static readonly HashSet verbatimTypes = new HashSet (StringComparer.Ordinal) { + "System.Boolean", + "System.Byte", + "System.Char", + "System.Double", + "System.Int16", + "System.Int32", + "System.Int64", + "System.IntPtr", + "System.SByte", + "System.Single", + "System.UInt16", + "System.UInt32", + "System.UInt64", + "System.Void", + }; + + readonly List paramTypes; + readonly string returnType; + readonly TaskLoggingHelper log; + + public NativeCallbackSignature (MethodDefinition target, TaskLoggingHelper log) + { + this.log = log; + returnType = MapType (target.ReturnType.FullName); + paramTypes = new List { + "System.IntPtr", // jnienv + "System.IntPtr", // native__this + }; + + foreach (ParameterDefinition pd in target.Parameters) { + paramTypes.Add (MapType (pd.ParameterType.FullName)); + } + } + + string MapType (string type) + { + if (verbatimTypes.Contains (type)) { + return type; + } + + return "System.IntPtr"; + } + + public bool Matches (MethodDefinition method) + { + if (method.Parameters.Count != paramTypes.Count || !method.IsStatic) { + log.LogDebugMessage ($"Method '{method.FullName}' doesn't match native callback signature (invalid parameter count or not static)"); + return false; + } + + if (String.Compare (returnType, method.ReturnType.FullName, StringComparison.Ordinal) != 0) { + log.LogDebugMessage ($"Method '{method.FullName}' doesn't match native callback signature (invalid return type)"); + return false; + } + + for (int i = 0; i < method.Parameters.Count; i++) { + ParameterDefinition pd = method.Parameters[i]; + if (String.Compare (pd.ParameterType.FullName, paramTypes[i], StringComparison.Ordinal) != 0) { + log.LogDebugMessage ($"Method '{method.FullName}' doesn't match native callback signature, expected parameter type '{paramTypes[i]}' at position {i}, found '{pd.ParameterType.FullName}'"); + return false; + } + } + + return true; + } + } + + TypeDefinitionCache tdCache; + DirectoryAssemblyResolver resolver; + Dictionary marshalMethods; + HashSet assemblies; + TaskLoggingHelper log; + bool haveDynamicMethods; + + public IDictionary MarshalMethods => marshalMethods; + public ICollection Assemblies => assemblies; + public bool FoundDynamicallyRegisteredMethods => haveDynamicMethods; +#endif + + public MarshalMethodsClassifier (TypeDefinitionCache tdCache, DirectoryAssemblyResolver res, TaskLoggingHelper log) + { +#if ENABLE_MARSHAL_METHODS + this.log = log ?? throw new ArgumentNullException (nameof (log)); + this.tdCache = tdCache ?? throw new ArgumentNullException (nameof (tdCache)); + resolver = res ?? throw new ArgumentNullException (nameof (tdCache)); + marshalMethods = new Dictionary (StringComparer.Ordinal); + assemblies = new HashSet (); +#endif + } + + public override bool ShouldBeDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute) + { +#if ENABLE_MARSHAL_METHODS + if (registeredMethod == null) { + throw new ArgumentNullException (nameof (registeredMethod)); + } + + if (implementedMethod == null) { + throw new ArgumentNullException (nameof (registeredMethod)); + } + + if (registerAttribute == null) { + throw new ArgumentNullException (nameof (registerAttribute)); + } + + if (!IsDynamicallyRegistered (registeredMethod, implementedMethod, registerAttribute)) { + return false; + } + + haveDynamicMethods = true; +#endif // def ENABLE_MARSHAL_METHODS + return true; + } + +#if ENABLE_MARSHAL_METHODS + bool IsDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute) + { + Console.WriteLine ($"Classifying:\n\tmethod: {implementedMethod.FullName}\n\tregistered method: {registeredMethod.FullName})\n\tAttr: {registerAttribute.AttributeType.FullName} (parameter count: {registerAttribute.ConstructorArguments.Count})"); + Console.WriteLine ($"\tManaged type: {registeredMethod.DeclaringType.FullName}, {registeredMethod.DeclaringType.GetPartialAssemblyName (tdCache)}"); + if (registerAttribute.ConstructorArguments.Count != 3) { + log.LogWarning ($"Method '{registeredMethod.FullName}' will be registered dynamically, not enough arguments to the [Register] attribute to generate marshal method."); + return true; + } + + var connector = new ConnectorInfo ((string)registerAttribute.ConstructorArguments[2].Value); + + Console.WriteLine ($"\tconnector: {connector.MethodName} (from spec: '{(string)registerAttribute.ConstructorArguments[2].Value}')"); + + if (IsStandardHandler (connector, registeredMethod)) { + return false; + } + + log.LogWarning ($"Method '{registeredMethod.FullName}' will be registered dynamically"); + return true; + } + + // TODO: Probably should check if all the methods and fields are private and static - only then it is safe(ish) to remove them + bool IsStandardHandler (ConnectorInfo connector, MethodDefinition registeredMethod) + { + const string HandlerNameStart = "Get"; + const string HandlerNameEnd = "Handler"; + + string connectorName = connector.MethodName; + if (connectorName.Length < HandlerNameStart.Length + HandlerNameEnd.Length + 1 || + !connectorName.StartsWith (HandlerNameStart, StringComparison.Ordinal) || + !connectorName.EndsWith (HandlerNameEnd, StringComparison.Ordinal)) { + log.LogWarning ($"\tConnector name '{connectorName}' must start with '{HandlerNameStart}', end with '{HandlerNameEnd}' and have at least one character between the two parts."); + return false; + } + + string callbackNameCore = connectorName.Substring (HandlerNameStart.Length, connectorName.Length - HandlerNameStart.Length - HandlerNameEnd.Length); + string nativeCallbackName = $"n_{callbackNameCore}"; + string delegateFieldName = $"cb_{Char.ToLowerInvariant (callbackNameCore[0])}{callbackNameCore.Substring (1)}"; + + TypeDefinition connectorDeclaringType = connector.AssemblyName == null ? registeredMethod.DeclaringType : FindType (resolver.Resolve (connector.AssemblyName), connector.TypeName); + Console.WriteLine ($"\tconnector name: {connectorName}\n\tnative callback name: {nativeCallbackName}\n\tdelegate field name: {delegateFieldName}"); + + MethodDefinition connectorMethod = FindMethod (connectorDeclaringType, connectorName); + if (connectorMethod == null) { + log.LogWarning ($"\tConnector method '{connectorName}' not found in type '{connectorDeclaringType.FullName}'"); + return false; + } + + if (String.Compare ("System.Delegate", connectorMethod.ReturnType.FullName, StringComparison.Ordinal) != 0) { + log.LogWarning ($"\tConnector '{connectorName}' in type '{connectorDeclaringType.FullName}' has invalid return type, expected 'System.Delegate', found '{connectorMethod.ReturnType.FullName}'"); + return false; + } + + var ncbs = new NativeCallbackSignature (registeredMethod, log); + MethodDefinition nativeCallbackMethod = FindMethod (connectorDeclaringType, nativeCallbackName, ncbs); + if (nativeCallbackMethod == null) { + log.LogWarning ($"\tUnable to find native callback method matching the '{registeredMethod.FullName}' signature"); + return false; + } + + // In the standard handler "pattern", the native callback backing field is private, static and thus in the same type + // as the native callback. + FieldDefinition delegateField = FindField (nativeCallbackMethod.DeclaringType, delegateFieldName); + if (delegateField != null) { + if (String.Compare ("System.Delegate", delegateField.FieldType.FullName, StringComparison.Ordinal) != 0) { + log.LogWarning ($"\tdelegate field '{delegateFieldName}' in type '{nativeCallbackMethod.DeclaringType.FullName}' has invalid type, expected 'System.Delegate', found '{delegateField.FieldType.FullName}'"); + return false; + } + } + + StoreMethod (connectorName, registeredMethod, new MarshalMethodEntry (nativeCallbackMethod, connectorMethod, registeredMethod, delegateField)); + StoreAssembly (connectorMethod.Module.Assembly); + StoreAssembly (nativeCallbackMethod.Module.Assembly); + if (delegateField != null) { + StoreAssembly (delegateField.Module.Assembly); + } + + return true; + } + + TypeDefinition FindType (AssemblyDefinition asm, string typeName) + { + foreach (ModuleDefinition md in asm.Modules) { + foreach (TypeDefinition td in md.Types) { + TypeDefinition match = GetMatchingType (td); + if (match != null) { + return match; + } + } + } + + return null; + + TypeDefinition GetMatchingType (TypeDefinition def) + { + if (String.Compare (def.FullName, typeName, StringComparison.Ordinal) == 0) { + return def; + } + + if (!def.HasNestedTypes) { + return null; + } + + TypeDefinition ret; + foreach (TypeDefinition nested in def.NestedTypes) { + ret = GetMatchingType (nested); + if (ret != null) { + return ret; + } + } + + return null; + } + } + + MethodDefinition FindMethod (TypeDefinition type, string methodName, IMethodSignatureMatcher signatureMatcher = null) + { + foreach (MethodDefinition method in type.Methods) { + if (!method.IsManaged || method.IsConstructor) { + continue; + } + + if (String.Compare (methodName, method.Name, StringComparison.Ordinal) != 0) { + continue; + } + + if (signatureMatcher == null || signatureMatcher.Matches (method)) { + return method; + } + } + + if (type.BaseType == null) { + return null; + } + + return FindMethod (tdCache.Resolve (type.BaseType), methodName, signatureMatcher); + } + + FieldDefinition FindField (TypeDefinition type, string fieldName, bool lookForInherited = false) + { + foreach (FieldDefinition field in type.Fields) { + if (String.Compare (field.Name, fieldName, StringComparison.Ordinal) == 0) { + return field; + } + } + + if (!lookForInherited || type.BaseType == null) { + return null; + } + + return FindField (tdCache.Resolve (type.BaseType), fieldName, lookForInherited); + } + + void StoreMethod (string connectorName, MethodDefinition registeredMethod, MarshalMethodEntry entry) + { + string typeName = registeredMethod.DeclaringType.FullName.Replace ('/', '+'); + string key = $"{typeName}, {registeredMethod.DeclaringType.GetPartialAssemblyName (tdCache)}\t{connectorName}"; + + // Several classes can override the same method, we need to generate the marshal method only once + if (marshalMethods.ContainsKey (key)) { + return; + } + + marshalMethods.Add (key, entry); + } + + void StoreAssembly (AssemblyDefinition asm) + { + if (assemblies.Contains (asm)) { + return; + } + + assemblies.Add (asm); + } +#endif + } +} diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs index cb2a8d56df5..ca67ab15c75 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection.Metadata; using System.Text; using Java.Interop.Tools.TypeNameMappings; @@ -17,6 +18,12 @@ namespace Xamarin.Android.Tasks { class MarshalMethodsNativeAssemblyGenerator : LlvmIrComposer { + sealed class MarshalMethodInfo + { + public MarshalMethodEntry Method { get; } + public string NativeSymbolName { get; } + } + // This is here only to generate strongly-typed IR internal sealed class MonoClass {} @@ -29,15 +36,17 @@ struct MarshalMethodsManagedClass MonoClass klass; }; - public ICollection UniqueAssemblyNames { get; set; } - public int NumberOfAssembliesInApk { get; set; } - public List OverriddenMethodDescriptors { get; set; } + public ICollection UniqueAssemblyNames { get; set; } + public int NumberOfAssembliesInApk { get; set; } + public IDictionary MarshalMethods { get; set; } StructureInfo monoImage; StructureInfo monoClass; public override void Init () - {} + { + Console.WriteLine ($"Marshal methods count: {MarshalMethods?.Count ?? 0}"); + } protected override void MapStructures (LlvmIrGenerator generator) { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs new file mode 100644 index 00000000000..212ec02c8e0 --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs @@ -0,0 +1,17 @@ +#if ENABLE_MARSHAL_METHODS +using System; +using System.Collections.Generic; + +namespace Xamarin.Android.Tasks +{ + sealed class MarshalMethodsState + { + public IDictionary MarshalMethods { get; } + + public MarshalMethodsState (IDictionary marshalMethods) + { + MarshalMethods = marshalMethods ?? throw new ArgumentNullException (nameof (marshalMethods)); + } + } +} +#endif diff --git a/src/monodroid/CMakeLists.txt b/src/monodroid/CMakeLists.txt index 7fca9eaf631..41577f4f55a 100644 --- a/src/monodroid/CMakeLists.txt +++ b/src/monodroid/CMakeLists.txt @@ -744,9 +744,9 @@ target_link_libraries( ${LINK_LIBS} xamarin-app ) -if((NOT DEBUG_BUILD) AND ANDROID AND ENABLE_NET AND ENABLE_MARSHAL_METHODS) - target_link_libraries( - ${XAMARIN_MONO_ANDROID_LIB} - ${XAMARIN_APP_MARSHALING_LIB} - ) -endif() +# if((NOT DEBUG_BUILD) AND ANDROID AND ENABLE_NET AND ENABLE_MARSHAL_METHODS) +# target_link_libraries( +# ${XAMARIN_MONO_ANDROID_LIB} +# ${XAMARIN_APP_MARSHALING_LIB} +# ) +# endif() diff --git a/src/monodroid/jni/embedded-assemblies.cc b/src/monodroid/jni/embedded-assemblies.cc index 31182173f89..0c4e0bf6986 100644 --- a/src/monodroid/jni/embedded-assemblies.cc +++ b/src/monodroid/jni/embedded-assemblies.cc @@ -357,6 +357,10 @@ EmbeddedAssemblies::assembly_store_open_from_bundles (dynamic_local_string Date: Mon, 4 Jul 2022 21:22:27 +0200 Subject: [PATCH 02/10] Update --- Java.Interop.diff | 6 +- .../Utilities/MarshalMethodsClassifier.cs | 70 ++++++++++++++----- 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/Java.Interop.diff b/Java.Interop.diff index 761d72c255c..82b27a26580 100644 --- a/Java.Interop.diff +++ b/Java.Interop.diff @@ -33,7 +33,7 @@ index ec1ea955..7f8e1a1b 100644 get { return connector; } set { connector = value; } diff --git a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs -index d30bce4a..afde1e48 100644 +index d30bce4a..b07d6046 100644 --- a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs +++ b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs @@ -25,40 +25,9 @@ namespace Java.Interop.Tools.JavaCallableWrappers { @@ -75,7 +75,7 @@ index d30bce4a..afde1e48 100644 - ManagedTypeName = fallbackManagedTypeName; - } - } -+ public abstract bool ShouldBeDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute); ++ public abstract bool ShouldBeDynamicallyRegistered (TypeDefinition topType, MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute); } public class JavaCallableWrapperGenerator { @@ -176,7 +176,7 @@ index d30bce4a..afde1e48 100644 Diagnostic.Error (4217, LookupSource (implementedMethod), Localization.Resources.JavaCallableWrappers_XA4217, attr.Name); - var msig = new Signature (implementedMethod, attr); -+ bool shouldBeDynamicallyRegistered = methodClassifier?.ShouldBeDynamicallyRegistered (registeredMethod, implementedMethod, attr.OriginAttribute) ?? true; ++ bool shouldBeDynamicallyRegistered = methodClassifier?.ShouldBeDynamicallyRegistered (this.type, registeredMethod, implementedMethod, attr.OriginAttribute) ?? true; + var msig = new Signature (implementedMethod, attr, shouldBeDynamicallyRegistered); if (!registeredMethod.IsConstructor && !methods.Any (m => m.Name == msig.Name && m.Params == msig.Params)) methods.Add (msig); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs index eea4007bfba..4bd29dc6231 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs @@ -3,6 +3,7 @@ using Java.Interop.Tools.Cecil; using Java.Interop.Tools.JavaCallableWrappers; +using Java.Interop.Tools.TypeNameMappings; using Microsoft.Android.Build.Tasks; using Microsoft.Build.Utilities; using Mono.Cecil; @@ -12,17 +13,29 @@ namespace Xamarin.Android.Tasks #if ENABLE_MARSHAL_METHODS public sealed class MarshalMethodEntry { - public MethodDefinition NativeCallback { get; } - public MethodDefinition Connector { get; } - public MethodDefinition TargetMethod { get; } - public FieldDefinition CallbackField { get; } - - public MarshalMethodEntry (MethodDefinition nativeCallback, MethodDefinition connector, MethodDefinition targetMethod, FieldDefinition callbackField) + public TypeDefinition TopType { get; } + public MethodDefinition NativeCallback { get; } + public MethodDefinition Connector { get; } + public MethodDefinition RegisteredMethod { get; } + public MethodDefinition ImplementedMethod { get; } + public FieldDefinition CallbackField { get; } + public string JniTypeName { get; } + public string JniMethodName { get; } + public string JniMethodSignature { get; } + + public MarshalMethodEntry (TypeDefinition topType, MethodDefinition nativeCallback, MethodDefinition connector, MethodDefinition + registeredMethod, MethodDefinition implementedMethod, FieldDefinition callbackField, string jniTypeName, + string jniName, string jniSignature) { + TopType = topType ?? throw new ArgumentNullException (nameof (topType)); NativeCallback = nativeCallback ?? throw new ArgumentNullException (nameof (nativeCallback)); Connector = connector ?? throw new ArgumentNullException (nameof (connector)); - TargetMethod = targetMethod ?? throw new ArgumentNullException (nameof (targetMethod)); + RegisteredMethod = registeredMethod ?? throw new ArgumentNullException (nameof (registeredMethod)); + ImplementedMethod = implementedMethod ?? throw new ArgumentNullException (nameof (implementedMethod)); CallbackField = callbackField; + JniTypeName = jniTypeName; + JniMethodName = jniName; + JniMethodSignature = jniSignature; } } #endif @@ -129,12 +142,12 @@ public bool Matches (MethodDefinition method) TypeDefinitionCache tdCache; DirectoryAssemblyResolver resolver; - Dictionary marshalMethods; + Dictionary> marshalMethods; HashSet assemblies; TaskLoggingHelper log; bool haveDynamicMethods; - public IDictionary MarshalMethods => marshalMethods; + public IDictionary> MarshalMethods => marshalMethods; public ICollection Assemblies => assemblies; public bool FoundDynamicallyRegisteredMethods => haveDynamicMethods; #endif @@ -145,12 +158,12 @@ public MarshalMethodsClassifier (TypeDefinitionCache tdCache, DirectoryAssemblyR this.log = log ?? throw new ArgumentNullException (nameof (log)); this.tdCache = tdCache ?? throw new ArgumentNullException (nameof (tdCache)); resolver = res ?? throw new ArgumentNullException (nameof (tdCache)); - marshalMethods = new Dictionary (StringComparer.Ordinal); + marshalMethods = new Dictionary> (StringComparer.Ordinal); assemblies = new HashSet (); #endif } - public override bool ShouldBeDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute) + public override bool ShouldBeDynamicallyRegistered (TypeDefinition topType, MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute) { #if ENABLE_MARSHAL_METHODS if (registeredMethod == null) { @@ -165,7 +178,7 @@ public override bool ShouldBeDynamicallyRegistered (MethodDefinition registeredM throw new ArgumentNullException (nameof (registerAttribute)); } - if (!IsDynamicallyRegistered (registeredMethod, implementedMethod, registerAttribute)) { + if (!IsDynamicallyRegistered (topType, registeredMethod, implementedMethod, registerAttribute)) { return false; } @@ -175,10 +188,10 @@ public override bool ShouldBeDynamicallyRegistered (MethodDefinition registeredM } #if ENABLE_MARSHAL_METHODS - bool IsDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute) + bool IsDynamicallyRegistered (TypeDefinition topType, MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute) { Console.WriteLine ($"Classifying:\n\tmethod: {implementedMethod.FullName}\n\tregistered method: {registeredMethod.FullName})\n\tAttr: {registerAttribute.AttributeType.FullName} (parameter count: {registerAttribute.ConstructorArguments.Count})"); - Console.WriteLine ($"\tManaged type: {registeredMethod.DeclaringType.FullName}, {registeredMethod.DeclaringType.GetPartialAssemblyName (tdCache)}"); + Console.WriteLine ($"\tTop type: {topType.FullName}\n\tManaged type: {registeredMethod.DeclaringType.FullName}, {registeredMethod.DeclaringType.GetPartialAssemblyName (tdCache)}"); if (registerAttribute.ConstructorArguments.Count != 3) { log.LogWarning ($"Method '{registeredMethod.FullName}' will be registered dynamically, not enough arguments to the [Register] attribute to generate marshal method."); return true; @@ -188,7 +201,7 @@ bool IsDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinitio Console.WriteLine ($"\tconnector: {connector.MethodName} (from spec: '{(string)registerAttribute.ConstructorArguments[2].Value}')"); - if (IsStandardHandler (connector, registeredMethod)) { + if (IsStandardHandler (topType, connector, registeredMethod, implementedMethod, jniName: (string)registerAttribute.ConstructorArguments[0].Value, jniSignature: (string)registerAttribute.ConstructorArguments[1].Value)) { return false; } @@ -197,7 +210,7 @@ bool IsDynamicallyRegistered (MethodDefinition registeredMethod, MethodDefinitio } // TODO: Probably should check if all the methods and fields are private and static - only then it is safe(ish) to remove them - bool IsStandardHandler (ConnectorInfo connector, MethodDefinition registeredMethod) + bool IsStandardHandler (TypeDefinition topType, ConnectorInfo connector, MethodDefinition registeredMethod, MethodDefinition implementedMethod, string jniName, string jniSignature) { const string HandlerNameStart = "Get"; const string HandlerNameEnd = "Handler"; @@ -245,7 +258,24 @@ bool IsStandardHandler (ConnectorInfo connector, MethodDefinition registeredMeth } } - StoreMethod (connectorName, registeredMethod, new MarshalMethodEntry (nativeCallbackMethod, connectorMethod, registeredMethod, delegateField)); + Console.WriteLine ($"##G1: {implementedMethod.DeclaringType.FullName} -> {JavaNativeTypeManager.ToJniName (implementedMethod.DeclaringType, tdCache)}"); + Console.WriteLine ($"##G1: top type: {topType.FullName} -> {JavaNativeTypeManager.ToJniName (topType, tdCache)}"); + + StoreMethod ( + connectorName, + registeredMethod, + new MarshalMethodEntry ( + topType, + nativeCallbackMethod, + connectorMethod, + registeredMethod, + implementedMethod, + delegateField, + JavaNativeTypeManager.ToJniName (topType, tdCache), + jniName, + jniSignature) + ); + StoreAssembly (connectorMethod.Module.Assembly); StoreAssembly (nativeCallbackMethod.Module.Assembly); if (delegateField != null) { @@ -338,7 +368,11 @@ void StoreMethod (string connectorName, MethodDefinition registeredMethod, Marsh return; } - marshalMethods.Add (key, entry); + if (!marshalMethods.TryGetValue (key, out IList list) || list == null) { + list = new List (); + marshalMethods.Add (key, list); + } + list.Add (entry); } void StoreAssembly (AssemblyDefinition asm) From 20cff4266ea614bd3de5b17c22fa63e978acad14 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 11 Jul 2022 10:02:37 +0200 Subject: [PATCH 03/10] Bump JI --- external/Java.Interop | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/Java.Interop b/external/Java.Interop index c942ab683c1..fadbb82c3b8 160000 --- a/external/Java.Interop +++ b/external/Java.Interop @@ -1 +1 @@ -Subproject commit c942ab683c12e88e0527ed584a13b411e005ea57 +Subproject commit fadbb82c3b8ab7979c19e9f139bdf2589e47549e From 9b2711ce4d05606a5b80b9872c3845e4aeea2da2 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 11 Jul 2022 10:14:20 +0200 Subject: [PATCH 04/10] Don't need the diff anymore --- Java.Interop.diff | 283 ---------------------------------------------- 1 file changed, 283 deletions(-) delete mode 100644 Java.Interop.diff diff --git a/Java.Interop.diff b/Java.Interop.diff deleted file mode 100644 index 82b27a26580..00000000000 --- a/Java.Interop.diff +++ /dev/null @@ -1,283 +0,0 @@ -diff --git a/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs b/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs -index ec1ea955..7f8e1a1b 100644 ---- a/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs -+++ b/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs -@@ -1,5 +1,7 @@ - using System; - -+using Mono.Cecil; -+ - namespace Android.Runtime { - - [AttributeUsage (AttributeTargets.Class | AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Interface | AttributeTargets.Method | AttributeTargets.Property)] -@@ -23,7 +25,21 @@ namespace Android.Runtime { - this.connector = connector; - this.signature = signature; - } -+#if HAVE_CECIL -+ public RegisterAttribute (string name, CustomAttribute originAttribute) -+ : this (name) -+ { -+ OriginAttribute = originAttribute; -+ } -+ -+ public RegisterAttribute (string name, string signature, string connector, CustomAttribute originAttribute) -+ : this (name, signature, connector) -+ { -+ OriginAttribute = originAttribute; -+ } - -+ public CustomAttribute OriginAttribute { get; } -+#endif - public string Connector { - get { return connector; } - set { connector = value; } -diff --git a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs -index d30bce4a..b07d6046 100644 ---- a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs -+++ b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs -@@ -25,40 +25,9 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - JavaInterop1, - } - -- public class OverriddenMethodDescriptor -+ public abstract class JavaCallableMethodClassifier - { -- static readonly char[] methodDescSplitChars = new char[] { ':' }; -- -- public string JavaPackageName { get; } -- public string NativeName { get; } -- public string JniSignature { get; } -- public string Connector { get; } -- public string ManagedTypeName { get; } -- public string OriginalDescString { get; } -- -- public OverriddenMethodDescriptor (string javaPackageName, string methodDescription, string fallbackManagedTypeName) -- { -- OriginalDescString = methodDescription; -- JavaPackageName = javaPackageName; -- string[] parts = methodDescription.Split (methodDescSplitChars, 4); -- -- if (parts.Length < 2) { -- throw new InvalidOperationException ($"Unexpected format for method description. Expected at least 2 parts, got {parts.Length} from: '{methodDescription}'"); -- } -- -- NativeName = parts[0]; -- JniSignature = parts[1]; -- if (parts.Length > 2) { -- Connector = parts[2]; -- if (parts.Length > 3) { -- ManagedTypeName = TypeDefinitionRocks.CecilTypeNameToReflectionTypeName (parts[3]); -- } -- } -- -- if (String.IsNullOrEmpty (ManagedTypeName)) { -- ManagedTypeName = fallbackManagedTypeName; -- } -- } -+ public abstract bool ShouldBeDynamicallyRegistered (TypeDefinition topType, MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute); - } - - public class JavaCallableWrapperGenerator { -@@ -95,21 +64,22 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - List methods = new List (); - List ctors = new List (); - List children; -- List overriddenMethodDescriptors; -+ - readonly IMetadataResolver cache; -+ readonly JavaCallableMethodClassifier methodClassifier; - - [Obsolete ("Use the TypeDefinitionCache overload for better performance.")] - public JavaCallableWrapperGenerator (TypeDefinition type, Action log) - : this (type, null, log, resolver: null) - { } - -- public JavaCallableWrapperGenerator (TypeDefinition type, Action log, TypeDefinitionCache cache) -- : this (type, log, (IMetadataResolver) cache) -+ public JavaCallableWrapperGenerator (TypeDefinition type, Action log, TypeDefinitionCache cache, JavaCallableMethodClassifier methodClassifier = null) -+ : this (type, log, (IMetadataResolver) cache, methodClassifier) - { - } - -- public JavaCallableWrapperGenerator (TypeDefinition type, Action log, IMetadataResolver resolver) -- : this (type, null, log, resolver) -+ public JavaCallableWrapperGenerator (TypeDefinition type, Action log, IMetadataResolver resolver, JavaCallableMethodClassifier methodClassifier = null) -+ : this (type, null, log, resolver, methodClassifier) - { - if (type.HasNestedTypes) { - children = new List (); -@@ -117,7 +87,6 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - } - } - -- public IList OverriddenMethodDescriptors => overriddenMethodDescriptors; - public string ApplicationJavaClass { get; set; } - public JavaPeerStyle CodeGenerationTarget { get; set; } - -@@ -125,6 +94,9 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - - public bool HasExport { get; private set; } - -+ // If there are no methods, we need to generate "empty" registration because of backward compatibility -+ public bool HasDynamicallyRegisteredMethods => methods.Count == 0 || methods.Any ((Signature sig) => sig.IsDynamicallyRegistered); -+ - /// - /// The Java source code to be included in Instrumentation.onCreate - /// -@@ -152,8 +124,9 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - HasExport |= children.Any (t => t.HasExport); - } - -- JavaCallableWrapperGenerator (TypeDefinition type, string outerType, Action log, IMetadataResolver resolver) -+ JavaCallableWrapperGenerator (TypeDefinition type, string outerType, Action log, IMetadataResolver resolver, JavaCallableMethodClassifier methodClassifier = null) - { -+ this.methodClassifier = methodClassifier; - this.type = type; - this.log = log; - this.cache = resolver ?? new TypeDefinitionCache (); -@@ -366,12 +339,13 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - // attr.Resolve (); - RegisterAttribute r = null; - if (attr.ConstructorArguments.Count == 1) -- r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value); -+ r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value, attr); - else if (attr.ConstructorArguments.Count == 3) - r = new RegisterAttribute ( - (string) attr.ConstructorArguments [0].Value, - (string) attr.ConstructorArguments [1].Value, -- (string) attr.ConstructorArguments [2].Value); -+ (string) attr.ConstructorArguments [2].Value, -+ attr); - if (r != null) { - var v = attr.Properties.FirstOrDefault (p => p.Name == "DoNotGenerateAcw"); - r.DoNotGenerateAcw = v.Name == null ? false : (bool) v.Argument.Value; -@@ -384,7 +358,7 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - // attr.Resolve (); - RegisterAttribute r = null; - if (attr.ConstructorArguments.Count == 1) -- r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value); -+ r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value, attr); - if (r != null) { - var v = attr.Properties.FirstOrDefault (p => p.Name == "GenerateJavaPeer"); - if (v.Name == null) { -@@ -403,7 +377,8 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - if (attr.ConstructorArguments.Count == 2) - r = new RegisterAttribute ((string) attr.ConstructorArguments [0].Value, - (string) attr.ConstructorArguments [1].Value, -- ""); -+ "", -+ attr); - return r; - } - -@@ -467,7 +442,8 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - if (attr.Name.Contains ("-impl") || (attr.Name.Length > 7 && attr.Name[attr.Name.Length - 8] == '-')) - Diagnostic.Error (4217, LookupSource (implementedMethod), Localization.Resources.JavaCallableWrappers_XA4217, attr.Name); - -- var msig = new Signature (implementedMethod, attr); -+ bool shouldBeDynamicallyRegistered = methodClassifier?.ShouldBeDynamicallyRegistered (this.type, registeredMethod, implementedMethod, attr.OriginAttribute) ?? true; -+ var msig = new Signature (implementedMethod, attr, shouldBeDynamicallyRegistered); - if (!registeredMethod.IsConstructor && !methods.Any (m => m.Name == msig.Name && m.Params == msig.Params)) - methods.Add (msig); - } -@@ -542,21 +518,38 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - - GenerateHeader (writer); - -- writer.WriteLine ("/** @hide */"); -- writer.WriteLine ("\tpublic static final String __md_methods;"); -- if (children != null) { -- foreach (var i in Enumerable.Range (1, children.Count)) -- writer.WriteLine ("\tstatic final String __md_{0}_methods;", i); -+ bool needCtor = false; -+ if (HasDynamicallyRegisteredMethods) { -+ needCtor = true; -+ writer.WriteLine ("/** @hide */"); -+ writer.WriteLine ("\tpublic static final String __md_methods;"); - } -- writer.WriteLine ("\tstatic {"); -- GenerateRegisterType (writer, this, "__md_methods"); -+ - if (children != null) { -- for (int i = 0; i < children.Count; ++i) { -- string methods = string.Format ("__md_{0}_methods", i + 1); -- GenerateRegisterType (writer, children [i], methods); -+ for (int i = 0; i < children.Count; i++) { -+ if (!children[i].HasDynamicallyRegisteredMethods) { -+ continue; -+ } -+ needCtor = true; -+ writer.WriteLine ("\tstatic final String __md_{0}_methods;", i + 1); -+ } -+ } -+ -+ if (needCtor) { -+ writer.WriteLine ("\tstatic {"); -+ -+ if (HasDynamicallyRegisteredMethods) { -+ GenerateRegisterType (writer, this, "__md_methods"); -+ } -+ -+ if (children != null) { -+ for (int i = 0; i < children.Count; ++i) { -+ string methods = string.Format ("__md_{0}_methods", i + 1); -+ GenerateRegisterType (writer, children [i], methods); -+ } - } -+ writer.WriteLine ("\t}"); - } -- writer.WriteLine ("\t}"); - - GenerateBody (writer); - -@@ -710,16 +703,19 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - - void GenerateRegisterType (TextWriter sw, JavaCallableWrapperGenerator self, string field) - { -- if (overriddenMethodDescriptors == null) { -- overriddenMethodDescriptors = new List (); -+ string managedTypeName = self.type.GetPartialAssemblyQualifiedName (cache); -+ string javaTypeName = $"{package}.{name}"; -+ -+ if (!self.HasDynamicallyRegisteredMethods) { -+ return; - } - - sw.WriteLine ("\t\t{0} = ", field); -- string managedTypeName = self.type.GetPartialAssemblyQualifiedName (cache); -- string javaTypeName = $"{package}.{name}"; -+ - foreach (Signature method in self.methods) { -- sw.WriteLine ("\t\t\t\"{0}\\n\" +", method.Method); -- overriddenMethodDescriptors.Add (new OverriddenMethodDescriptor (javaTypeName, method.Method, managedTypeName)); -+ if (method.IsDynamicallyRegistered) { -+ sw.WriteLine ("\t\t\t\"{0}\\n\" +", method.Method); -+ } - } - sw.WriteLine ("\t\t\t\"\";"); - if (CannotRegisterInStaticConstructor (self.type)) -@@ -772,12 +768,13 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - - class Signature { - -- public Signature (MethodDefinition method, RegisterAttribute register) : this (method, register, null, null) {} -+ public Signature (MethodDefinition method, RegisterAttribute register, bool shouldBeDynamicallyRegistered = true) : this (method, register, null, null, shouldBeDynamicallyRegistered) {} - -- public Signature (MethodDefinition method, RegisterAttribute register, string managedParameters, string outerType) -+ public Signature (MethodDefinition method, RegisterAttribute register, string managedParameters, string outerType, bool shouldBeDynamicallyRegistered = true) - : this (register.Name, register.Signature, register.Connector, managedParameters, outerType, null) - { - Annotations = JavaCallableWrapperGenerator.GetAnnotationsString ("\t", method.CustomAttributes); -+ IsDynamicallyRegistered = shouldBeDynamicallyRegistered; - } - - public Signature (MethodDefinition method, ExportAttribute export, IMetadataResolver cache) -@@ -880,6 +877,7 @@ namespace Java.Interop.Tools.JavaCallableWrappers { - public readonly string Method; - public readonly bool IsExport; - public readonly bool IsStatic; -+ public readonly bool IsDynamicallyRegistered = true; - public readonly string [] ThrownTypeNames; - public readonly string Annotations; - } From 146b745e2cdd68c5729039cb5c481746e7ee8a98 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 13 Jul 2022 19:17:42 +0200 Subject: [PATCH 05/10] No need to check if the item exists before adding it --- src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index 160f25acb78..e96bd6d3621 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -372,9 +372,7 @@ void StoreMarshalAssemblyPath (string name, ITaskItem asm) marshalMethodsAssemblyPaths.Add (name, assemblyPaths); } - if (!assemblyPaths.Contains (asm.ItemSpec)) { - assemblyPaths.Add (asm.ItemSpec); - } + assemblyPaths.Add (asm.ItemSpec); } #endif } From 776cb7d227050eb89c30fab1650d4e3cd9a67c10 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 14 Jul 2022 09:56:19 +0200 Subject: [PATCH 06/10] Update code to implemented some requested changes --- .../Tasks/GenerateJavaStubs.cs | 4 +- .../Tasks/GeneratePackageManagerJava.cs | 2 +- .../MarshalMethodsAssemblyRewriter.cs | 41 ++++++++++++------- .../Utilities/MarshalMethodsClassifier.cs | 25 +++++++---- .../MarshalMethodsNativeAssemblyGenerator.cs | 2 +- .../Utilities/MarshalMethodsState.cs | 4 +- 6 files changed, 49 insertions(+), 29 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index e96bd6d3621..75276ef370c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -183,9 +183,7 @@ void Run (DirectoryAssemblyResolver res) if (!userAssemblies.ContainsKey (name)) userAssemblies.Add (name, asm.ItemSpec); #if ENABLE_MARSHAL_METHODS - if (!Debug) { - StoreMarshalAssemblyPath (name, asm); - } + StoreMarshalAssemblyPath (name, asm); #endif } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs index b6f6f92b05c..16105940dd2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs @@ -444,7 +444,7 @@ void AddEnvironment () #if ENABLE_MARSHAL_METHODS var marshalMethodsState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (GenerateJavaStubs.MarshalMethodsRegisterTaskKey, RegisteredTaskObjectLifetime.Build); - var marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator () { + var marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator { NumberOfAssembliesInApk = assemblyCount, UniqueAssemblyNames = uniqueAssemblyNames, MarshalMethods = marshalMethodsState?.MarshalMethods, diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs index 524411ed9ce..ee7b33ad732 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs @@ -10,11 +10,11 @@ namespace Xamarin.Android.Tasks { class MarshalMethodsAssemblyRewriter { - IDictionary methods; + IDictionary> methods; ICollection uniqueAssemblies; IDictionary > assemblyPaths; - public MarshalMethodsAssemblyRewriter (IDictionary methods, ICollection uniqueAssemblies, IDictionary > assemblyPaths) + public MarshalMethodsAssemblyRewriter (IDictionary> methods, ICollection uniqueAssemblies, IDictionary > assemblyPaths) { this.methods = methods ?? throw new ArgumentNullException (nameof (methods)); this.uniqueAssemblies = uniqueAssemblies ?? throw new ArgumentNullException (nameof (uniqueAssemblies)); @@ -23,17 +23,20 @@ public MarshalMethodsAssemblyRewriter (IDictionary m public void Rewrite (DirectoryAssemblyResolver resolver) { + MethodDefinition unmanagedCallersOnlyAttributeCtor = GetUnmanagedCallersOnlyAttributeConstructor (resolver); var unmanagedCallersOnlyAttributes = new Dictionary (); foreach (AssemblyDefinition asm in uniqueAssemblies) { - unmanagedCallersOnlyAttributes.Add (asm, GetUnmanagedCallersOnlyAttribute (asm, resolver)); + unmanagedCallersOnlyAttributes.Add (asm, CreateImportedUnmanagedCallersOnlyAttribute (asm, unmanagedCallersOnlyAttributeCtor)); } Console.WriteLine ("Adding the [UnmanagedCallersOnly] attribute to native callback methods and removing unneeded fields+methods"); - foreach (MarshalMethodEntry method in methods.Values) { - Console.WriteLine ($"\t{method.NativeCallback.FullName} (token: 0x{method.NativeCallback.MetadataToken.RID:x})"); - method.NativeCallback.CustomAttributes.Add (unmanagedCallersOnlyAttributes [method.NativeCallback.Module.Assembly]); - method.Connector.DeclaringType.Methods.Remove (method.Connector); - method.CallbackField?.DeclaringType.Fields.Remove (method.CallbackField); + foreach (IList methodList in methods.Values) { + foreach (MarshalMethodEntry method in methodList) { + Console.WriteLine ($"\t{method.NativeCallback.FullName} (token: 0x{method.NativeCallback.MetadataToken.RID:x})"); + method.NativeCallback.CustomAttributes.Add (unmanagedCallersOnlyAttributes [method.NativeCallback.Module.Assembly]); + method.Connector.DeclaringType.Methods.Remove (method.Connector); + method.CallbackField?.DeclaringType.Fields.Remove (method.CallbackField); + } } Console.WriteLine (); @@ -75,8 +78,10 @@ public void Rewrite (DirectoryAssemblyResolver resolver) Console.WriteLine (); Console.WriteLine ("Method tokens:"); - foreach (MarshalMethodEntry method in methods.Values) { - Console.WriteLine ($"\t{method.NativeCallback.FullName} (token: 0x{method.NativeCallback.MetadataToken.RID:x})"); + foreach (IList methodList in methods.Values) { + foreach (MarshalMethodEntry method in methodList) { + Console.WriteLine ($"\t{method.NativeCallback.FullName} (token: 0x{method.NativeCallback.MetadataToken.RID:x})"); + } } void MoveFile (string source, string target) @@ -99,7 +104,7 @@ ICollection GetAssemblyPaths (AssemblyDefinition asm) return paths; } - CustomAttribute GetUnmanagedCallersOnlyAttribute (AssemblyDefinition targetAssembly, DirectoryAssemblyResolver resolver) + MethodDefinition GetUnmanagedCallersOnlyAttributeConstructor (DirectoryAssemblyResolver resolver) { AssemblyDefinition asm = resolver.Resolve ("System.Runtime.InteropServices"); TypeDefinition unmanagedCallersOnlyAttribute = null; @@ -118,16 +123,24 @@ CustomAttribute GetUnmanagedCallersOnlyAttribute (AssemblyDefinition targetAssem } } - MethodDefinition attrConstructor = null; + if (unmanagedCallersOnlyAttribute == null) { + throw new InvalidOperationException ("Unable to find the System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute type"); + } + foreach (MethodDefinition md in unmanagedCallersOnlyAttribute.Methods) { if (!md.IsConstructor) { continue; } - attrConstructor = md; + return md; } - return new CustomAttribute (targetAssembly.MainModule.ImportReference (attrConstructor)); + throw new InvalidOperationException ("Unable to find the System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute type constructor"); + } + + CustomAttribute CreateImportedUnmanagedCallersOnlyAttribute (AssemblyDefinition targetAssembly, MethodDefinition unmanagedCallersOnlyAtributeCtor) + { + return new CustomAttribute (targetAssembly.MainModule.ImportReference (unmanagedCallersOnlyAtributeCtor)); } } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs index 4bd29dc6231..5fb4ce7475e 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs @@ -13,7 +13,7 @@ namespace Xamarin.Android.Tasks #if ENABLE_MARSHAL_METHODS public sealed class MarshalMethodEntry { - public TypeDefinition TopType { get; } + public TypeDefinition DeclaringType { get; } public MethodDefinition NativeCallback { get; } public MethodDefinition Connector { get; } public MethodDefinition RegisteredMethod { get; } @@ -24,18 +24,27 @@ public sealed class MarshalMethodEntry public string JniMethodSignature { get; } public MarshalMethodEntry (TypeDefinition topType, MethodDefinition nativeCallback, MethodDefinition connector, MethodDefinition - registeredMethod, MethodDefinition implementedMethod, FieldDefinition callbackField, string jniTypeName, - string jniName, string jniSignature) + registeredMethod, MethodDefinition implementedMethod, FieldDefinition callbackField, string jniTypeName, + string jniName, string jniSignature) { - TopType = topType ?? throw new ArgumentNullException (nameof (topType)); + DeclaringType = topType ?? throw new ArgumentNullException (nameof (topType)); NativeCallback = nativeCallback ?? throw new ArgumentNullException (nameof (nativeCallback)); Connector = connector ?? throw new ArgumentNullException (nameof (connector)); RegisteredMethod = registeredMethod ?? throw new ArgumentNullException (nameof (registeredMethod)); ImplementedMethod = implementedMethod ?? throw new ArgumentNullException (nameof (implementedMethod)); - CallbackField = callbackField; - JniTypeName = jniTypeName; - JniMethodName = jniName; - JniMethodSignature = jniSignature; + CallbackField = callbackField; // we don't require the callback field to exist + JniTypeName = EnsureNonEmpty (jniTypeName, nameof (jniTypeName)); + JniMethodName = EnsureNonEmpty (jniName, nameof (jniName)); + JniMethodSignature = EnsureNonEmpty (jniSignature, nameof (jniSignature)); + } + + string EnsureNonEmpty (string s, string argName) + { + if (String.IsNullOrEmpty (s)) { + throw new ArgumentException ("must not be null or empty", argName); + } + + return s; } } #endif diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs index ca67ab15c75..e18ab55f20b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs @@ -38,7 +38,7 @@ struct MarshalMethodsManagedClass public ICollection UniqueAssemblyNames { get; set; } public int NumberOfAssembliesInApk { get; set; } - public IDictionary MarshalMethods { get; set; } + public IDictionary> MarshalMethods { get; set; } StructureInfo monoImage; StructureInfo monoClass; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs index 212ec02c8e0..85063dc7dbe 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs @@ -6,9 +6,9 @@ namespace Xamarin.Android.Tasks { sealed class MarshalMethodsState { - public IDictionary MarshalMethods { get; } + public IDictionary> MarshalMethods { get; } - public MarshalMethodsState (IDictionary marshalMethods) + public MarshalMethodsState (IDictionary> marshalMethods) { MarshalMethods = marshalMethods ?? throw new ArgumentNullException (nameof (marshalMethods)); } From 52586cd3df25b8a17a753445aa532c38ead4a709 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 14 Jul 2022 10:20:22 +0200 Subject: [PATCH 07/10] Rename, for consistency --- .../Utilities/MarshalMethodsClassifier.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs index 5fb4ce7475e..7285b88ea2e 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs @@ -23,11 +23,11 @@ public sealed class MarshalMethodEntry public string JniMethodName { get; } public string JniMethodSignature { get; } - public MarshalMethodEntry (TypeDefinition topType, MethodDefinition nativeCallback, MethodDefinition connector, MethodDefinition + public MarshalMethodEntry (TypeDefinition declaringType, MethodDefinition nativeCallback, MethodDefinition connector, MethodDefinition registeredMethod, MethodDefinition implementedMethod, FieldDefinition callbackField, string jniTypeName, string jniName, string jniSignature) { - DeclaringType = topType ?? throw new ArgumentNullException (nameof (topType)); + DeclaringType = declaringType ?? throw new ArgumentNullException (nameof (declaringType)); NativeCallback = nativeCallback ?? throw new ArgumentNullException (nameof (nativeCallback)); Connector = connector ?? throw new ArgumentNullException (nameof (connector)); RegisteredMethod = registeredMethod ?? throw new ArgumentNullException (nameof (registeredMethod)); From 86de7c2ba66c3b021e1b9b50dc465b6c40cf9182 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 14 Jul 2022 11:44:30 +0200 Subject: [PATCH 08/10] Remove unneeded argument --- src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index 75276ef370c..801ecdba903 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -211,7 +211,7 @@ void Run (DirectoryAssemblyResolver res) } #endif // Step 2 - Generate Java stub code - var success = CreateJavaSources (javaTypes, cache, res, classifier); + var success = CreateJavaSources (javaTypes, cache, classifier); if (!success) return; @@ -375,7 +375,7 @@ void StoreMarshalAssemblyPath (string name, ITaskItem asm) #endif } - bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCache cache, DirectoryAssemblyResolver resolver, MarshalMethodsClassifier classifier) + bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCache cache, MarshalMethodsClassifier classifier) { string outputPath = Path.Combine (OutputDirectory, "src"); string monoInit = GetMonoInitSource (AndroidSdkPlatform); From e99febb1a3acc498c10e9706fdc7c4cb7e4e9e7b Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 14 Jul 2022 12:02:47 +0200 Subject: [PATCH 09/10] Use CopyIfChanged --- .../Tasks/GenerateJavaStubs.cs | 2 +- .../Utilities/MarshalMethodsAssemblyRewriter.cs | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index 801ecdba903..52317659f12 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -217,7 +217,7 @@ void Run (DirectoryAssemblyResolver res) #if ENABLE_MARSHAL_METHODS if (!Debug) { - var rewriter = new MarshalMethodsAssemblyRewriter (classifier.MarshalMethods, classifier.Assemblies, marshalMethodsAssemblyPaths); + var rewriter = new MarshalMethodsAssemblyRewriter (classifier.MarshalMethods, classifier.Assemblies, marshalMethodsAssemblyPaths, Log); rewriter.Rewrite (res); } #endif diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs index ee7b33ad732..24e841e5252 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs @@ -4,7 +4,10 @@ using System.IO; using Java.Interop.Tools.Cecil; +using Microsoft.Android.Build.Tasks; +using Microsoft.Build.Utilities; using Mono.Cecil; +using Xamarin.Android.Tools; namespace Xamarin.Android.Tasks { @@ -13,12 +16,14 @@ class MarshalMethodsAssemblyRewriter IDictionary> methods; ICollection uniqueAssemblies; IDictionary > assemblyPaths; + TaskLoggingHelper log; - public MarshalMethodsAssemblyRewriter (IDictionary> methods, ICollection uniqueAssemblies, IDictionary > assemblyPaths) + public MarshalMethodsAssemblyRewriter (IDictionary> methods, ICollection uniqueAssemblies, IDictionary > assemblyPaths, TaskLoggingHelper log) { this.methods = methods ?? throw new ArgumentNullException (nameof (methods)); this.uniqueAssemblies = uniqueAssemblies ?? throw new ArgumentNullException (nameof (uniqueAssemblies)); this.assemblyPaths = assemblyPaths ?? throw new ArgumentNullException (nameof (assemblyPaths)); + this.log = log ?? throw new ArgumentNullException (nameof (log)); } public void Rewrite (DirectoryAssemblyResolver resolver) @@ -87,11 +92,12 @@ public void Rewrite (DirectoryAssemblyResolver resolver) void MoveFile (string source, string target) { Console.WriteLine ($"Moving '{source}' => '{target}'"); - if (File.Exists (target)) { - File.Delete (target); + Files.CopyIfChanged (source, target); + try { + File.Delete (source); + } catch (Exception ex) { + log.LogWarning ($"Unable to delete source file '{source}' when moving it to '{target}'"); } - - File.Move (source, target); } } From f3c1d348a14300b2dcc257943f2716e777e85d49 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 18 Jul 2022 11:47:39 +0200 Subject: [PATCH 10/10] Adjust API for future JI changes (in JI/main) --- .../Utilities/MarshalMethodsClassifier.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs index 7285b88ea2e..a6801fa6d13 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs @@ -172,7 +172,7 @@ public MarshalMethodsClassifier (TypeDefinitionCache tdCache, DirectoryAssemblyR #endif } - public override bool ShouldBeDynamicallyRegistered (TypeDefinition topType, MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute registerAttribute) + public override bool ShouldBeDynamicallyRegistered (TypeDefinition topType, MethodDefinition registeredMethod, MethodDefinition implementedMethod, CustomAttribute? registerAttribute) { #if ENABLE_MARSHAL_METHODS if (registeredMethod == null) {