From cc6bfa30f97875d0bb6da9b85e6a2f9a3e98f777 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 13 Feb 2025 16:32:09 -0600 Subject: [PATCH 1/2] [nativeaot] fix typemap logic involving duplicates 70bd636b introduced an "MVP" for a managed typemap for NativeAOT. Unfortunately, a `dotnet new maui` project template still crashes with: AndroidRuntime: Process: net.dot.hellonativeaot, PID: 16075 AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidCastException: Arg_InvalidCastException AndroidRuntime: at Java.Lang.Object._GetObject[T](IntPtr, JniHandleOwnership) + 0x64 AndroidRuntime: at Microsoft.Maui.WindowOverlay.Initialize() + 0x168 AndroidRuntime: at Microsoft.Maui.Handlers.WindowHandler.OnRootViewChanged(Object sender, EventArgs e) + 0x8c AndroidRuntime: at Microsoft.Maui.Platform.NavigationRootManager.SetContentView(IView) + 0x17c AndroidRuntime: at Microsoft.Maui.Platform.NavigationRootManager.Connect(IView, IMauiContext) + 0xf0 AndroidRuntime: at Microsoft.Maui.Handlers.WindowHandler.CreateRootViewFromContent(IWindowHandler, IWindow) + 0x4c AndroidRuntime: at Microsoft.Maui.Handlers.WindowHandler.MapContent(IWindowHandler handler, IWindow window) + 0x20 AndroidRuntime: at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.b__0(IElementHandler h, IElement v) + 0x130 AndroidRuntime: at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String, IElementHandler, IElement) + 0x58 AndroidRuntime: at Microsoft.Maui.PropertyMapper.UpdateProperties(IElementHandler, IElement) + 0x74 AndroidRuntime: at Microsoft.Maui.Handlers.ElementHandler.SetVirtualView(IElement) + 0x15c AndroidRuntime: at Microsoft.Maui.Controls.Element.SetHandler(IElementHandler) + 0x124 AndroidRuntime: at Microsoft.Maui.Controls.Element.set_Handler(IElementHandler value) + 0xc AndroidRuntime: at Microsoft.Maui.Platform.ElementExtensions.SetHandler(Context, IElement, IMauiContext) + 0xfc AndroidRuntime: at Microsoft.Maui.Platform.ApplicationExtensions.CreatePlatformWindow(Activity, IApplication, Bundle) + 0x184 AndroidRuntime: at Microsoft.Maui.MauiAppCompatActivity.OnCreate(Bundle) + 0x74 AndroidRuntime: at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_savedInstanceState) + 0xc0 AndroidRuntime: at my.MainActivity.n_onCreate(Native Method) AndroidRuntime: at my.MainActivity.onCreate(MainActivity.java:36) AndroidRuntime: at android.app.Activity.performCreate(Activity.java:8960) AndroidRuntime: at android.app.Activity.performCreate(Activity.java:8938) AndroidRuntime: at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1526) AndroidRuntime: at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3975) AndroidRuntime: at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:4173) AndroidRuntime: at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:114) AndroidRuntime: at android.app.servertransaction.TransactionExecutor.executeNonLifecycleItem(TransactionExecutor.java:231) AndroidRuntime: at android.app.servertransaction.TransactionExecutor.executeTransactionItems(TransactionExecutor.java:152) AndroidRuntime: at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:93) AndroidRuntime: at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2595) AndroidRuntime: at android.os.Handler.dispatchMessage(Handler.java:107) AndroidRuntime: at android.os.Looper.loopOnce(Looper.java:232) AndroidRuntime: at android.os.Looper.loop(Looper.java:317) AndroidRuntime: at android.app.ActivityThread.main(ActivityThread.java:8592) AndroidRuntime: at java.lang.reflect.Method.invoke(Native Method) AndroidRuntime: at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580) AndroidRuntime: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:878) One cause may be that `Java.Interop.JavaObject` is in the typemap in favor of `Java.Lang.Object`! This could cause `InvalidCastException`. The typemap implementation is missing two edge cases: 1. Types in `Mono.Android.dll` should be preferred over types in other assemblies. 2. Abstract or interface types should be preferred over their equivalent `*Invoker` types. There was also some missing logic regarding `*Invoker` types and abstract types. I fixed these cases in `TypeMappingStep.cs` and added a test that can validate the typemap during a build. --- .../TypeMappingStep.cs | 44 ++++++++++++++++--- .../Xamarin.Android.Build.Tests/BuildTest2.cs | 42 ++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs b/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs index 9b82e500fd8..6aa0185aa27 100644 --- a/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs +++ b/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs @@ -16,7 +16,7 @@ public class TypeMappingStep : BaseStep { const string AssemblyName = "Microsoft.Android.Runtime.NativeAOT"; const string TypeName = "Microsoft.Android.Runtime.NativeAotTypeManager"; - readonly IDictionary TypeMappings = new Dictionary (StringComparer.Ordinal); + readonly IDictionary> TypeMappings = new Dictionary> (StringComparer.Ordinal); AssemblyDefinition? MicrosoftAndroidRuntimeNativeAot; protected override void ProcessAssembly (AssemblyDefinition assembly) @@ -66,7 +66,7 @@ protected override void EndProcess () var il = method.Body.GetILProcessor (); var addMethod = module.ImportReference (typeof (IDictionary).GetMethod ("Add")); var getTypeFromHandle = module.ImportReference (typeof (Type).GetMethod ("GetTypeFromHandle")); - foreach (var (javaKey, typeDefinition) in TypeMappings) { + foreach (var (javaName, list) in TypeMappings) { /* * IL_0000: ldarg.0 * IL_0001: ldfld class [System.Runtime]System.Collections.Generic.IDictionary`2 Microsoft.Android.Runtime.NativeAotTypeManager::TypeMappings @@ -77,8 +77,8 @@ protected override void EndProcess () */ il.Emit (Mono.Cecil.Cil.OpCodes.Ldarg_0); il.Emit (Mono.Cecil.Cil.OpCodes.Ldfld, field); - il.Emit (Mono.Cecil.Cil.OpCodes.Ldstr, javaKey); - il.Emit (Mono.Cecil.Cil.OpCodes.Ldtoken, module.ImportReference (typeDefinition)); + il.Emit (Mono.Cecil.Cil.OpCodes.Ldstr, javaName); + il.Emit (Mono.Cecil.Cil.OpCodes.Ldtoken, module.ImportReference (SelectTypeDefinition (javaName, list))); il.Emit (Mono.Cecil.Cil.OpCodes.Call, getTypeFromHandle); il.Emit (Mono.Cecil.Cil.OpCodes.Callvirt, addMethod); } @@ -86,12 +86,44 @@ protected override void EndProcess () il.Emit (Mono.Cecil.Cil.OpCodes.Ret); } + TypeDefinition SelectTypeDefinition (string javaName, List list) + { + if (list.Count == 1) + return list[0]; + + var best = list[0]; + foreach (var type in list) { + if (type == best) + continue; + // We found the `Invoker` type *before* the declared type + // Fix things up so the abstract type is first, and the `Invoker` is considered a duplicate. + if ((type.IsAbstract || type.IsInterface) && + !best.IsAbstract && + !best.IsInterface && + type.IsAssignableFrom (best, Context)) { + best = type; + } + } + foreach (var type in list) { + if (type == best) + continue; + Context.LogMessage ($"Duplicate typemap entry for {javaName} => {type.FullName}"); + } + return best; + } + void ProcessType (AssemblyDefinition assembly, TypeDefinition type) { if (type.HasJavaPeer (Context)) { var javaName = JavaNativeTypeManager.ToJniName (type, Context); - if (!TypeMappings.TryAdd (javaName, type)) { - Context.LogMessage ($"Duplicate typemap entry for {javaName}"); + if (!TypeMappings.TryGetValue (javaName, out var list)) { + TypeMappings.Add (javaName, list = new List ()); + } + // Types in Mono.Android assembly should be first in the list + if (assembly.Name.Name == "Mono.Android") { + list.Insert (0, type); + } else { + list.Add (type); } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs index 77045ab29c0..bbc29aabb75 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs @@ -135,6 +135,7 @@ public void NativeAOT () proj.SetProperty ("PublishAot", "true"); proj.SetProperty ("PublishAotUsingRuntimePack", "true"); proj.SetProperty ("AndroidNdkDirectory", AndroidNdkPath); + proj.SetProperty ("_ExtraTrimmerArgs", "--verbose"); using var b = CreateApkBuilder (); Assert.IsTrue (b.Build (proj), "Build should have succeeded."); @@ -165,6 +166,38 @@ public void NativeAOT () Assert.IsNotNull (method, $"{linkedMonoAndroidAssembly} should contain {typeName}.{methodName}"); } + var typemap = new Dictionary (); + var linkedRuntimeAssembly = Path.Combine (intermediate, "linked", "Microsoft.Android.Runtime.NativeAOT.dll"); + FileAssert.Exists (linkedRuntimeAssembly); + using (var assembly = AssemblyDefinition.ReadAssembly (linkedRuntimeAssembly)) { + var type = assembly.MainModule.Types.FirstOrDefault (t => t.Name == "NativeAotTypeManager"); + Assert.IsNotNull (type, $"{linkedRuntimeAssembly} should contain NativeAotTypeManager"); + var method = type.Methods.FirstOrDefault (m => m.Name == "InitializeTypeMappings"); + Assert.IsNotNull (method, "NativeAotTypeManager should contain InitializeTypeMappings"); + + foreach (var i in method.Body.Instructions) { + if (i.OpCode != Mono.Cecil.Cil.OpCodes.Ldstr) + continue; + if (i.Operand is not string javaName) + continue; + if (i.Next.Operand is not TypeReference t) + continue; + typemap.Add (javaName, t); + } + + // Basic types + AssertTypeMap ("java/lang/Object", "Java.Lang.Object"); + AssertTypeMap ("java/lang/String", "Java.Lang.String"); + AssertTypeMap ("android/app/Activity", "Android.App.Activity"); + AssertTypeMap ("android/widget/Button", "Android.Widget.Button"); + + // Special *Invoker case + Assert.IsFalse (StringAssertEx.ContainsText (b.LastBuildOutput, + "ILLink: Duplicate typemap entry for android/view/View$OnClickListener => Android.Views.View/IOnClickListenerInvoker"), + "Should get log message about duplicate IOnClickListenerInvoker!"); + AssertTypeMap ("android/view/View$OnClickListener", "Android.Views.View/IOnClickListener"); + } + var dexFile = Path.Combine (intermediate, "android", "bin", "classes.dex"); FileAssert.Exists (dexFile); foreach (var className in mono_classes) { @@ -180,6 +213,15 @@ public void NativeAOT () foreach (var nativeaot_file in nativeaot_files) { Assert.IsTrue (zip.ContainsEntry (nativeaot_file, caseSensitive: true), $"APK must contain `{nativeaot_file}`."); } + + void AssertTypeMap(string javaName, string managedName) + { + if (typemap.TryGetValue (javaName, out var reference)) { + Assert.AreEqual (managedName, reference.ToString ()); + } else { + Assert.Fail ($"InitializeTypeMappings should contain Ldstr \"{javaName}\"!"); + } + } } [Test] From b0c99b8a0e9a2fa7cd5e4e05363b602895679e22 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 14 Feb 2025 11:08:34 -0600 Subject: [PATCH 2/2] Unify sorting for `java/util/ArrayList` --- .../TypeMappingStep.cs | 14 ++++++++------ .../Xamarin.Android.Build.Tests/BuildTest2.cs | 13 +++++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs b/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs index 6aa0185aa27..0288433f8f3 100644 --- a/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs +++ b/src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs @@ -95,6 +95,12 @@ TypeDefinition SelectTypeDefinition (string javaName, List list) foreach (var type in list) { if (type == best) continue; + // Types in Mono.Android assembly should be first in the list + if (best.Module.Assembly.Name.Name != "Mono.Android" && + type.Module.Assembly.Name.Name == "Mono.Android") { + best = type; + continue; + } // We found the `Invoker` type *before* the declared type // Fix things up so the abstract type is first, and the `Invoker` is considered a duplicate. if ((type.IsAbstract || type.IsInterface) && @@ -102,6 +108,7 @@ TypeDefinition SelectTypeDefinition (string javaName, List list) !best.IsInterface && type.IsAssignableFrom (best, Context)) { best = type; + continue; } } foreach (var type in list) { @@ -119,12 +126,7 @@ void ProcessType (AssemblyDefinition assembly, TypeDefinition type) if (!TypeMappings.TryGetValue (javaName, out var list)) { TypeMappings.Add (javaName, list = new List ()); } - // Types in Mono.Android assembly should be first in the list - if (assembly.Name.Name == "Mono.Android") { - list.Insert (0, type); - } else { - list.Add (type); - } + list.Add (type); } if (!type.HasNestedTypes) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs index bbc29aabb75..d24cbec9d06 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs @@ -137,6 +137,10 @@ public void NativeAOT () proj.SetProperty ("AndroidNdkDirectory", AndroidNdkPath); proj.SetProperty ("_ExtraTrimmerArgs", "--verbose"); + // Required for java/util/ArrayList assertion below + proj.MainActivity = proj.DefaultMainActivity + .Replace ("//${AFTER_ONCREATE}", "new Android.Runtime.JavaList (); new Android.Runtime.JavaList ();"); + using var b = CreateApkBuilder (); Assert.IsTrue (b.Build (proj), "Build should have succeeded."); b.Output.AssertTargetIsNotSkipped ("_PrepareLinking"); @@ -188,14 +192,19 @@ public void NativeAOT () // Basic types AssertTypeMap ("java/lang/Object", "Java.Lang.Object"); AssertTypeMap ("java/lang/String", "Java.Lang.String"); + AssertTypeMap ("[Ljava/lang/Object;", "Java.Interop.JavaArray`1"); + AssertTypeMap ("java/util/ArrayList", "Android.Runtime.JavaList"); AssertTypeMap ("android/app/Activity", "Android.App.Activity"); AssertTypeMap ("android/widget/Button", "Android.Widget.Button"); + Assert.IsFalse (StringAssertEx.ContainsText (b.LastBuildOutput, + "Duplicate typemap entry for java/util/ArrayList => Android.Runtime.JavaList`1"), + "Should get log message about duplicate Android.Runtime.JavaList`1!"); // Special *Invoker case + AssertTypeMap ("android/view/View$OnClickListener", "Android.Views.View/IOnClickListener"); Assert.IsFalse (StringAssertEx.ContainsText (b.LastBuildOutput, - "ILLink: Duplicate typemap entry for android/view/View$OnClickListener => Android.Views.View/IOnClickListenerInvoker"), + "Duplicate typemap entry for android/view/View$OnClickListener => Android.Views.View/IOnClickListenerInvoker"), "Should get log message about duplicate IOnClickListenerInvoker!"); - AssertTypeMap ("android/view/View$OnClickListener", "Android.Views.View/IOnClickListener"); } var dexFile = Path.Combine (intermediate, "android", "bin", "classes.dex");