Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Fixup indirect resource references (#8416)
Browse files Browse the repository at this point in the history
Fixes: dotnet/maui#17265

Context: dc3ccf2
Context: https://github.com/Zack-G-I-T/SfListView.Net8Bug/tree/ecb25af3329391858d1d64c4875ca58771e2b66c

Commit dc3ccf2 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
dc3ccf2, 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 <dellis1972@googlemail.com>
  • Loading branch information
jonathanpeppers and dellis1972 authored Oct 17, 2023
1 parent 2b0761b commit 03018e0
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -152,18 +178,16 @@ 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) {
LogMessage ($"DEBUG! Failed to find {key}! Trying case insensitive lookup.");
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
53 changes: 53 additions & 0 deletions tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => @"<?xml version=""1.0"" encoding=""utf-8""?>
<resources>
<string name=""hello"">Hi!</string>
</resources>",
},
},
};
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.");
}
}
}
}

0 comments on commit 03018e0

Please sign in to comment.