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

NativeAOT: Consider enabling trim/AOT/single-file warnings by default #18571

Closed
Tracked by #17339
ivanpovazan opened this issue Jul 21, 2023 · 8 comments
Closed
Tracked by #17339
Labels
dotnet An issue or pull request related to .NET (6)
Milestone

Comments

@ivanpovazan
Copy link
Contributor

ivanpovazan commented Jul 21, 2023

Since NativeAOT is much more restrictive in terms of trimming and AOT compatibility of compiled applications (and used frameworks) compared to Mono, we should enable trimming, AOT and single-file warnings by default. Especially for the fact that NativeAOT does not have a safety - fallback mechanism like Mono's interpreter.

Additionally, since we will release NativeAOT on iOS as an experimental feature, enabling these warnings (and treating them as errors) would let us detect issues with our frameworks and compiled applications, which will enable us to address them early and improve the AOT experience for our customers.

@ivanpovazan ivanpovazan changed the title Enable trim/aot warnings by default NativeAOT: Consider enabling trim and aot warnings by default Jul 21, 2023
@ivanpovazan ivanpovazan changed the title NativeAOT: Consider enabling trim and aot warnings by default NativeAOT: Consider enabling trim and AOT warnings by default Jul 21, 2023
@ivanpovazan ivanpovazan added this to the .NET 8 milestone Jul 21, 2023
@ivanpovazan ivanpovazan changed the title NativeAOT: Consider enabling trim and AOT warnings by default NativeAOT: Consider enabling trim/AOT/single file warnings by default Jul 21, 2023
@ivanpovazan ivanpovazan changed the title NativeAOT: Consider enabling trim/AOT/single file warnings by default NativeAOT: Consider enabling trim/AOT/single-file warnings by default Jul 21, 2023
@MichalStrehovsky
Copy link

If we don't do anything to break this, adding <PublishAot>true</PublishAot> to the project should already enable these analyzers (we should validate).

To validate, one should be able to create a new MAUI project in VS and add:

public class Class1
{
    static Type t;

    static void Frob()
        => t.MakeGenericType(Array.Empty<Type>());
}

To the .cs somewhere. You should see IL3050 and IL2055 warnings when you build (and green squiggles in VS).

@ivanpovazan
Copy link
Contributor Author

If we don't do anything to break this, adding true to the project should already enable these analyzers (we should validate).

If I did not miss anything, there are two places/properties which affect what can be currently seen in the build output:

Building a MAUI template application with how the analysers are currently configured (from above) the build log does not include precise information:

/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/obj/Release/net8.0-ios/ios-arm64/linked/Microsoft.iOS.dll : warning IL3053: Assembly 'Microsoft.iOS' produced AOT analysis warnings. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/MauiTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/obj/Release/net8.0-ios/ios-arm64/linked/Microsoft.Maui.Controls.Xaml.dll : warning IL3053: Assembly 'Microsoft.Maui.Controls.Xaml' produced AOT analysis warnings. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/MauiTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/obj/Release/net8.0-ios/ios-arm64/linked/Microsoft.Maui.dll : warning IL3053: Assembly 'Microsoft.Maui' produced AOT analysis warnings. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/MauiTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/obj/Release/net8.0-ios/ios-arm64/linked/System.Linq.Expressions.dll : warning IL3053: Assembly 'System.Linq.Expressions' produced AOT analysis warnings. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-preview.7.23360.1/MauiTest/MauiTest.csproj]

To get the full analysis log, we would have to pass -p:SuppressTrimAnalysisWarnings=false -p:TrimmerSingleWarn=false

Another thing worth considering is enabling these properties only for the ILC pass, and not for ILLink, to avoid getting duplicate warnings from the two trimming tools, or even incorrect warnings due to ILLink-ILC compatibility and difference in trimming capabilities.

/cc: @vitek-karas

@ivanpovazan
Copy link
Contributor Author

Here are the list of warnings for two different apps built with Xamarin+NativeAOT with analyzers enabled:

Apps were built with the current state of: https://github.com/xamarin/xamarin-macios/tree/release/8.0.1xx-preview7

@MichalStrehovsky
Copy link

Would it be possible to at least address the ones in new ios?

MAUI is probably a lost cause for .NET 8 and there will be warnings to tell people they're not getting a great experience. It would be nice if we could address at least those that are in an empty app but I don't know if it's possible.

To get the full analysis log, we would have to pass -p:SuppressTrimAnalysisWarnings=false -p:TrimmerSingleWarn=false

We should disable warning suppression in the SDK. TrimmerSingleWarn is useful to turn off for us, but brings no value to customers (customer knowing a MAUI assembly is not trim safe is better than spamming them with a dozen warnings they can do nothing about because they don't have MAUI source code they can edit).

@vitek-karas
Copy link

+1 on leaving TrimmerSingleWarn=true, it's a much better experience.
+10 for trying to make the default template for ios warning free, but it might be too difficult

In general I think it would make sense to suppress all trim analysis warnings from the linker, not only due to duplicates but also due to possible differences (the tools will run with different input and settings). For correctness those from the NativeAOT should be sufficient (and more precise).

@dalexsoto dalexsoto added the dotnet An issue or pull request related to .NET (6) label Jul 30, 2023
@rolfbjarne
Copy link
Member

We have these two issues already:

@rolfbjarne
Copy link
Member

I've added an issue to fix the warnings, with a list of all the warnings: #18629

Once they're fixed, and the warnings enabled by default, we should also add tests to ensure we don't regress.

rolfbjarne pushed a commit that referenced this issue Aug 18, 2023
…18759)

This PR enables trim warnings with NativeAOT by default.

In our current ILLink+ILC setup, we first need to suppress the warnings for ILLink, after which we need to enable them for ILC. 
For this reason, setting up `SuppressTrimAnalysisWarnings` properly needs to happen very early as it will get overwritten by https://github.com/dotnet/runtime/blob/45acd380b37c9ee883070a70a2ef2cb7eca77683/src/tools/illink/src/ILLink.Tasks/build/Microsoft.NET.ILLink.targets#L54-L59 

Verified with building an iOS application that the trim warnings are displayed only during ILC trimming (after `Generating native code` message below):
```                 
../dotnet publish -c Release -r ios-arm64 -bl /p:PublishAot=true /p:PublishAotUsingRuntimePack=true
MSBuild version 17.7.0+5785ed5c2 for .NET
  Determining projects to restore...
  Restored /Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj (in 127 ms).
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/sdk/8.0.100-rc.1.23415.19/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(311,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
  Detected signing identity:
          
    Code Signing Key: "Apple Development: Ivan Povazan (53PXX466YZ)" (3CC7B9372E3BB19DEBECBA95A6AF3E0EB26C5B29)
    Provisioning Profile: "iOS Team Provisioning Profile: *" (6e598984-dd21-4632-b707-72cfa247991d)
    Bundle Id: com.companyname.TrimTest
    App Id: SGGM6D27TK.com.companyname.TrimTest
  TrimTest -> /Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/bin/Release/net8.0-ios/ios-arm64/TrimTest.dll
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/packs/Microsoft.iOS.Sdk/16.4.8777-ci.trim-warn/targets/Xamarin.Shared.Sdk.targets(1826,3): warning : The file '/Users/ivan/repos/xamarin/xamarin-macios/packages/microsoft.netcore.app.runtime.nativeaot.ios-arm64/8.0.0-rc.1.23414.4/runtimes/ios-arm64/native/libbootstrapper.o' does not specify a 'PublishFolderType' metadata, and a default value could not be calculated. The file will not be copied to the app bundle. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/packs/Microsoft.iOS.Sdk/16.4.8777-ci.trim-warn/targets/Xamarin.Shared.Sdk.targets(1826,3): warning : The file '/Users/ivan/repos/xamarin/xamarin-macios/packages/microsoft.netcore.app.runtime.nativeaot.ios-arm64/8.0.0-rc.1.23414.4/runtimes/ios-arm64/native/libbootstrapperdll.o' does not specify a 'PublishFolderType' metadata, and a default value could not be calculated. The file will not be copied to the app bundle. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/packs/Microsoft.iOS.Sdk/16.4.8777-ci.trim-warn/targets/Xamarin.Shared.Sdk.targets(536,3): warning : All assemblies must be processed by the linker when using NativeAOT. Please don't set neither the 'MtouchLink' nor the 'TrimMode' property, so that the build can default to linking all assemblies. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
  Optimizing assemblies for size may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
  Optimizing assemblies for size. This process might take a while.
  Generating native code
resource ILLink.LinkAttributes.xml in System.Private.CoreLib(335,8): warning IL2049: System.Private.CoreLib: The internal attribute name 'RemoveAttributeInstances' being used in the xml is not supported by ILLink, check the spelling and the supported internal attributes. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
resource ILLink.LinkAttributes.xml in System.Private.CoreLib(342,8): warning IL2049: System.Private.CoreLib: The internal attribute name 'RemoveAttributeInstances' being used in the xml is not supported by ILLink, check the spelling and the supported internal attributes. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
resource ILLink.LinkAttributes.xml in System.Private.CoreLib(349,8): warning IL2049: System.Private.CoreLib: The internal attribute name 'RemoveAttributeInstances' being used in the xml is not supported by ILLink, check the spelling and the supported internal attributes. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
resource ILLink.LinkAttributes.xml in System.Private.CoreLib(356,8): warning IL2049: System.Private.CoreLib: The internal attribute name 'RemoveAttributeInstances' being used in the xml is not supported by ILLink, check the spelling and the supported internal attributes. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
resource ILLink.LinkAttributes.xml in System.Private.CoreLib(363,8): warning IL2049: System.Private.CoreLib: The internal attribute name 'RemoveAttributeInstances' being used in the xml is not supported by ILLink, check the spelling and the supported internal attributes. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
resource ILLink.LinkAttributes.xml in System.Private.CoreLib(370,8): warning IL2049: System.Private.CoreLib: The internal attribute name 'RemoveAttributeInstances' being used in the xml is not supported by ILLink, check the spelling and the supported internal attributes. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/src/ObjCRuntime/Runtime.CoreCLR.cs(171): Trim analysis warning IL2026: ObjCRuntime.Runtime.ResolvingEventHandler(AssemblyLoadContext,AssemblyName): Using member 'System.Runtime.Loader.AssemblyLoadContext.LoadFromAssemblyPath(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types and members the loaded assembly depends on might be removed. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/src/ObjCRuntime/Runtime.CoreCLR.cs(267): Trim analysis warning IL2026: ObjCRuntime.Runtime.FindAssembly(IntPtr): Using member 'System.Reflection.Assembly.LoadFrom(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types and members the loaded assembly depends on might be removed. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/obj/Release/net8.0-ios/ios-arm64/linked/Microsoft.iOS.dll : warning IL3053: Assembly 'Microsoft.iOS' produced AOT analysis warnings. [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/obj/Release/net8.0-ios/ios-arm64/linked/Microsoft.iOS.dll : warning IL2104: Assembly 'Microsoft.iOS' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries [/Users/ivan/repos/xamarin/xamarin-macios/builds/downloads/dotnet-sdk-8.0.100-rc.1.23415.19/TrimTest/TrimTest.csproj]
  Created the package: bin/Release/net8.0-ios/ios-arm64/publish//TrimTest.ipa
```

---
Fixes: #18571

---------

Co-authored-by: Ivan Povazan <ivan.povazan@gmail.com>
@ivanpovazan
Copy link
Contributor Author

Fixed with 90cc165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotnet An issue or pull request related to .NET (6)
Projects
None yet
Development

No branches or pull requests

5 participants