From 03018e02d0e33eb37309ae83fd9c6ad549fe1ce5 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 17 Oct 2023 10:00:40 -0500 Subject: [PATCH] [Xamarin.Android.Build.Tasks] Fixup indirect resource references (#8416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/dotnet/maui/issues/17265 Context: dc3ccf28cdbe9f8c0a705400b83c11a85c81a980 Context: https://github.com/Zack-G-I-T/SfListView.Net8Bug/tree/ecb25af3329391858d1d64c4875ca58771e2b66c Commit dc3ccf28 completely reworked how Android Resources work, moving from a `$(RootNamespace).Resource` type which contained fields (which needed to be updated at runtime across numerous assemblies) to a "`_Microsoft.Android.Resource.Designer` reference assembly" which contained *methods* for each Resource id. To maintain backward compatibility, pre-.NET 8 assemblies were rewritten so that instead of accessing fields: ldsfld int32 $(RootNamespace).Resource/Layout::Toolbar they became method calls: call int32 [_Microsoft.Android.Resource.Designer]_Microsoft.Android.Resource.Designer.Resource/Layout::get_Toolbar() Unfortunately we encountered a missing corner case: the previous fixup logic only operated on assemblies that themselves contained a `$(RootNamespace).Resource` type, which in turn (generally) required that the assembly be built from a project that had `@(AndroidResource)` items. In dotnet/maui#17265 and Zack-G-I-T/SfListView.Net8Bug, we encountered an assembly that: 1. Did *not* contain a `Resource` type, and 2. Used Resource ids from other assemblies, and 3. Was a .NET 7 assembly, so it and its dependencies were all using the .NET 7 field-based Resource approach. Setup: * `Ako` and `Bko` are net7.0-android projects which contain an `@(AndroidResource)` * `RefsLibs` is a net7.0-android project which references `Ako` and `Bko`, and uses the Resource values from them. * `App` is a net8.0-android project which references `RefsLibs`. "Repro" setup: dotnet new androidlib -n Ako # Set `$(TargetFramework)`=net7.0-android # Add Ako/Resources/values/strings.xml with String resource ako_name dotnet new androidlib -n Bko # Set `$(TargetFramework)`=net7.0-android # Add Bko/Resources/values/strings.xml with String resource bko_name dotnet new androidlib -n RefsLibs # Set `$(TargetFramework)`=net7.0-android # Add ProjectReference to ..\Ako\Ako.csproj, ..\Bko\Bko.csproj # Update `RefsLibs\Class1.cs` to use Ako.Resource.String.ako_name, Bko.Resource.String.bko_name dotnet new android -n App # *Remains* `$(TargetFramework)`=net8.0-android # Add ProjectReference to ..\RefsLibs\RefsLibs.csproj The punch: dotnet build App/App.csproj -p:Configuration=Release This fails to build with .NET 8 RC1: ILLink : error IL1013: Error processing '/Users/jon/Downloads/dotnet-sdk-8.0.100-rc.1.23455.8-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.0.0-rc.1.432/targets/../PreserveLists/Mono.Android.xml'. Fatal error in IL Linker Unhandled exception. Mono.Linker.LinkerFatalErrorException: ILLink: error IL1013: Error processing '/Users/jon/Downloads/dotnet-sdk-8.0.100-rc.1.23455.8-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.0.0-rc.1.432/targets/../PreserveLists/Mono.Android.xml'. ---> System.ArgumentNullException: Value cannot be null. (Parameter 'key') at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value) … …/build/Microsoft.NET.ILLink.targets(87,5): error NETSDK1144: Optimizing assemblies for size failed. Optimization can be disabled by setting the PublishTrimmed property to false. "Interestingly", it *succeeds* with .NET 8 RC2 (no build errors). Regardless, with both .NET 8 RC1 and RC2, the app is *broken*: % dotnet tool install --global dotnet-ilverify % $HOME/.dotnet/tools/ilverify App/obj/Release/net8.0-android/android-arm/linked/RefsLibs.dll \ --tokens --system-module System.Private.CoreLib \ -r 'App/obj/Release/net8.0-android/android-arm/linked/*.dll' [IL]: Error [ClassLoadGeneral]: […/App/obj/Release/net8.0-android/android-arm/linked/RefsLibs.dll : RefsLibs.Class1::.ctor()] Failed to load type 'String' from assembly 'Ako, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' [IL]: Error [CallCtor]: […/App/obj/Release/net8.0-android/android-arm/linked/RefsLibs.dll : RefsLibs.Resource::.ctor()][offset 0x00000001] call to .ctor only allowed to initialize this pointer from within a .ctor. Try newobj. [IL]: Error [ThisUninitReturn]: […/App/obj/Release/net8.0-android/android-arm/linked/RefsLibs.dll : RefsLibs.Resource::.ctor()][offset 0x00000006] Return from .ctor when this is uninitialized. 3 Error(s) Verifying …/App/obj/Release/net8.0-android/android-arm/linked/RefsLibs.dll If you disassemble `RefsLibs.dll`, you find that it's using `ldsfld`, not `call`: % ikdasm App/obj/Release/net8.0-android/android-arm/linked/RefsLibs.dll … .method public hidebysig specialname rtspecialname instance void .ctor() cil managed { // Code size 29 (0x1d) .maxstack 8 IL_0000: ldarg.0 IL_0001: ldsfld int32 [Ako]Ako.Resource/String::ako_name IL_0006: stfld int32 RefsLibs.Class1::a IL_000b: ldarg.0 IL_000c: ldsfld int32 [Bko]Bko.Resource/String::bko_name IL_0011: stfld int32 RefsLibs.Class1::b IL_0016: ldarg.0 IL_0017: call instance void [System.Private.CoreLib]System.Object::.ctor() IL_001c: ret } // end of method Class1::.ctor If you attempt to run the .NET 8 RC2 build, it fails at runtime: I MonoDroid: android.runtime.JavaProxyThrowable: [System.TypeLoadException]: Arg_TypeLoadException I MonoDroid: at App.MainActivity.OnCreate(Unknown Source:0) I MonoDroid: at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(Unknown Source:0) The problem is that `RefsLibs.dll` isn't being fixed up as part of dc3ccf28, because it doesn't contain any `@(AndroidResource)` values or an `[assembly: ResourceDesigner]` custom attribute. `RefsLibs.dll` is "free floating", with nothing to indicate that it needs to be updated. Fix this scenario by updating `LinkDesignerBase` to "sanity check" all "Resource-like" member references which cannot be resolved. Consider: % monodis --memberref RefsLibs/bin/Release/net7.0-android/RefsLibs.dll … 18: TypeRef[23] ako_name Resolved: [Ako]Ako.Resource/String.ako_name Signature: int32 19: TypeRef[25] bko_name Resolved: [Bko]Bko.Resource/String.bko_name Signature: int32 These are field references. In the context of .NET 8/dc3ccf28, these fields *will not exist*; they cannot be resolved. `LinkDesignerBase` will check the member references table of *all* assemblies included in the app, and if any member references contain a declaring type which contains `.Resource/` *and* that member reference cannot be resolved, we will assume that it is an `@(AndroidReference)` and replace it with a `call` to the appropriate method. With the fix in place, `ilverify` no longer reports `Error [ClassLoadGeneral]`, and `ikdasm` shows: % ikdasm App/obj/Release/net8.0-android/android-arm/linked/RefsLibs.dll … .method public hidebysig specialname rtspecialname instance void .ctor() cil managed { // Code size 29 (0x1d) .maxstack 8 IL_0000: ldarg.0 IL_0001: call int32 [_Microsoft.Android.Resource.Designer]_Microsoft.Android.Resource.Designer.Resource/String::get_ako_name() IL_0006: stfld int32 RefsLibs.Class1::a IL_000b: ldarg.0 IL_000c: call int32 [_Microsoft.Android.Resource.Designer]_Microsoft.Android.Resource.Designer.Resource/String::get_bko_name() IL_0011: stfld int32 RefsLibs.Class1::b IL_0016: ldarg.0 IL_0017: call instance void [System.Private.CoreLib]System.Object::.ctor() IL_001c: ret } // end of method Class1::.ctor Note that the .NET 7 `ldsfld` has been replaced with `call`. Co-authored-by: Dean Ellis --- .../FixLegacyResourceDesignerStep.cs | 34 ++++++++++-- .../MonoDroid.Tuner/LinkDesignerBase.cs | 31 +++++++++++ .../Tests/InstallAndRunTests.cs | 53 +++++++++++++++++++ 3 files changed, 113 insertions(+), 5 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixLegacyResourceDesignerStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixLegacyResourceDesignerStep.cs index ae4473d6cd1..e07ec5e9751 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixLegacyResourceDesignerStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixLegacyResourceDesignerStep.cs @@ -139,6 +139,32 @@ string GetNativeTypeNameFromManagedTypeName (string name) } } + string GetFixupKey (Instruction instruction, string designerFullName) + { + string line = instruction.ToString (); + int idx = line.IndexOf (designerFullName, StringComparison.Ordinal); + if (idx >= 0) { + return line.Substring (idx + designerFullName.Length); + } + if (instruction.Operand is FieldReference fieldRef && + (fieldRef.DeclaringType?.ToString()?.Contains (".Resource/") ?? false)) { + var canResolve = false; + try { + var resolved = fieldRef.Resolve (); + canResolve = resolved != null; + } catch (Exception) { + } + if (canResolve) + return null; + var type = fieldRef.DeclaringType.FullName; + var s = type.LastIndexOf ('/'); + type = type.Substring (s + 1); + var key = type + "::" + fieldRef.Name; + return key; + } + return null; + } + protected override void FixBody (MethodBody body, TypeDefinition designer) { // replace @@ -152,10 +178,8 @@ protected override void FixBody (MethodBody body, TypeDefinition designer) { if (i.OpCode != OpCodes.Ldsfld) continue; - string line = i.ToString (); - int idx = line.IndexOf (designerFullName, StringComparison.Ordinal); - if (idx >= 0) { - string key = line.Substring (idx + designerFullName.Length); + var key = GetFixupKey (i, designerFullName); + if (key != null) { LogMessage ($"Looking for {key}."); var found = lookup.TryGetValue (key, out MethodDefinition method); if (!found) { @@ -163,7 +187,7 @@ protected override void FixBody (MethodBody body, TypeDefinition designer) found = lookupCaseInsensitive.TryGetValue (key, out method); } if (found) { - var importedMethod = designer.Module.ImportReference (method); + var importedMethod = body.Method.Module.ImportReference (method); var newIn = Instruction.Create (OpCodes.Call, importedMethod); instructions.Add (i, newIn); } else { diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs index d8c65f1b573..d999506b286 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs @@ -60,6 +60,37 @@ protected bool FindResourceDesigner (AssemblyDefinition assembly, bool mainAppli } } + + if (string.IsNullOrEmpty(designerFullName)) { + LogMessage ($"Inspecting member references for assembly: {assembly.FullName};"); + var memberRefs = assembly.MainModule.GetMemberReferences (); + foreach (var memberRef in memberRefs) { + string declaringType = memberRef.DeclaringType?.ToString () ?? string.Empty; + if (!declaringType.Contains (".Resource/")) { + continue; + } + if (declaringType.Contains ("_Microsoft.Android.Resource.Designer")) { + continue; + } + var resolved = false; + try { + var def = memberRef.Resolve (); + if (resolved = def != null) { + LogMessage ($"Resolved member `{memberRef?.Name}`"); + } + } catch (Exception ex) { + LogMessage ($"Exception resolving member `{memberRef?.Name}`: {ex}"); + resolved = false; + } + if (!resolved) { + LogMessage ($"Adding _Linker.Generated.Resource to {assembly.Name.Name}. Could not resolve {memberRef?.Name} : {declaringType}"); + designer = new TypeDefinition ("_Linker.Generated", "Resource", TypeAttributes.Public | TypeAttributes.AnsiClass); + designer.BaseType = new TypeDefinition ("System", "Object", TypeAttributes.Public | TypeAttributes.AnsiClass); + return true; + } + } + } + if (string.IsNullOrEmpty(designerFullName)) return false; diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index a6e903cc6dd..b6553ecdef6 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -1115,5 +1115,58 @@ public void EnableAndroidStripILAfterAOT ([Values (false, true)] bool profiledAO Assert.IsTrue(didLaunch, "Activity should have started."); } + [Test] + public void FixLegacyResourceDesignerStep ([Values (true, false)] bool isRelease) + { + string previousTargetFramework = "net7.0-android"; + + var library1 = new XamarinAndroidLibraryProject { + IsRelease = isRelease, + TargetFramework = previousTargetFramework, + ProjectName = "Library1", + AndroidResources = { + new AndroidItem.AndroidResource (() => "Resources\\values\\strings2.xml") { + TextContent = () => @" + + Hi! +", + }, + }, + }; + var library2 = new XamarinAndroidLibraryProject { + IsRelease = isRelease, + TargetFramework = previousTargetFramework, + ProjectName = "Library2", + OtherBuildItems = { + new BuildItem.Source("Foo.cs") { + TextContent = () => "public class Foo { public static int Hello => Library1.Resource.String.hello; } ", + } + } + }; + library2.AndroidResources.Clear (); + library2.SetProperty ("AndroidGenerateResourceDesigner", "false"); // Disable Android Resource Designer generation + library2.AddReference (library1); + proj = new XamarinAndroidApplicationProject { + IsRelease = isRelease, + ProjectName = "MyApp", + }; + proj.AddReference (library2); + proj.MainActivity = proj.DefaultMainActivity.Replace ("//${AFTER_ONCREATE}", "Console.WriteLine(Foo.Hello);"); + + using (var library1Builder = CreateDllBuilder (Path.Combine ("temp", TestName, library1.ProjectName))) + using (var library2Builder = CreateDllBuilder (Path.Combine ("temp", TestName, library2.ProjectName))) { + builder = CreateApkBuilder (Path.Combine ("temp", TestName, proj.ProjectName)); + Assert.IsTrue (library1Builder.Build (library1, doNotCleanupOnUpdate: true), $"Build of {library1.ProjectName} should have succeeded."); + Assert.IsTrue (library2Builder.Build (library2, doNotCleanupOnUpdate: true), $"Build of {library2.ProjectName} should have succeeded."); + Assert.IsTrue (builder.Build (proj), $"Build of {proj.ProjectName} should have succeeded."); + + RunProjectAndAssert (proj, builder); + + WaitForPermissionActivity (Path.Combine (Root, builder.ProjectDirectory, "permission-logcat.log")); + bool didLaunch = WaitForActivityToStart (proj.PackageName, "MainActivity", + Path.Combine (Root, builder.ProjectDirectory, "logcat.log"), 30); + Assert.IsTrue (didLaunch, "Activity should have started."); + } + } } }