[android] .NET 9 p6, trimmer step no longer saving changes to disk

[jonathanpeppers]

Description

We noticed this happening on both of these PRs:

• [main] Update dependencies from dotnet/sdk android#9031
• [release/9.0.1xx-preview6] Update dependencies from dotnet/sdk android#9051

We have a test failure:

• https://github.com/dotnet/android/blob/e9ee3d4cc35edd5986629d2d5a5415ff93dbe0d5/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs#L183-L207

Xamarin.Android.JcwGen_Tests, Xamarin.Android.JcwGenTests.BindingTests.JavaAbstractMethodTest / Release
Error message
System.TypeLoadException : VTable setup of type Library.MyClrCursor failed
Stack trace
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

This test is verifying that this trimmer step works:

• https://github.com/dotnet/android/blob/e9ee3d4cc35edd5986629d2d5a5415ff93dbe0d5/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets#L60
• https://github.com/dotnet/android/blob/e9ee3d4cc35edd5986629d2d5a5415ff93dbe0d5/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs#L305

What the trimmer step does, is attempt to "fill in" new abstract members of new Android APIs. This would allow existing net8.0-android NuGet packages to work in .NET 9, when Android adds new abstract members. (unfortunately Google does!). The general idea, is we'd prevent an exception unless the new member was actually called.

It seems like our trimmer step is doing its work:

ILLink: Added method: System.Void Test.Bindings.ICursor::NewMethod() to type: Library.MyClrCursor scope: Xamarin.Android.FixJavaAbstractMethod-Library.dll
ILLink: Added method: System.Int32 Test.Bindings.ICursor::NewMethodWithParams(System.Int32,System.String) to type: Library.MyClrCursor scope: Xamarin.Android.FixJavaAbstractMethod-Library.dll
ILLink: Added method: System.Int32 Test.Bindings.ICursor::NewMethodWithParams(System.Int32,System.String,System.Single) to type: Library.MyClrCursor scope: Xamarin.Android.FixJavaAbstractMethod-Library.dll
ILLink: Added method: System.Int32 Test.Bindings.ICursor::NewMethodWithRT() to type: Library.MyClrCursor scope: Xamarin.Android.FixJavaAbstractMethod-Library.dll

But we don't see the changes in illink's output.

Reproduction Steps

• Build .NET for Android
• Run test project

.\dotnet-local.cmd build tests\CodeGen-Binding\Xamarin.Android.JcwGen-Tests\Xamarin.Android.JcwGen-Tests.csproj -c Release -t:Install,RunTestApks --p:_ExtraTrimmerArgs=--verbose

Expected behavior

JavaAbstractMethodTest passes

Actual behavior

JavaAbstractMethodTest fails with TypeLoadException

Regression?

Yes, this worked in .NET 9 Preview 5

Known Workarounds

None

Configuration

Runtime: 9.0.0-preview.6.24321.8 or newer

Other information

.binlog: illink-verbose.zip

Output assemblies: linked.zip

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

[agocke]

@sbomer

[vitek-karas]

On debug build this asserts in the trimmer, but it looks unrelated to the problem:

runtime/src/tools/illink/src/linker/Linker/Annotations.cs

Line 307 in 203f993

Debug.Assert (preserve != TypePreserve.Nothing);

The call comes from:
https://github.com/dotnet/android/blob/06bb1dc6a292ef5618a3bb6ecca3ca869253ff2e/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs#L346

This assert has been there for 3 years now - added in dotnet/linker#1768
And the caller was updated to match the new APIs in dotnet/android#5748 - apparently not correctly :-)

So this is probably a long standing bug which doesn't seem to matter much - I didn't try to figure out what the assert guards against and thus what is the problem (been a while).

@sbomer - probably worth filing a bug/PR in dotnet/android for this.

[vitek-karas]

(This looks to be a bit of a bug farm :-))
For context - The above asserts is happening around type Java.Lang.AbstractMethodError
Ignoring the above assert leads to another assert, this time in:

runtime/src/tools/illink/src/linker/Linker/Annotations.cs

Line 511 in a900bbf

Debug.Assert (GetPreservedMethods (definition) == null);

The method in question is Java.Lang.ICharSequenceInvoker..ctor - so it's very likely unrelated to the previous assert.
The caller is (a bit indirectly):
https://github.com/dotnet/android/blob/06bb1dc6a292ef5618a3bb6ecca3ca869253ff2e/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs#L189

and then

https://github.com/dotnet/android/blob/06bb1dc6a292ef5618a3bb6ecca3ca869253ff2e/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs#L147

I vaguely remember something around the AddPreserveMethod and the complexity behind it. This feels like an internal illink problem which is triggered by the custom step. So either unexpected call sequence from the custom step or some other problem.

Another very likely unrelated bug to the original issue here.

This second assert is hit many times over on various other methods, in all cases the class name seems to include the word Invoker - but I don't know if that makes it special somehow.