Skip to content

Commit 6b1789b

Browse files
jonathanpeppersjonathanpeppers
authored
[NativeAOT] fix typemap logic involving duplicates (#9794)
Context: 70bd636 Context: #9747 Context: dotnet/java-interop@2a7183a Context: 7acf328 Commit 70bd636 introduced an "Minimum Viable Product" for a managed typemap for NativeAOT. Unfortunately, a `dotnet new maui` project template (#9747) still crashed 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.<Add>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) The actual cause of the above `InvalidCastException` was fixed in dotnet/java-interop@2a7183a1. However, we also determined that the typemap associated `java/lang/Object` with `Java.Interop.JavaObject`, *not* to `Java.Lang.Object`, because 70bd636 processed `Java.Interop.dll` before `Mono.Android.dll` and other assemblies. (Related: commit 7acf328, which began treating `Java.Interop.dll` as an assembly that could contain type mappings, and had to be special- cased in order to ensure `java/lang/Object` was mapped to `Java.Lang.Object, Mono.Android`.) The typemap implementation was 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 validates the typemap during a build.
1 parent dafc1aa commit 6b1789b

File tree

2 files changed

+91
-6
lines changed

2 files changed

+91
-6
lines changed

‎src/Microsoft.Android.Sdk.ILLink/TypeMappingStep.cs

+40-6
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public class TypeMappingStep : BaseStep
1616
{
1717
const string AssemblyName = "Microsoft.Android.Runtime.NativeAOT";
1818
const string TypeName = "Microsoft.Android.Runtime.NativeAotTypeManager";
19-
readonly IDictionary<string, TypeDefinition> TypeMappings = new Dictionary<string, TypeDefinition> (StringComparer.Ordinal);
19+
readonly IDictionary<string, List<TypeDefinition>> TypeMappings = new Dictionary<string, List<TypeDefinition>> (StringComparer.Ordinal);
2020
AssemblyDefinition? MicrosoftAndroidRuntimeNativeAot;
2121

2222
protected override void ProcessAssembly (AssemblyDefinition assembly)
@@ -66,7 +66,7 @@ protected override void EndProcess ()
6666
var il = method.Body.GetILProcessor ();
6767
var addMethod = module.ImportReference (typeof (IDictionary<string, Type>).GetMethod ("Add"));
6868
var getTypeFromHandle = module.ImportReference (typeof (Type).GetMethod ("GetTypeFromHandle"));
69-
foreach (var (javaKey, typeDefinition) in TypeMappings) {
69+
foreach (var (javaName, list) in TypeMappings) {
7070
/*
7171
* IL_0000: ldarg.0
7272
* IL_0001: ldfld class [System.Runtime]System.Collections.Generic.IDictionary`2<string, class [System.Runtime]System.Type> Microsoft.Android.Runtime.NativeAotTypeManager::TypeMappings
@@ -77,22 +77,56 @@ protected override void EndProcess ()
7777
*/
7878
il.Emit (Mono.Cecil.Cil.OpCodes.Ldarg_0);
7979
il.Emit (Mono.Cecil.Cil.OpCodes.Ldfld, field);
80-
il.Emit (Mono.Cecil.Cil.OpCodes.Ldstr, javaKey);
81-
il.Emit (Mono.Cecil.Cil.OpCodes.Ldtoken, module.ImportReference (typeDefinition));
80+
il.Emit (Mono.Cecil.Cil.OpCodes.Ldstr, javaName);
81+
il.Emit (Mono.Cecil.Cil.OpCodes.Ldtoken, module.ImportReference (SelectTypeDefinition (javaName, list)));
8282
il.Emit (Mono.Cecil.Cil.OpCodes.Call, getTypeFromHandle);
8383
il.Emit (Mono.Cecil.Cil.OpCodes.Callvirt, addMethod);
8484
}
8585

8686
il.Emit (Mono.Cecil.Cil.OpCodes.Ret);
8787
}
8888

89+
TypeDefinition SelectTypeDefinition (string javaName, List<TypeDefinition> list)
90+
{
91+
if (list.Count == 1)
92+
return list[0];
93+
94+
var best = list[0];
95+
foreach (var type in list) {
96+
if (type == best)
97+
continue;
98+
// Types in Mono.Android assembly should be first in the list
99+
if (best.Module.Assembly.Name.Name != "Mono.Android" &&
100+
type.Module.Assembly.Name.Name == "Mono.Android") {
101+
best = type;
102+
continue;
103+
}
104+
// We found the `Invoker` type *before* the declared type
105+
// Fix things up so the abstract type is first, and the `Invoker` is considered a duplicate.
106+
if ((type.IsAbstract || type.IsInterface) &&
107+
!best.IsAbstract &&
108+
!best.IsInterface &&
109+
type.IsAssignableFrom (best, Context)) {
110+
best = type;
111+
continue;
112+
}
113+
}
114+
foreach (var type in list) {
115+
if (type == best)
116+
continue;
117+
Context.LogMessage ($"Duplicate typemap entry for {javaName} => {type.FullName}");
118+
}
119+
return best;
120+
}
121+
89122
void ProcessType (AssemblyDefinition assembly, TypeDefinition type)
90123
{
91124
if (type.HasJavaPeer (Context)) {
92125
var javaName = JavaNativeTypeManager.ToJniName (type, Context);
93-
if (!TypeMappings.TryAdd (javaName, type)) {
94-
Context.LogMessage ($"Duplicate typemap entry for {javaName}");
126+
if (!TypeMappings.TryGetValue (javaName, out var list)) {
127+
TypeMappings.Add (javaName, list = new List<TypeDefinition> ());
95128
}
129+
list.Add (type);
96130
}
97131

98132
if (!type.HasNestedTypes)

‎src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs

+51
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,11 @@ public void NativeAOT ()
135135
proj.SetProperty ("PublishAot", "true");
136136
proj.SetProperty ("PublishAotUsingRuntimePack", "true");
137137
proj.SetProperty ("AndroidNdkDirectory", AndroidNdkPath);
138+
proj.SetProperty ("_ExtraTrimmerArgs", "--verbose");
139+
140+
// Required for java/util/ArrayList assertion below
141+
proj.MainActivity = proj.DefaultMainActivity
142+
.Replace ("//${AFTER_ONCREATE}", "new Android.Runtime.JavaList (); new Android.Runtime.JavaList<int> ();");
138143

139144
using var b = CreateApkBuilder ();
140145
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
@@ -165,6 +170,43 @@ public void NativeAOT ()
165170
Assert.IsNotNull (method, $"{linkedMonoAndroidAssembly} should contain {typeName}.{methodName}");
166171
}
167172

173+
var typemap = new Dictionary<string, TypeReference> ();
174+
var linkedRuntimeAssembly = Path.Combine (intermediate, "linked", "Microsoft.Android.Runtime.NativeAOT.dll");
175+
FileAssert.Exists (linkedRuntimeAssembly);
176+
using (var assembly = AssemblyDefinition.ReadAssembly (linkedRuntimeAssembly)) {
177+
var type = assembly.MainModule.Types.FirstOrDefault (t => t.Name == "NativeAotTypeManager");
178+
Assert.IsNotNull (type, $"{linkedRuntimeAssembly} should contain NativeAotTypeManager");
179+
var method = type.Methods.FirstOrDefault (m => m.Name == "InitializeTypeMappings");
180+
Assert.IsNotNull (method, "NativeAotTypeManager should contain InitializeTypeMappings");
181+
182+
foreach (var i in method.Body.Instructions) {
183+
if (i.OpCode != Mono.Cecil.Cil.OpCodes.Ldstr)
184+
continue;
185+
if (i.Operand is not string javaName)
186+
continue;
187+
if (i.Next.Operand is not TypeReference t)
188+
continue;
189+
typemap.Add (javaName, t);
190+
}
191+
192+
// Basic types
193+
AssertTypeMap ("java/lang/Object", "Java.Lang.Object");
194+
AssertTypeMap ("java/lang/String", "Java.Lang.String");
195+
AssertTypeMap ("[Ljava/lang/Object;", "Java.Interop.JavaArray`1");
196+
AssertTypeMap ("java/util/ArrayList", "Android.Runtime.JavaList");
197+
AssertTypeMap ("android/app/Activity", "Android.App.Activity");
198+
AssertTypeMap ("android/widget/Button", "Android.Widget.Button");
199+
Assert.IsFalse (StringAssertEx.ContainsText (b.LastBuildOutput,
200+
"Duplicate typemap entry for java/util/ArrayList => Android.Runtime.JavaList`1"),
201+
"Should get log message about duplicate Android.Runtime.JavaList`1!");
202+
203+
// Special *Invoker case
204+
AssertTypeMap ("android/view/View$OnClickListener", "Android.Views.View/IOnClickListener");
205+
Assert.IsFalse (StringAssertEx.ContainsText (b.LastBuildOutput,
206+
"Duplicate typemap entry for android/view/View$OnClickListener => Android.Views.View/IOnClickListenerInvoker"),
207+
"Should get log message about duplicate IOnClickListenerInvoker!");
208+
}
209+
168210
var dexFile = Path.Combine (intermediate, "android", "bin", "classes.dex");
169211
FileAssert.Exists (dexFile);
170212
foreach (var className in mono_classes) {
@@ -180,6 +222,15 @@ public void NativeAOT ()
180222
foreach (var nativeaot_file in nativeaot_files) {
181223
Assert.IsTrue (zip.ContainsEntry (nativeaot_file, caseSensitive: true), $"APK must contain `{nativeaot_file}`.");
182224
}
225+
226+
void AssertTypeMap(string javaName, string managedName)
227+
{
228+
if (typemap.TryGetValue (javaName, out var reference)) {
229+
Assert.AreEqual (managedName, reference.ToString ());
230+
} else {
231+
Assert.Fail ($"InitializeTypeMappings should contain Ldstr \"{javaName}\"!");
232+
}
233+
}
183234
}
184235

185236
[Test]

0 commit comments

Comments
 (0)