From a017561b1e44c51a9af79fae0baaa50fe01c4123 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 8 May 2020 16:33:09 +0000 Subject: [PATCH] [typemaps] Handle Java to managed type maps properly (#4656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/xamarin/xamarin-android/issues/4596 Fixes: https://github.com/xamarin/xamarin-android/issues/4660 Context: https://github.com/xamarin/monodroid/commit/192b1f1b71c98650d04bf0a4e52ee337a054ff4c Context: https://github.com/xamarin/java.interop/blob/fc18c54b2ccf14f444fadac0a1e7e81f837cc470/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/TypeNameMapGenerator.cs#L149-L186 Context: https://github.com/xamarin/java.interop/blob/010161d1ef6625b762429334f02e7b15e1f7caae/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/TypeNameMapGenerator.cs#L155 ~~ Debug Mode ~~ In some environments, `BundleTest.TestBundleIntegerArrayList2()` fails when using the commercial shared runtime: Test 'Xamarin.Android.RuntimeTests.BundleTest.TestBundleIntegerArrayList2' failed: System.MemberAccessException : Cannot create an instance of Android.Runtime.JavaList`1[T] because Type.ContainsGenericParameters is true. at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) at System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters) at Java.Interop.TypeManager.CreateProxy (System.Type type, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType) at Java.Lang.Object.GetObject (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type type) at Java.Lang.Object._GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) at Java.Lang.Object.GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) at Android.OS.Bundle.Get (System.String key) at Xamarin.Android.RuntimeTests.BundleTest.TestBundleIntegerArrayList2 () 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) The problem appears to be rooted in ce2bc689. In a pre-ce2bc689 world, there were two separate sets of "typemap" files that the application could use: * Java-to-managed mappings, in `typemap.jm` * Managed-to-Java mappings, in `typemap.mj`. These files did *not* need to have the same number of entries! % unzip /Library/Frameworks/Xamarin.Android.framework/Versions/10.2.0.100/lib/xamarin.android/xbuild/Xamarin/Android/Mono.Android.DebugRuntime-debug.apk typemap.jm % unzip /Library/Frameworks/Xamarin.Android.framework/Versions/10.2.0.100/lib/xamarin.android/xbuild/Xamarin/Android/Mono.Android.DebugRuntime-debug.apk typemap.mj % strings typemap.jm | wc -l 10318 % strings typemap.mj | wc -l 11956 The reason why they have a different number of entries is *aliasing*: there can be *multiple* bindings for a given Java type. The managed-to-Java mappings can contain *all* of them; the Java-to-managed mapping *must* pick Only One. For example, [`java.util.ArrayList`][0] contains (at least?) *three* bindings: * [`Android.Runtime.JavaList`][1] * [`Android.Runtime.JavaList`][2] * `Java.Util.ArrayList` (generated at build time) In a pre-ce2bc689 world, this was "fine": we'd binary search each file *separately* to find type mappings. This changed in ce2bc689, as the typemap information was merged into a *single* array in order to de-duplicate information. This reduced `.apk` size. An unanticipated result of this is that the Java-to-managed mappings can now contain *duplicate* keys, "as if" the original `typemap.jm` had the contents: java/util/ArrayList Android.Runtime.JavaList, Mono.Android java/util/ArrayList Android.Runtime.JavaList`1, Mono.Android java/util/ArrayList Java.Util.ArrayLlist, Mono.Android Whereas pre-ce2bc689 there would have only been the first entry. "Sometimes" this duplication is "fine": the typemap search "Just happens" to return the first entry, or the 3rd entry, and apps continue to work as intended. "Sometimes" it isn't: the typemap binary search finds the 2nd entry, which is a generic type. This results in attempting to instantiate a generic type *without appropriate type parameters*, which results in the aforementioned `MemberAccessException`. Update typemap generation code in `Xamarin.Android.Build.Tasks.dll` so that all the duplicate Java type names will point to the same managed type name. Additionally, make sure we select the managed type in the same fashion the old typemap generator in `Java.Interop` worked: prefer base types to the derived ones if the type is an interface or an abstract class. The effect of this change is that no matter which entry `EmbeddedAssemblies::binary_search()` ends up selecting it will always return the same managed type name for all aliased Java types. ~~ Release config apps ~~ Another issue introduced in ce2bc689 is that we no longer ignore interface and generic types when generating the Java-to-managed maps. This adversely affects the `Release` mode in which it is possible that an attempt to instantiate an open generic type (or an interface) will happen, leading to error similar to: FATAL EXCEPTION: main Process: com.companyname.androidsingleviewapp1, PID: 24068 android.runtime.JavaProxyThrowable: System.MemberAccessException: Cannot create an instance of Com.Contoso.Javalibrary1.Class0`1[T] because Type.ContainsGenericParameters is true. at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) at System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters) at Java.Interop.TypeManager.CreateProxy (System.Type type, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType) at Java.Lang.Object.GetObject (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type type) at Java.Lang.Object._GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) at Java.Lang.Object.GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) at Com.Contoso.Javalibrary1.Class1.Create () at AndroidSingleViewApp1.MainActivity.OnCreate (Android.OS.Bundle savedInstanceState) at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_savedInstanceState) at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.1(intptr,intptr,intptr) at crc647d7dcab8da8ee782.MainActivity.n_onCreate(Native Method) at crc647d7dcab8da8ee782.MainActivity.onCreate(MainActivity.java:29) at android.app.Activity.performCreate(Activity.java:7144) at android.app.Activity.performCreate(Activity.java:7135) at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1271) at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2931) at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3086) at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:78) at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:108) at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:68) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1816) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loop(Looper.java:193) at android.app.ActivityThread.main(ActivityThread.java:6718) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858) Update typemap generation code to also ignore generic types and interfaces when generating the Java to Managed map in `Release` mode. We must not simply skip outputting entries for such types because we need to maintain element count parity between java-to-managed and managed-to-java lookup tables. Therefore, the way we skip such types is to output a type token ID of `0` instead of the actual one. This will cause the runtime code to fail to find a mapping, thus allowing the managed code to deal with such a type properly. ~~ Test Updates ~~ Update `BundleTest.TestBundleIntegerArrayList2()` so that it asserts the runtime *type* of `obj` returned, asserting that it is in fact a `JavaList` and not e.g. `Java.Util.ArrayList` or something. (The linker could otherwise "change" things, and this assertion will ensure that `JavaList` won't be linked away…) Add a unit test for the Release config app bug in Issue #4660; thanks to @brendanzagaeski. It works by provoking the binary search bug by "ensuring" that a *generic* type comes *before* the "normally preferred" *non*-generic type. Thus, even if/when binary search returns the first entry, that would still be the *wrong entry*, because that'll be a generic type! [0]: https://developer.android.com/reference/java/util/ArrayList [1]: https://github.com/xamarin/xamarin-android/blob/61f09bf2ef0c8f09550e2d77eb97f417432b315b/src/Mono.Android/Android.Runtime/JavaList.cs#L11-L13 [2]: https://github.com/xamarin/xamarin-android/blob/61f09bf2ef0c8f09550e2d77eb97f417432b315b/src/Mono.Android/Android.Runtime/JavaList.cs#L667-L668 --- .../Test/Android.OS/BundleTest.cs | 1 + .../Tasks/GenerateJavaStubs.cs | 6 +- .../Utilities/TypeMapGenerator.cs | 98 ++++++++++++++++--- ...TypeMappingDebugNativeAssemblyGenerator.cs | 2 +- ...peMappingReleaseNativeAssemblyGenerator.cs | 2 +- src/monodroid/jni/embedded-assemblies.cc | 13 ++- .../BindingTests.cs | 7 ++ .../Xamarin.Android.McwGen-Tests/Timing.cs | 6 ++ .../java/com/xamarin/android/Timing.java | 4 + 9 files changed, 118 insertions(+), 21 deletions(-) diff --git a/src/Mono.Android/Test/Android.OS/BundleTest.cs b/src/Mono.Android/Test/Android.OS/BundleTest.cs index cb456af7b1f..f2e96399c5e 100644 --- a/src/Mono.Android/Test/Android.OS/BundleTest.cs +++ b/src/Mono.Android/Test/Android.OS/BundleTest.cs @@ -19,6 +19,7 @@ public void TestBundleIntegerArrayList() Assert.NotNull (list, "'key' doesn't refer to a list of integers"); var obj = b.Get ("key"); Assert.NotNull (obj, "Missing 'key' in bundle"); + Assert.IsTrue (obj is global::Android.Runtime.JavaList, "`obj` should be a JavaList!"); try { list = b.GetIntegerArrayList ("key"); Assert.NotNull (list, "'key' doesn't refer to a list of integers after non-generic call"); diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index a738f99f28e..74b26a7e49f 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -174,7 +174,7 @@ void Run (DirectoryAssemblyResolver res) // Step 2 - Generate type maps // Type mappings need to use all the assemblies, always. - WriteTypeMappings (allJavaTypes); + WriteTypeMappings (allJavaTypes, cache); var javaTypes = new List (); foreach (TypeDefinition td in allJavaTypes) { @@ -399,10 +399,10 @@ void SaveResource (string resource, string filename, string destDir, Func types) + void WriteTypeMappings (List types, TypeDefinitionCache cache) { var tmg = new TypeMapGenerator ((string message) => Log.LogDebugMessage (message), SupportedAbis); - if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState)) + if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, cache, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState)) throw new XamarinAndroidException (4308, Properties.Resources.XA4308); GeneratedBinaryTypeMaps = tmg.GeneratedBinaryTypeMaps.ToArray (); BuildEngine4.RegisterTaskObject (ApplicationConfigTaskState.RegisterTaskObjectKey, appConfState, RegisteredTaskObjectLifetime.Build, allowEarlyCollection: false); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs index 25d2ff1cd2e..82315b02caf 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Text; +using Java.Interop.Tools.Cecil; using Mono.Cecil; namespace Xamarin.Android.Tasks @@ -14,6 +15,7 @@ class TypeMapGenerator const string TypeMapIndexMagicString = "XATI"; // Xamarin Android Typemap Index const uint TypeMapFormatVersion = 2; // Keep in sync with the value in src/monodroid/jni/xamarin-app.hh const string TypemapExtension = ".typemap"; + const uint InvalidJavaToManagedMappingIndex = UInt32.MaxValue; internal sealed class ModuleUUIDArrayComparer : IComparer { @@ -52,6 +54,7 @@ internal sealed class TypeMapReleaseEntry public uint Token; public int AssemblyNameIndex = -1; public int ModuleIndex = -1; + public bool SkipInJavaToManaged; } internal sealed class ModuleReleaseData @@ -76,6 +79,8 @@ internal sealed class TypeMapDebugEntry public string ManagedLabel; public int JavaIndex; public int ManagedIndex; + public TypeDefinition TypeDefinition; + public bool SkipInJavaToManaged; } // Widths include the terminating nul character but not the padding! @@ -87,6 +92,7 @@ internal sealed class ModuleDebugData public List JavaToManagedMap; public List ManagedToJavaMap; public string OutputFilePath; + public string ModuleName; public byte[] ModuleNameBytes; } @@ -125,7 +131,7 @@ void UpdateApplicationConfig (TypeDefinition javaType, ApplicationConfigTaskStat } } - public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, string outputDirectory, bool generateNativeAssembly, out ApplicationConfigTaskState appConfState) + public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, TypeDefinitionCache cache, string outputDirectory, bool generateNativeAssembly, out ApplicationConfigTaskState appConfState) { if (String.IsNullOrEmpty (outputDirectory)) throw new ArgumentException ("must not be null or empty", nameof (outputDirectory)); @@ -140,25 +146,26 @@ public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAt string typemapsOutputDirectory = Path.Combine (outputDirectory, "typemaps"); if (debugBuild) { - return GenerateDebug (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, typemapsOutputDirectory, generateNativeAssembly, appConfState); + return GenerateDebug (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, typemapsOutputDirectory, generateNativeAssembly, appConfState); } return GenerateRelease (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, typemapsOutputDirectory, appConfState); } - bool GenerateDebug (bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, string outputDirectory, bool generateNativeAssembly, ApplicationConfigTaskState appConfState) + bool GenerateDebug (bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, TypeDefinitionCache cache, string outputDirectory, bool generateNativeAssembly, ApplicationConfigTaskState appConfState) { if (generateNativeAssembly) - return GenerateDebugNativeAssembly (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, outputDirectory, appConfState); - return GenerateDebugFiles (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, outputDirectory, appConfState); + return GenerateDebugNativeAssembly (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, outputDirectory, appConfState); + return GenerateDebugFiles (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, outputDirectory, appConfState); } - bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState) + bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, TypeDefinitionCache cache, string outputDirectory, ApplicationConfigTaskState appConfState) { var modules = new Dictionary (StringComparer.Ordinal); int maxModuleFileNameWidth = 0; int maxModuleNameWidth = 0; + var javaDuplicates = new Dictionary> (StringComparer.Ordinal); foreach (TypeDefinition td in javaTypes) { UpdateApplicationConfig (td, appConfState); string moduleName = td.Module.Assembly.Name.Name; @@ -173,7 +180,8 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L JavaToManagedMap = new List (), ManagedToJavaMap = new List (), OutputFilePath = Path.Combine (outputDirectory, outputFileName), - ModuleNameBytes = outputEncoding.GetBytes (moduleName) + ModuleName = moduleName, + ModuleNameBytes = outputEncoding.GetBytes (moduleName), }; if (module.ModuleNameBytes.Length > maxModuleNameWidth) @@ -186,6 +194,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L } TypeMapDebugEntry entry = GetDebugEntry (td); + HandleDebugDuplicates (javaDuplicates, entry, td, cache); if (entry.JavaName.Length > module.JavaNameWidth) module.JavaNameWidth = (uint)entry.JavaName.Length + 1; @@ -195,6 +204,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L module.JavaToManagedMap.Add (entry); module.ManagedToJavaMap.Add (entry); } + SyncDebugDuplicates (javaDuplicates); foreach (ModuleDebugData module in modules.Values) { PrepareDebugMaps (module); @@ -217,18 +227,22 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L return true; } - bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState) + bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, TypeDefinitionCache cache, string outputDirectory, ApplicationConfigTaskState appConfState) { var javaToManaged = new List (); var managedToJava = new List (); + var javaDuplicates = new Dictionary> (StringComparer.Ordinal); foreach (TypeDefinition td in javaTypes) { UpdateApplicationConfig (td, appConfState); TypeMapDebugEntry entry = GetDebugEntry (td); + HandleDebugDuplicates (javaDuplicates, entry, td, cache); + javaToManaged.Add (entry); managedToJava.Add (entry); } + SyncDebugDuplicates (javaDuplicates); var data = new ModuleDebugData { EntryCount = (uint)javaToManaged.Count, @@ -246,6 +260,39 @@ bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttribu return true; } + void SyncDebugDuplicates (Dictionary> javaDuplicates) + { + foreach (List duplicates in javaDuplicates.Values) { + if (duplicates.Count < 2) { + continue; + } + + TypeMapDebugEntry template = duplicates [0]; + for (int i = 1; i < duplicates.Count; i++) { + duplicates[i].TypeDefinition = template.TypeDefinition; + duplicates[i].ManagedName = template.ManagedName; + } + } + } + + void HandleDebugDuplicates (Dictionary> javaDuplicates, TypeMapDebugEntry entry, TypeDefinition td, TypeDefinitionCache cache) + { + List duplicates; + + if (!javaDuplicates.TryGetValue (entry.JavaName, out duplicates)) { + javaDuplicates.Add (entry.JavaName, new List { entry }); + } else { + duplicates.Add (entry); + TypeMapDebugEntry oldEntry = duplicates[0]; + if (td.IsAbstract || td.IsInterface || oldEntry.TypeDefinition.IsAbstract || oldEntry.TypeDefinition.IsInterface) { + if (td.IsAssignableFrom (oldEntry.TypeDefinition, cache)) { + oldEntry.TypeDefinition = td; + oldEntry.ManagedName = GetManagedTypeName (td); + } + } + } + } + void PrepareDebugMaps (ModuleDebugData module) { module.JavaToManagedMap.Sort ((TypeMapDebugEntry a, TypeMapDebugEntry b) => String.Compare (a.JavaName, b.JavaName, StringComparison.Ordinal)); @@ -261,6 +308,16 @@ void PrepareDebugMaps (ModuleDebugData module) } TypeMapDebugEntry GetDebugEntry (TypeDefinition td) + { + return new TypeMapDebugEntry { + JavaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td), + ManagedName = GetManagedTypeName (td), + TypeDefinition = td, + SkipInJavaToManaged = ShouldSkipInJavaToManaged (td), + }; + } + + string GetManagedTypeName (TypeDefinition td) { // This is necessary because Mono runtime will return to us type name with a `.` for nested types (not a // `/` or a `+`. So, for instance, a type named `DefaultRenderer` found in the @@ -277,17 +334,13 @@ TypeMapDebugEntry GetDebugEntry (TypeDefinition td) // string managedTypeName = td.FullName.Replace ('/', '+'); - return new TypeMapDebugEntry { - JavaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td), - ManagedName = $"{managedTypeName}, {td.Module.Assembly.Name.Name}", - }; + return $"{managedTypeName}, {td.Module.Assembly.Name.Name}"; } bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState) { int assemblyId = 0; int maxJavaNameLength = 0; - int maxModuleFileNameLength = 0; var knownAssemblies = new Dictionary (StringComparer.Ordinal); var tempModules = new Dictionary (); Dictionary moduleCounter = null; @@ -329,12 +382,18 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List } string javaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td); + // We will ignore generic types and interfaces when generating the Java to Managed map, but we must not + // omit them from the table we output - we need the same number of entries in both java-to-managed and + // managed-to-java tables. `SkipInJavaToManaged` set to `true` will cause the native assembly generator + // to output `0` as the token id for the type, thus effectively causing the runtime unable to match such + // a Java type name to a managed type. This fixes https://github.com/xamarin/xamarin-android/issues/4660 var entry = new TypeMapReleaseEntry { JavaName = javaName, JavaNameLength = outputEncoding.GetByteCount (javaName), ManagedTypeName = td.FullName, Token = td.MetadataToken.ToUInt32 (), - AssemblyNameIndex = knownAssemblies [assemblyName] + AssemblyNameIndex = knownAssemblies [assemblyName], + SkipInJavaToManaged = ShouldSkipInJavaToManaged (td), }; if (entry.JavaNameLength > maxJavaNameLength) @@ -376,6 +435,11 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List return true; } + bool ShouldSkipInJavaToManaged (TypeDefinition td) + { + return td.IsInterface || td.HasGenericParameters; + } + void GenerateNativeAssembly (Func getGenerator) { NativeAssemblerTargetProvider asmTargetProvider; @@ -491,6 +555,10 @@ void OutputModule (ModuleDebugData moduleData) // void OutputModule (BinaryWriter bw, ModuleDebugData moduleData) { + if ((uint)moduleData.JavaToManagedMap.Count == InvalidJavaToManagedMappingIndex) { + throw new InvalidOperationException ($"Too many types in module {moduleData.ModuleName}"); + } + bw.Write (moduleMagicString); bw.Write (TypeMapFormatVersion); bw.Write (moduleData.JavaToManagedMap.Count); @@ -502,7 +570,7 @@ void OutputModule (BinaryWriter bw, ModuleDebugData moduleData) foreach (TypeMapDebugEntry entry in moduleData.JavaToManagedMap) { bw.Write (outputEncoding.GetBytes (entry.JavaName)); PadField (bw, entry.JavaName.Length, (int)moduleData.JavaNameWidth); - bw.Write (entry.ManagedIndex); + bw.Write (entry.SkipInJavaToManaged ? InvalidJavaToManagedMappingIndex : (uint)entry.ManagedIndex); } foreach (TypeMapDebugEntry entry in moduleData.ManagedToJavaMap) { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs index fb9d51d2218..3ae7ba809e4 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs @@ -70,7 +70,7 @@ protected override void WriteSymbols (StreamWriter output) if (haveJavaToManaged) { foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.JavaToManagedMap) { size += WritePointer (output, entry.JavaLabel); - size += WritePointer (output, entry.ManagedLabel); + size += WritePointer (output, entry.SkipInJavaToManaged ? null : entry.ManagedLabel); } } WriteStructureSize (output, JavaToManagedSymbol, size, alwaysWriteSize: true); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs index efb6253177c..7fff3f64abb 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs @@ -219,7 +219,7 @@ uint WriteJavaMapEntry (StreamWriter output, TypeMapGenerator.TypeMapReleaseEntr size += WriteData (output, entry.ModuleIndex); WriteCommentLine (output, "type_token_id"); - size += WriteData (output, entry.Token); + size += WriteData (output, entry.SkipInJavaToManaged ? 0 : entry.Token); WriteCommentLine (output, "java_name"); size += WriteAsciiData (output, entry.JavaName, mappingData.JavaNameWidth); diff --git a/src/monodroid/jni/embedded-assemblies.cc b/src/monodroid/jni/embedded-assemblies.cc index 3debfc31fa3..d9e9ec6954a 100644 --- a/src/monodroid/jni/embedded-assemblies.cc +++ b/src/monodroid/jni/embedded-assemblies.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -208,6 +209,10 @@ EmbeddedAssemblies::typemap_java_to_managed (const char *java_type_name) } const char *managed_type_name = entry->to; + if (managed_type_name == nullptr) { + log_debug (LOG_ASSEMBLY, "typemap: Java type '%s' maps either to an open generic type or an interface type."); + return nullptr; + } log_debug (LOG_DEFAULT, "typemap: Java type '%s' corresponds to managed type '%s'", java_type_name, managed_type_name); MonoType *type = mono_reflection_type_from_name (const_cast(managed_type_name), nullptr); @@ -731,12 +736,18 @@ EmbeddedAssemblies::typemap_load_file (BinaryTypeMapHeader &header, const char * uint8_t *managed_pos = managed_start; TypeMapEntry *cur; + constexpr uint32_t INVALID_TYPE_INDEX = UINT32_MAX; for (size_t i = 0; i < module.entry_count; i++) { cur = &module.java_to_managed[i]; cur->from = reinterpret_cast(java_pos); uint32_t idx = *(reinterpret_cast(java_pos + header.java_name_width)); - cur->to = reinterpret_cast(managed_start + (managed_entry_size * idx)); + if (idx < INVALID_TYPE_INDEX) { + cur->to = reinterpret_cast(managed_start + (managed_entry_size * idx)); + } else { + // Ignore the type mapping + cur->to = nullptr; + } java_pos += java_entry_size; cur = &module.managed_to_java[i]; diff --git a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs index d6cb60dbb9a..568096e40e8 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs +++ b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs @@ -12,6 +12,13 @@ namespace Xamarin.Android.JcwGenTests { [TestFixture] public class BindingTests { + [Test] + public void TestTimingCreateTimingIsCorrectType () + { + var t = Com.Xamarin.Android.Timing.CreateTiming (); + Assert.IsTrue (t is Com.Xamarin.Android.Timing); + } + [Test] public void TestResourceId () { diff --git a/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Timing.cs b/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Timing.cs index 0f9a4facff8..dd592581889 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Timing.cs +++ b/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Timing.cs @@ -8,6 +8,12 @@ namespace Com.Xamarin.Android { + // Provoke https://github.com/xamarin/xamarin-android/issues/4660 + // We need this type to appear *before* `Timing` in `monodis --typedef` output + [Register ("com/xamarin/android/Timing", DoNotGenerateAcw = true)] + public partial class Timinf : Timing { + } + public partial class Timing { static IntPtr jonp_id_VirtualVoidMethod; diff --git a/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/java/com/xamarin/android/Timing.java b/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/java/com/xamarin/android/Timing.java index 53d06b1a7a1..472afedb93e 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/java/com/xamarin/android/Timing.java +++ b/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/java/com/xamarin/android/Timing.java @@ -4,6 +4,10 @@ public class Timing { + public static Timing createTiming () { + return new Timing (); + } + public static void StaticVoidMethod () { }