Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Runtime errors with IL stripped assemblies #7088

Closed
jxbrenner opened this issue Jun 11, 2022 · 19 comments · Fixed by #7263
Closed

Regression: Runtime errors with IL stripped assemblies #7088

jxbrenner opened this issue Jun 11, 2022 · 19 comments · Fixed by #7263
Assignees
Labels
Area: App+Library Build Issues when building Library projects or Application projects. need-attention A xamarin-android contributor needs to review

Comments

@jxbrenner
Copy link

jxbrenner commented Jun 11, 2022

Android application type

Classic Xamarin.Android (MonoAndroid12.0, etc.)

Affected platform version

Xamarin.Android 12.3

Description

Mono runtime error, app hangs and crashes. See device log output below.

Sample project attached for reference. MSBuild configured to match what I'm using in VS 2019 for app distribution.
XamarinAndroidApp.zip

Steps to Reproduce

  1. New Android App (Xamarin) - Blank App
  2. Configure MSBuild for static compilation, IL stripping, and target arm64-v8a
  3. Build and run

Did you find any workaround?

Build with Xamarin.Android 12.0.

Relevant log output

Time	Device Name	Type	PID	Tag	Message
	Sonimtech XP8800	Info	1785	ActivityManager	Process com.companyname.xamarinandroidapp (pid 14562) has died: fore TOP 
	Sonimtech XP8800	Error	14562	mono-rt	
  at Java.Interop.JniRuntime.GetRegisteredRuntime (System.IntPtr invocationPointer) <0x720f10ff0c + 0x0007c> in <4be0212dda1448dbbb5bcb78bcd13cf4>:0 
  at Java.Interop.JniEnvironmentInfo.set_EnvironmentPointer (System.IntPtr value) <0x720f12534c + 0x0008b> in <4be0212dda1448dbbb5bcb78bcd13cf4>:0 
  at Java.Interop.JniRuntime..ctor (Java.Interop.JniRuntime+CreationOptions options) <0x720f110234 + 0x005b7> in <4be0212dda1448dbbb5bcb78bcd13cf4>:0 
  at Android.Runtime.AndroidRuntime..ctor (System.IntPtr jnienv, System.IntPtr vm, System.Boolean allocNewObjectSupported, System.IntPtr classLoader, System.IntPtr classLoader_loadClass, System.Boolean jniAddNativeMethodRegistrationAttributePresent) <0x720f4c15b0 + 0x0008b> in <97754039425649c180140cd9408a4bd2>:0 
  at Android.Runtime.JNIEnv.Initialize (Android.Runtime.JnienvInitializeArgs* args) [0x000f3] in <97754039425649c180140cd9408a4bd2>:0 
	Sonimtech XP8800	Error	14562	mono-rt	
	Sonimtech XP8800	Error	14562	mono-rt	[ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidProgramException: Invalid IL code in System.Collections.Generic.Dictionary`2<intptr, Java.Interop.JniRuntime>:TryGetValue (intptr,Java.Interop.JniRuntime&): IL_0000: ret       
@jxbrenner jxbrenner added Area: App+Library Build Issues when building Library projects or Application projects. needs-triage Issues that need to be assigned. labels Jun 11, 2022
@jonathanpeppers
Copy link
Member

This setting is not supported in .NET 6, does it help to remove it?

<BundleAssemblies>True</BundleAssemblies>

Likewise, we don't have support for this "experimental" setting either:

<AndroidAotMode>Hybrid</AndroidAotMode>

Which is what would use cil-strip. Can you give some more info on why you'd like to use it? Thanks!

@jonathanpeppers jonathanpeppers added need-info Issues that need more information from the author. and removed needs-triage Issues that need to be assigned. labels Jun 13, 2022
@ghost
Copy link

ghost commented Jun 13, 2022

Hi @jxbrenner. We have added the "need-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@jxbrenner
Copy link
Author

jxbrenner commented Jun 13, 2022

Hi @jonathanpeppers, thanks for the response! I opened the issue for a regression I've observed when building with VS 2022 against Xamarin TFMs, specifically MonoAndroid12.0. I would very much like to see IL stripping work with .net6.0-android as well, as I can't migrate to a unified framework and MAUI until it does.

Regarding assembly bundling, no removing that from the build process doesn't help.

Regarding the whole "experimental hybrid setting", I'd like to focus more on the fact that I can't execute the Mono CIL Stripper tool on my statically compiled assemblies anymore, and less on what MSBuild properties are experimental. I appreciate that the example I gave included that property as a means to strip the IL as that's how it's triggered from the project file; however, regardless of how I run the Mono CIL Stripper against my assemblies, I still get the same fatal runtime error about invalid IL code when building with VS 2022. This is not the case with VS 2019.

I personally don't care what IL remains in any of the assemblies except for my shared and head project. Unfortunately, I can't even IL strip these assemblies when building with VS 2022 (same fatal runtime error). So to put a finer point on the issue here, I just want to be able to continue to execute the Mono CIL Stripper against my shared and head project assemblies when building against MonoAndroid. However, a fix that applies to .net-android would be even better since this gives my app a path forward to MAUI.

Regarding motivation for IL stripping, see dotnet/runtime#44855 and #1218. TLDR on that is reducing assemblies to metadata protects IP and improves security of closed-source projects. Decompiling obfuscated IL back to C# is trivial.

@ghost ghost added need-attention A xamarin-android contributor needs to review and removed need-info Issues that need more information from the author. labels Jun 13, 2022
@jonathanpeppers
Copy link
Member

@jxbrenner cil-strip is located here:

https://github.com/mono/mono/tree/main/mcs/tools/cil-strip

Should you file an issue there instead?

@jxbrenner
Copy link
Author

jxbrenner commented Jun 13, 2022

@jonathanpeppers hey what gives, now you're making me think! I don't think so. I spent some time comparing the output of stripped assemblies produced when I build the example project with VS 2019 and VS 2022, and so far I believe cil-strip is still producing the same output for the given input (go figure, it's version 0.6.9.0 in both versions of VS). Definitely the case with XamarinAndroidApp and System.Collections.Generic.Dictionary (what I'm getting the runtime error on in the example).

While I was doing that I found that the packages built with VS 2022 keep all the managed assemblies in assemblies/assemblies.blob instead of directly in the assemblies folder (or bundled). That's something different from VS 2019, I'm wondering if there's an easy fix here. Is there a build property I can set to disable the assembly store task?

@jonathanpeppers
Copy link
Member

Yes, you might try this for your Release configuration:

<AndroidUseAssemblyStore>false</AndroidUseAssemblyStore>

And if that solves the issue, there might be a problem with this combination of settings. Let us know, thanks!

@jxbrenner
Copy link
Author

jxbrenner commented Jun 14, 2022

Unfortunately if there's an easy fix, that's not it, still the same runtime error.

To help you help me I've attached a diff report of the build properties from the MSBuild logs of the example project (VS 2019 on the left, VS 2022 on the right). Does anything jump off the page at you? How about the runtime error? Invalid IL code in the BCL, I mean there shouldn't be any since we stripped it, and why would we need it since it's all statically compiled? As mentioned, mscorlib output from cil-strip appears exactly the same, ex. all methods in the method table have their bodies stripped and are patched NoInlining.

I'm wondering about the differences in the AndroidAotEnableLazyLoad, AndroidAotStripLibraries properties. They're both false in the VS2019 build, but I can't seem to override them in VS2022 with the project file.

BuildPropDiffs.zip

@dellis1972
Copy link
Contributor

dellis1972 commented Jun 14, 2022

@jxbrenner can you use the monodis tool (if you have access to mono) to get the contents on the stripped assembly from both VS2019 output and VS2022 output. Then diff that output to see what changed.

@jxbrenner jxbrenner changed the title Regression: cil-strip crashes on startup Regression: Runtime errors with IL stripped assemblies Jun 14, 2022
@jxbrenner
Copy link
Author

jxbrenner commented Jun 14, 2022

@dellis1972 I was doing a pretty superficial comparison just looking at some of the tables and method signatures using dnSpy, but I installed mono, dumped the CIL images, and diffed to see what changed. I focused on mscorlib since the runtime error is complaining about invalid IL in the Dictionary type.

For a given input (mscorlib v2.0.5.0), there are definitely differences in the build output of VS 2019 and VS 2022 despite passing the same build parameters. However, these differences are not a function of cil-strip. I compared both the stripped and non-stripped IL and any differences between the two assemblies is unrelated to stripping. I didn't see any obvious differences (to me) between System.Collections.Generic.Dictionary, but I did see quite a few overloads present in the 2019-built assembly that were not present in the 2022-built assembly (ex. System.Action, ILGenerator.Emit).

All my Android development has been at a higher level of abstraction, so I'm a bit out of my depth here. My analysis is basically that these things are different, and shouldn't they be the same? Beyond that I still wonder why my build log shows AndroidAotEnableLazyLoad and AndroidAotStripLibraries as true when I set them false in the project file, and wonder if there is correlation here. Could the symbol stripping and/or lazy loading cause this runtime error on projects where the IL has been stripped? I assume the missing overloads in mscorlib is linker related. Would not being able to override these properties be issues in themselves?

Any thoughts guys? I've attached an archive with the assemblies, IL dumps, and WinMerge project files. I'm willing to try whatever build properties you can think of.

mscorlib-il-diff.zip

@dellis1972
Copy link
Contributor

Regarding AndroidAotStripLibraries the actual name of that property is _ AndroidAotStripLibraries as its an "internal" property. So be sure you include the underscrore when setting it in your project.

AndroidAotEnableLazyLoad is not an internal property so that one should work...

can you post a snippet from your csproj where you are setting these properties?

@grendello we might need some runtime assistance on this issue. I'm not sure how to go about getting the required diagnsotic data from the runtime to debug this issue.

@grendello
Copy link
Contributor

@dellis1972 @jxbrenner I'd try this, even though it appears to be a JIT problem:

$ adb logcat -G 16M
$ adb shell setprop debug.mono.log "default,mono_log_level=debug,mono_log_mask=asm:type"
$ adb logcat -c
# Run the app here
$ adb logcat -d > log.txt

If it indeed is a JIT problem, we might need more extensive logging.

@jxbrenner would you be able to generate a crash log using the above commands?

I wonder if we get a native crash anywhere there.

@jxbrenner
Copy link
Author

jxbrenner commented Jun 15, 2022

@dellis1972, thanks for the heads up, silly mistake. Here's my project file (appended .txt so I could upload):
XamarinAndroidApp.csproj.txt

@grendello here's the device log:
log.txt

I spent some time trying different versions of VS 2022 and found that I'm able to execute both the blank template app and my app with these build settings when building with VS 17.1.7 (Xamarin.Android 12.2.4). I get this runtime error with VS 17.2.0 and up (>= Xamarin.Android 12.3.0).

The values of_AndroidAotStripLibraries and AndroidAotEnableLazyLoad appear to have no bearing on the issue, since I can build with XA 12.2.4 with these properties either true or false. Also, I still get the runtime error when setting either property true or false when building with XA 12.3.0.

It seems pretty clear Xamarin.Android 12.3 breaks IL stripping. It doesn't matter if I manipulate the targets file to get the CilStrip task to run on just the shared and/or head project assembly, any IL stripping results in an InvalidProgramException for Invalid IL code.

@gabriel-kozma
Copy link

This issue interests me as well, I use the hybrid aot and have noticed the same thing

@grendello
Copy link
Contributor

grendello commented Jun 20, 2022

@lambdageek would you mind looking at this issue? It seems like mono/mono JIT generating somehow invalid code for generics? Thanks! It happens the very first time we call managed code during startup (we call this method from this code)

@gabriel-kozma
Copy link

any updates on this?

@jxbrenner
Copy link
Author

@grendello @lambdageek
Any ideas yet as to why the mono JIT is breaking with hybrid AoT in Xamarin.Android 12.3? Isn't the JIT successful in compiling the same code when there is no AoT enabled?

@lambdageek lambdageek self-assigned this Aug 15, 2022
@lambdageek
Copy link
Member

Hi folks @jxbrenner , @gabriel-kozma, add this to your .csproj:

    <AndroidAotAdditionalArguments>hybrid</AndroidAotAdditionalArguments>

@grendello @jonathanpeppers @dellis1972 as far as I can tell, setting <AndroidAotMode>Hybrid</AndroidAotMode> doesn't actually pass --aot=...,hybrid,... to the mono AOT compiler.

for future reference: I used the following to get extra logging during aot compilation:

    <AndroidAotAdditionalArguments>log-generics,hybrid</AndroidAotAdditionalArguments>
    <AndroidExtraAotOptions>-v</AndroidExtraAotOptions>

which dumps information about added generic instances and also each method that gets compiled.

@lambdageek lambdageek removed their assignment Aug 15, 2022
@jxbrenner
Copy link
Author

@lambdageek Thank you! I passed that build property and was able to execute my app built with VS 17.3.0 (Xamarin.Android 13.0). I'll just keep that one in my project file from now on and keep and eye on the device log for the Mono AOT mode setting going forward.

With regards to closing the issue, I suppose the regression lives somewhere in the changes to the build tasks in Xamarin.Android 12.3. <AndroidAotMode>Hybrid</AndroidAotMode> no longer sets the runtime to hybrid AoT. Is the intent then to make it so it does again, or just leave this as tribal knowledge here in this issue as this is not a documented feature?

@simon10says
Copy link

@lambdageek , thanks. I'm also able to my app working with your solution.

Can I just double confirm, to get the hybrid aot mode, we just need to add the following?

<AotAssemblies>true</AotAssemblies>
<AndroidAotAdditionalArguments>hybrid</AndroidAotAdditionalArguments>

Do I still need to set <AndroidAotMode>?

jonpryor pushed a commit that referenced this issue Aug 22, 2022
Fixes: #7088

When the AOT hybrid mode is enabled when the `$(AndroidAotMode)`
MSBuild property is set to `Hybrid`, we failed to inform the AOT
compiler about its desired mode by omitting the `hybrid` argument
from the list of options passed to the compiler via `--aot`.

The same would apply to the full AOT mode.

Add `hybrid` and `full` options to the AOT compiler command line, if
enabled via the `$(AndroidAotMode)` MSBuild property.

The mode is set after processing the `$(AndroidAotAdditionalArguments)`
MSBuild property.  The `$(AndroidAotMode)` property should always be
the definitive source of AOT compiler mode, as it specifies the
options directly supported by us.

Note that the AOT compiler doesn't appear to validate options passed
to it too rigorously, so setting multiple modes in some way, may have
weird/invalid effects.  I don't think it's our place to verify the
modes, thus I'm not adding any code to that effect.
jonathanpeppers pushed a commit that referenced this issue Aug 22, 2022
Fixes: #7088

When the AOT hybrid mode is enabled when the `$(AndroidAotMode)`
MSBuild property is set to `Hybrid`, we failed to inform the AOT
compiler about its desired mode by omitting the `hybrid` argument
from the list of options passed to the compiler via `--aot`.

The same would apply to the full AOT mode.

Add `hybrid` and `full` options to the AOT compiler command line, if
enabled via the `$(AndroidAotMode)` MSBuild property.

The mode is set after processing the `$(AndroidAotAdditionalArguments)`
MSBuild property.  The `$(AndroidAotMode)` property should always be
the definitive source of AOT compiler mode, as it specifies the
options directly supported by us.

Note that the AOT compiler doesn't appear to validate options passed
to it too rigorously, so setting multiple modes in some way, may have
weird/invalid effects.  I don't think it's our place to verify the
modes, thus I'm not adding any code to that effect.
@ghost ghost locked as resolved and limited conversation to collaborators Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App+Library Build Issues when building Library projects or Application projects. need-attention A xamarin-android contributor needs to review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants