Skip to content

Commit 6836818

Browse files
authored
[Xamarin.Android.Build.Tasks] per-RID assemblies & typemaps (dotnet#8164)
Context: 929e701 Context: ce2bc68 Context: dotnet#7473 Context: dotnet#8155 The managed linker can produce assemblies optimized for the target `$(RuntimeIdentifier)` (RID), which means that they will differ between different RIDs. Our "favorite" example of this is `IntPtr.Size`, which is inlined by the linker into `4` or `8` when targeting 32-bit or 64-bit platforms. (See also dotnet#7473 and 929e701.) Another platform difference may come in the shape of CPU intrinsics which will change the JIT-generated native code in ways that will crash the application if the assembler instructions generated for the intrinsics aren't supported by the underlying processor. In addition, the per-RID assemblies will have different [MVID][0]s and **may** have different type and method metadata token IDs, which is important because typemaps *use* type and metadata token IDs; see also ce2bc68. All of this taken together invalidates our previous assumption that all the managed assemblies are identical. "Simply" using `IntPtr.Size` in an assembly that contains `Java.Lang.Object` subclasses will break things. This in turn could cause "mysterious" behavior or crashes in Release applications; see also Issue dotnet#8155. Prevent the potential problems by processing each per-RID assembly separately and output correct per-RID LLVM IR assembly using the appropriate per-RID information. Additionally, during testing I found that for our use of Cecil within `<GenerateJavaStubs/>` doesn't consistently remove the fields, delegates, and methods we remove in `MarshalMethodsAssemblyRewriter` when marshal methods are enabled, or it generates subtly broken assemblies which cause **some** applications to segfault at run time like so: I monodroid-gc: 1 outstanding GREFs. Performing a full GC! F libc : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x8 in tid 12379 (t6.helloandroid), pid 12379 (t6.helloandroid) F DEBUG : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** F DEBUG : Build fingerprint: 'google/raven_beta/raven:14/UPB3.230519.014/10284690:user/release-keys' F DEBUG : Revision: 'MP1.0' F DEBUG : ABI: 'arm64' F DEBUG : Timestamp: 2023-07-04 22:09:58.762982002+0200 F DEBUG : Process uptime: 1s F DEBUG : Cmdline: com.microsoft.net6.helloandroid F DEBUG : pid: 12379, tid: 12379, name: t6.helloandroid >>> com.microsoft.net6.helloandroid <<< F DEBUG : uid: 10288 F DEBUG : tagged_addr_ctrl: 0000000000000001 (PR_TAGGED_ADDR_ENABLE) F DEBUG : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0000000000000008 F DEBUG : Cause: null pointer dereference F DEBUG : x0 0000000000000000 x1 0000007ba1401af0 x2 00000000000000fa x3 0000000000000001 F DEBUG : x4 0000007ba1401b38 x5 0000007b9f2a8360 x6 0000000000000000 x7 0000000000000000 F DEBUG : x8 ffffffffffc00000 x9 0000007b9f800000 x10 0000000000000000 x11 0000007ba1400000 F DEBUG : x12 0000000000000000 x13 0000007ba374ad58 x14 0000000000000000 x15 00000013ead77d66 F DEBUG : x16 0000007ba372f210 x17 0000007ebdaa4a80 x18 0000007edf612000 x19 000000000000001f F DEBUG : x20 0000000000000000 x21 0000007b9f2a8320 x22 0000007b9fb02000 x23 0000000000000018 F DEBUG : x24 0000007ba374ad08 x25 0000000000000004 x26 0000007b9f2a4618 x27 0000000000000000 F DEBUG : x28 ffffffffffffffff x29 0000007fc592a780 F DEBUG : lr 0000007ba3701f44 sp 0000007fc592a730 pc 0000007ba3701e0c pst 0000000080001000 F DEBUG : 8 total frames F DEBUG : backtrace: F DEBUG : #00 pc 00000000002d4e0c /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877) F DEBUG : #1 pc 00000000002c29e8 /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877) F DEBUG : #2 pc 00000000002c34bc /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877) F DEBUG : #3 pc 00000000002c2254 /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877) F DEBUG : dotnet#4 pc 00000000002be0bc /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877) F DEBUG : dotnet#5 pc 00000000002bf050 /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877) F DEBUG : dotnet#6 pc 00000000002a53a4 /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (mono_gc_collect+44) (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877) F DEBUG : dotnet#7 pc 000000000000513c <anonymous:7ec716b000> This is because we generate Java Callable Wrappers over a set of original (linked or not) assemblies, then we scan them for classes derived from `Java.Lang.Object` and use that set as input to the marshal methods rewriter, which makes the changes (generates wrapper methods, decorates wrapped methods with `[UnmanagedCallersOnly]`, removes the old delegate methods as well as delegate backing fields) to all the `Java.Lang.Object` subclasses, then writes the modified assembly to a `new/<assembly.dll>` location (efa14e2), followed by copying the newly written assemblies back to the original location. At this point, we have the results returned by the subclass scanner in memory and **new** versions of those types on disk, but they are out of sync, since the types in memory refer to the **old** assemblies, but AOT is ran on the **new** assemblies which have a different layout, changed MVIDs and, potentially, different type and method token IDs (because we added some methods, removed others etc) and thus it causes the crashes at the run time. The now invalid set of "old" types is passed to the typemap generator. This only worked by accident, because we (incorrectly) used only the first linked assembly which happened to be the same one passed to the JLO scanner and AOT - so everything was fine at the execution time. Address this by *disabling* LLVM Marshal Methods (8bc7a3e) for .NET 8, setting `$(AndroidEnableMarshalMethods)`=False by default. We'll attempt to fix these issues for .NET 9. [0]: https://learn.microsoft.com/dotnet/api/system.reflection.module.moduleversionid?view=net-7.0
1 parent f21e10d commit 6836818

16 files changed

+1123
-354
lines changed

src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs

Lines changed: 206 additions & 70 deletions
Large diffs are not rendered by default.

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,10 +1326,10 @@ public void Dispose ()
13261326
using (var builder = CreateApkBuilder (Path.Combine ("temp", TestContext.CurrentContext.Test.Name))) {
13271327
builder.ThrowOnBuildFailure = false;
13281328
Assert.IsFalse (builder.Build (proj), "Build should have failed with XA4212.");
1329-
StringAssertEx.Contains ($"error XA4", builder.LastBuildOutput, "Error should be XA4212");
1329+
StringAssertEx.Contains ($"error : XA4", builder.LastBuildOutput, "Error should be XA4212");
13301330
StringAssertEx.Contains ($"Type `UnnamedProject.MyBadJavaObject` implements `Android.Runtime.IJavaObject`", builder.LastBuildOutput, "Error should mention MyBadJavaObject");
13311331
Assert.IsTrue (builder.Build (proj, parameters: new [] { "AndroidErrorOnCustomJavaObject=False" }), "Build should have succeeded.");
1332-
StringAssertEx.Contains ($"warning XA4", builder.LastBuildOutput, "warning XA4212");
1332+
StringAssertEx.Contains ($"warning : XA4", builder.LastBuildOutput, "warning XA4212");
13331333
}
13341334
}
13351335

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public partial class BuildTest2 : BaseTest
2626
new object[] {
2727
/* isClassic */ false,
2828
/* isRelease */ true,
29-
/* marshalMethodsEnabled */ true,
29+
/* marshalMethodsEnabled */ false,
3030
},
3131
new object[] {
3232
/* isClassic */ false,

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
using System;
22
using System.IO;
33
using System.Linq;
4+
using System.Text;
45
using Java.Interop.Tools.Cecil;
56
using Mono.Cecil;
7+
using Mono.Cecil.Cil;
68
using Mono.Linker;
9+
using Mono.Tuner;
710
using MonoDroid.Tuner;
811
using NUnit.Framework;
912
using Xamarin.ProjectTools;
@@ -514,7 +517,7 @@ public void AndroidUseNegotiateAuthentication ([Values (true, false, null)] bool
514517
}
515518

516519
[Test]
517-
public void DoNotErrorOnPerArchJavaTypeDuplicates ()
520+
public void DoNotErrorOnPerArchJavaTypeDuplicates ([Values(true, false)] bool enableMarshalMethods)
518521
{
519522
if (!Builder.UseDotNet)
520523
Assert.Ignore ("Test only valid on .NET");
@@ -525,26 +528,73 @@ public void DoNotErrorOnPerArchJavaTypeDuplicates ()
525528
lib.Sources.Add (new BuildItem.Source ("Library1.cs") {
526529
TextContent = () => @"
527530
namespace Lib1;
528-
public class Library1 : Java.Lang.Object {
531+
public class Library1 : Com.Example.Androidlib.MyRunner {
529532
private static bool Is64Bits = IntPtr.Size >= 8;
530533
531534
public static bool Is64 () {
532535
return Is64Bits;
533536
}
537+
538+
public override void Run () => Console.WriteLine (Is64Bits);
534539
}",
540+
});
541+
lib.Sources.Add (new BuildItem ("AndroidJavaSource", "MyRunner.java") {
542+
Encoding = new UTF8Encoding (encoderShouldEmitUTF8Identifier: false),
543+
TextContent = () => @"
544+
package com.example.androidlib;
545+
546+
public abstract class MyRunner {
547+
public abstract void run();
548+
}"
535549
});
536550
var proj = new XamarinAndroidApplicationProject { IsRelease = true, ProjectName = "App1" };
537551
proj.References.Add(new BuildItem.ProjectReference (Path.Combine ("..", "Lib1", "Lib1.csproj"), "Lib1"));
538552
proj.MainActivity = proj.DefaultMainActivity.Replace (
539553
"base.OnCreate (bundle);",
540554
"base.OnCreate (bundle);\n" +
541555
"if (Lib1.Library1.Is64 ()) Console.WriteLine (\"Hello World!\");");
556+
proj.SetProperty ("AndroidEnableMarshalMethods", enableMarshalMethods.ToString ());
542557

543558

544559
using var lb = CreateDllBuilder (Path.Combine (path, "Lib1"));
545560
using var b = CreateApkBuilder (Path.Combine (path, "App1"));
546561
Assert.IsTrue (lb.Build (lib), "build should have succeeded.");
547562
Assert.IsTrue (b.Build (proj), "build should have succeeded.");
563+
564+
var intermediate = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath);
565+
var dll = $"{lib.ProjectName}.dll";
566+
Assert64Bit ("android-arm", expected64: false);
567+
Assert64Bit ("android-arm64", expected64: true);
568+
Assert64Bit ("android-x86", expected64: false);
569+
Assert64Bit ("android-x64", expected64: true);
570+
571+
void Assert64Bit(string rid, bool expected64)
572+
{
573+
var assembly = AssemblyDefinition.ReadAssembly (Path.Combine (intermediate, rid, "linked", "shrunk", dll));
574+
var type = assembly.MainModule.FindType ("Lib1.Library1");
575+
Assert.NotNull (type, "Should find Lib1.Library1!");
576+
var cctor = type.GetTypeConstructor ();
577+
Assert.NotNull (type, "Should find Lib1.Library1.cctor!");
578+
Assert.AreNotEqual (0, cctor.Body.Instructions.Count);
579+
580+
/*
581+
* IL snippet
582+
* .method private hidebysig specialname rtspecialname static
583+
* void .cctor () cil managed
584+
* {
585+
* // Is64Bits = 4 >= 8;
586+
* IL_0000: ldc.i4 4
587+
* IL_0005: ldc.i4.8
588+
* ...
589+
*/
590+
var instruction = cctor.Body.Instructions [0];
591+
Assert.AreEqual (OpCodes.Ldc_I4, instruction.OpCode);
592+
if (expected64) {
593+
Assert.AreEqual (8, instruction.Operand, $"Expected 64-bit: {expected64}");
594+
} else {
595+
Assert.AreEqual (4, instruction.Operand, $"Expected 64-bit: {expected64}");
596+
}
597+
}
548598
}
549599
}
550600
}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,64 +2,64 @@
22
"Comment": null,
33
"Entries": {
44
"AndroidManifest.xml": {
5-
"Size": 3032
5+
"Size": 3036
66
},
77
"assemblies/_Microsoft.Android.Resource.Designer.dll": {
88
"Size": 1024
99
},
1010
"assemblies/Java.Interop.dll": {
11-
"Size": 58703
11+
"Size": 58990
1212
},
1313
"assemblies/Mono.Android.dll": {
14-
"Size": 86588
14+
"Size": 88074
1515
},
1616
"assemblies/Mono.Android.Runtime.dll": {
17-
"Size": 5798
17+
"Size": 5819
1818
},
1919
"assemblies/rc.bin": {
2020
"Size": 1235
2121
},
2222
"assemblies/System.Console.dll": {
23-
"Size": 6442
23+
"Size": 6448
2424
},
2525
"assemblies/System.Linq.dll": {
26-
"Size": 9123
26+
"Size": 9135
2727
},
2828
"assemblies/System.Private.CoreLib.dll": {
29-
"Size": 536436
29+
"Size": 537441
3030
},
3131
"assemblies/System.Runtime.dll": {
32-
"Size": 2623
32+
"Size": 2629
3333
},
3434
"assemblies/System.Runtime.InteropServices.dll": {
35-
"Size": 3752
35+
"Size": 3768
3636
},
3737
"assemblies/UnnamedProject.dll": {
38-
"Size": 3349
38+
"Size": 3222
3939
},
4040
"classes.dex": {
41-
"Size": 19748
41+
"Size": 377064
4242
},
4343
"lib/arm64-v8a/libmono-component-marshal-ilgen.so": {
44-
"Size": 93552
44+
"Size": 97392
4545
},
4646
"lib/arm64-v8a/libmonodroid.so": {
47-
"Size": 380832
47+
"Size": 380704
4848
},
4949
"lib/arm64-v8a/libmonosgen-2.0.so": {
50-
"Size": 3160360
50+
"Size": 3177168
5151
},
5252
"lib/arm64-v8a/libSystem.IO.Compression.Native.so": {
5353
"Size": 723560
5454
},
5555
"lib/arm64-v8a/libSystem.Native.so": {
56-
"Size": 94392
56+
"Size": 94424
5757
},
5858
"lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": {
5959
"Size": 154904
6060
},
6161
"lib/arm64-v8a/libxamarin-app.so": {
62-
"Size": 16624
62+
"Size": 11080
6363
},
6464
"META-INF/BNDLTOOL.RSA": {
6565
"Size": 1213
@@ -95,5 +95,5 @@
9595
"Size": 1904
9696
}
9797
},
98-
"PackageSize": 2685258
98+
"PackageSize": 2771274
9999
}

0 commit comments

Comments
 (0)