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

Audit reflection usage in our assemblies #10405

Closed
Tracked by #44736
rolfbjarne opened this issue Jan 13, 2021 · 1 comment
Closed
Tracked by #44736

Audit reflection usage in our assemblies #10405

rolfbjarne opened this issue Jan 13, 2021 · 1 comment
Assignees
Labels
dotnet An issue or pull request related to .NET (6) enhancement The issue or pull request is an enhancement estimate-1w iOS Issues affecting iOS macOS Issues affecting macOS
Milestone

Comments

@rolfbjarne
Copy link
Member

The ILlink is now doing reflection analysis resulting in reported warnings. Context: https://github.com/mono/linker/blob/master/docs/design/reflection-flow.md

We should audit our reflection usage by checking the reported warnings and look into how to solve these. (by updating the code where needed or suppressing the warnings)

Example output:

/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(172,27): Trim analysis warning IL2080: Registrar.DynamicRegistrar.<FindMethods>d__14.MoveNext(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'System.Type.GetMethods(BindingFlags)'. The field 'System.Type Registrar.DynamicRegistrar/<FindMethods>d__14::type' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Blocks.cs(307,4): Trim analysis warning IL2075: ObjCRuntime.BlockLiteral.GetBlockForDelegate(MethodInfo,Object,UInt32,String): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicFields' in call to 'System.Type.GetField(String,BindingFlags)'. The return value of method 'ObjCRuntime.BlockLiteral.GetDelegateProxyType(MethodInfo,UInt32,MethodInfo&)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Blocks.cs(185,5): Trim analysis warning IL2075: ObjCRuntime.BlockLiteral.SetupBlock(Delegate,Delegate): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The return value of method 'ObjCRuntime.MonoPInvokeCallbackAttribute.DelegateType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Blocks.cs(105,5): Trim analysis warning IL2075: ObjCRuntime.BlockLiteral.SetupBlock(Delegate,Delegate,Boolean): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The return value of method 'ObjCRuntime.UserDelegateTypeAttribute.UserDelegateType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Class.cs(457,5): Trim analysis warning IL2026: ObjCRuntime.Class.ResolveToken(Module,UInt32): Calling 'System.Reflection.Module.ResolveMethod(Int32)' which has `RequiresUnreferencedCodeAttribute` can break functionality when trimming application code. Trimming changes metadata tokens.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Class.cs(451,5): Trim analysis warning IL2026: ObjCRuntime.Class.ResolveToken(Module,UInt32): Calling 'System.Reflection.Module.ResolveType(Int32)' which has `RequiresUnreferencedCodeAttribute` can break functionality when trimming application code. Trimming changes metadata tokens.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(198,4): Trim analysis warning IL2070: Registrar.DynamicRegistrar.CollectConstructors(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'System.Type.GetConstructors(BindingFlags)'. The parameter 'type' of method 'Registrar.DynamicRegistrar.CollectConstructors(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(203,4): Trim analysis warning IL2070: Registrar.DynamicRegistrar.CollectMethods(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'System.Type.GetMethods(BindingFlags)'. The parameter 'type' of method 'Registrar.DynamicRegistrar.CollectMethods(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(208,4): Trim analysis warning IL2070: Registrar.DynamicRegistrar.CollectProperties(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.NonPublicProperties' in call to 'System.Type.GetProperties(BindingFlags)'. The parameter 'type' of method 'Registrar.DynamicRegistrar.CollectProperties(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(214,4): Trim analysis warning IL2026: Registrar.DynamicRegistrar.CollectTypes(Assembly): Calling 'System.Reflection.Assembly.GetTypes()' which has `RequiresUnreferencedCodeAttribute` can break functionality when trimming application code. Types might be removed.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(115,23): Trim analysis warning IL2026: Registrar.DynamicRegistrar.ContainsPlatformReference(Assembly): Calling 'System.Reflection.Assembly.GetReferencedAssemblies()' which has `RequiresUnreferencedCodeAttribute` can break functionality when trimming application code. Assembly references might be removed.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(179,4): Trim analysis warning IL2070: Registrar.DynamicRegistrar.FindProperty(Type,String): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.NonPublicProperties' in call to 'System.Type.GetProperty(String,BindingFlags)'. The parameter 'type' of method 'Registrar.DynamicRegistrar.FindProperty(Type,String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(184,25): Trim analysis warning IL2026: Registrar.DynamicRegistrar.FindType(Type,String,String): Calling 'System.Reflection.Assembly.GetTypes()' which has `RequiresUnreferencedCodeAttribute` can break functionality when trimming application code. Types might be removed.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(395,4): Trim analysis warning IL2070: Registrar.DynamicRegistrar.GetFields(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.NonPublicFields' in call to 'System.Type.GetFields(BindingFlags)'. The parameter 'type' of method 'Registrar.DynamicRegistrar.GetFields(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(537,4): Trim analysis warning IL2045: Registrar.DynamicRegistrar.HasThisAttributeImpl(MethodBase): Attribute 'System.Runtime.CompilerServices.ExtensionAttribute' is being referenced in code but the linker was instructed to remove all instances of this attribute. If the attribute instances are necessary make sure to either remove the linker attribute XML portion which removes the attribute instances, or override the removal by using the linker XML descriptor to keep the attribute type (which in turn keeps all of its instances).
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(1053,30): Trim analysis warning IL2070: Registrar.DynamicRegistrar.TryMatchProperty(Type,PropertyInfo): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.NonPublicProperties' in call to 'System.Type.GetProperties(BindingFlags)'. The parameter 'type' of method 'Registrar.DynamicRegistrar.TryMatchProperty(Type,PropertyInfo)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/DynamicRegistrar.cs(497,5): Trim analysis warning IL2055: Registrar.DynamicRegistrar.VerifyIsConstrainedToNSObject(Type,Type&): Call to 'System.Type.MakeGenericType(Type[])' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic type.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(529,23): Trim analysis warning IL2026: ObjCRuntime.Runtime.CollectReferencedAssemblies(List<Assembly>,Assembly): Calling 'System.Reflection.Assembly.GetReferencedAssemblies()' which has `RequiresUnreferencedCodeAttribute` can break functionality when trimming application code. Assembly references might be removed.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(1711,23): Trim analysis warning IL2070: ObjCRuntime.Runtime.FindClosedMethod(Type,MethodBase): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'System.Type.GetMethods(BindingFlags)'. The parameter 'closed_type' of method 'ObjCRuntime.Runtime.FindClosedMethod(Type,MethodBase)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(1031,23): Trim analysis warning IL2075: ObjCRuntime.Runtime.FindPropertyInfo(MethodInfo): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperties()'. The return value of method 'System.Reflection.MemberInfo.DeclaringType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(776,6): Trim analysis warning IL2075: ObjCRuntime.Runtime.GetBlockProxyAttributeMethod(MethodInfo,Int32): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The return value of method 'ObjCRuntime.BlockProxyAttribute.Type.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(882,5): Trim analysis warning IL2026: ObjCRuntime.Runtime.GetBlockWrapperCreator(MethodInfo,Int32): Calling 'System.Reflection.Assembly.GetType(String,Boolean)' which has `RequiresUnreferencedCodeAttribute` can break functionality when trimming application code. Types might be removed.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(891,6): Trim analysis warning IL2075: ObjCRuntime.Runtime.GetBlockWrapperCreator(MethodInfo,Int32): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String,BindingFlags,Binder,Type[],ParameterModifier[])'. The return value of method 'System.Reflection.Assembly.GetType(String,Boolean)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(873,7): Trim analysis warning IL2065: ObjCRuntime.Runtime.GetBlockWrapperCreator(MethodInfo,Int32): Value passed to implicit 'this' parameter of method 'System.Type.GetMethod(String)' can not be statically determined and may not meet 'DynamicallyAccessedMembersAttribute' requirements.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(1165,4): Trim analysis warning IL2070: ObjCRuntime.Runtime.GetIntPtr_BoolConstructor(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'System.Type.GetConstructors(BindingFlags)'. The parameter 'type' of method 'ObjCRuntime.Runtime.GetIntPtr_BoolConstructor(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Runtime.cs(1147,4): Trim analysis warning IL2070: ObjCRuntime.Runtime.GetIntPtrConstructor(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'System.Type.GetConstructors(BindingFlags)'. The parameter 'type' of method 'ObjCRuntime.Runtime.GetIntPtrConstructor(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Stret.cs(192,27): Trim analysis warning IL2070: ObjCRuntime.Stret.GetValueTypeSize(Type,List<Type>,Boolean,Object): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.NonPublicFields' in call to 'System.Type.GetFields(BindingFlags)'. The parameter 'type' of method 'ObjCRuntime.Stret.GetValueTypeSize(Type,List<Type>,Boolean,Object)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/Users/rolf/work/maccore/onedotnet/xamarin-macios/src/ObjCRuntime/Stret.cs(283,26): Trim analysis warning IL2070: ObjCRuntime.Stret.GetValueTypeSize(Type,Type,List<Type>,Boolean,Int32&,Int32&,Object): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.NonPublicFields' in call to 'System.Type.GetFields(BindingFlags)'. The parameter 'type' of method 'ObjCRuntime.Stret.GetValueTypeSize(Type,Type,List<Type>,Boolean,Int32&,Int32&,Object)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Note: warnings are only reported when SuppressTrimAnalysisWarnings=false

@rolfbjarne rolfbjarne added enhancement The issue or pull request is an enhancement macOS Issues affecting macOS iOS Issues affecting iOS dotnet An issue or pull request related to .NET (6) labels Jan 13, 2021
@rolfbjarne rolfbjarne added this to the .NET 6 milestone Jan 13, 2021
@rolfbjarne rolfbjarne changed the title [.NET 6] Audit reflection usage [.NET 6] Audit reflection usage in our assemblies Jan 13, 2021
@rolfbjarne rolfbjarne added dotnet-pri1 .NET 6: important for stable release estimate-1w labels Feb 3, 2021
@rolfbjarne rolfbjarne modified the milestones: .NET 6, .NET 7 Jan 25, 2022
@rolfbjarne rolfbjarne removed the dotnet-pri1 .NET 6: important for stable release label Jan 25, 2022
@rolfbjarne rolfbjarne modified the milestones: .NET 7, .NET 8 Aug 26, 2022
@rolfbjarne rolfbjarne modified the milestones: .NET 8, .NET 9 Sep 12, 2023
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Feb 16, 2024
Add test to keep track of trimmer warnings and to make sure we don't accidentally
introduce new ones.

Ref: xamarin#10405
rolfbjarne added a commit that referenced this issue Feb 16, 2024
Add test to keep track of trimmer warnings and to make sure we don't accidentally
introduce new ones.

Ref: #10405
@rolfbjarne rolfbjarne changed the title [.NET 6] Audit reflection usage in our assemblies Audit reflection usage in our assemblies Feb 20, 2024
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Mar 18, 2024
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Mar 19, 2024
rolfbjarne added a commit that referenced this issue Mar 20, 2024
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Mar 20, 2024
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Mar 21, 2024
rolfbjarne added a commit that referenced this issue Mar 21, 2024
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Mar 21, 2024
…entation a bit.

* Extract code the trimmer wants to warn about into a separate method.
* Suppress the trimmer warning we get.

Contributes towards xamarin#10405.
rolfbjarne added a commit that referenced this issue Mar 22, 2024
…entation a bit. (#20353)

* Extract code the trimmer wants to warn about into a separate method.
* Suppress the trimmer warning we get.

Contributes towards #10405.
rolfbjarne added a commit that referenced this issue Apr 22, 2024
As we have solved all trimming warnings in the our workloads, we can now go
all in on trimming.

Early in .NET 6 we hid many trimming warnings as we did not yet plan to solve
them:

    <SuppressTrimAnalysisWarnings Condition=" '$(SuppressTrimAnalysisWarnings)' == '' ">true</SuppressTrimAnalysisWarnings>

Ref: #10405

These warnings were not *actionable* at the time for customers, as
many warnings were from our code.

Going forward, let's stop suppressing these warnings if `TrimMode=full`.

We can also enable trimming for new projects:

* `dotnet new ios|tvos|macos|maccatalyst`

These will have the `TrimMode` property set to `Full` by default for `Release`
builds:

```xml
<!--
  Enable full trimming in Release mode.
  To learn more, see: https://learn.microsoft.com/dotnet/core/deploying/trimming/trimming-options#trimming-granularity
-->
<PropertyGroup Condition="'$(Configuration)' == 'Release'">
  <TrimMode>full</TrimMode>
</PropertyGroup>
```

We wouldn't want to do this for existing projects, as they might have existing
code, NuGet packages, etc. where trimming warnings might be present.

We can also improve the templates for class libraries and binding libraries:

* `dotnet new ioslib|iosbinding|tvoslib|tvosbinding|...`

```xml
<!--
  Enable trim analyzers for Android class libraries.
  To learn more, see: https://learn.microsoft.com/dotnet/core/deploying/trimming/prepare-libraries-for-trimming
-->
<IsTrimmable>true</IsTrimmable>
```

This way, new class libraries and binding libraries will be trimmable by
default and be able to react to trimming warnings.

Fixes #10405.

Ref: dotnet/android#8805
@rolfbjarne
Copy link
Member Author

This is done now.

@github-project-automation github-project-automation bot moved this to Done in .NET 9 Aug 27, 2024
rolfbjarne added a commit that referenced this issue Sep 27, 2024
Our own code shouldn't produce trim analysis warnings anymore (see #10405), so
we don't need to suppress trim analysis warnings for everyone.

Fixes #11246.
rolfbjarne added a commit that referenced this issue Oct 1, 2024
Our own code shouldn't produce trim analysis warnings anymore (see #10405), so
we don't need to suppress trim analysis warnings for everyone.

Fixes #21293.
rolfbjarne added a commit that referenced this issue Oct 24, 2024
Our own code shouldn't produce trim analysis warnings anymore (see #10405), so we don't need to suppress trim analysis warnings for everyone. 

Except: we still want to suppress trim analysis warnings if no assemblies are trimmed,  because otherwise we'll get warnings for code that would otherwise be trimmed away.

On the other hand, we want trim analyzer always enabled, so that warnings are reported for user code.

The difference between "trim analyzer" and "trim analysis warnings" is that the former is an analyzer that analyzes the currently compiled code (which we always want), while the latter is reported by the trimmers, and as such will report warnings from *all* code, including all references.

This also required bumping Touch.Unit and MonoTouch.Dialog.

* New commits in xamarin/MonoTouch.Dialog:

    * xamarin/MonoTouch.Dialog@d157950 Exclude code that's not trimmer safe.

        Diff: [77b3337..d157950](https://github.com/xamarin/MonoTouch.Dialog/compare/77b3337dbbc9e3e2f1b06dab3d37d2822488b0b3..d157950b6e6ed32cf53d4074fe19223dc1f1e8fe)

* New commits in xamarin/Touch.Unit:

    * xamarin/Touch.Unit@957faca [Touch.Client] Disable features that aren't trimmer safe.

        Diff: [92a0726..957faca](https://github.com/xamarin/Touch.Unit/compare/92a072683b69b2f61f235a7a249c2c7f261236d4..957facad80e753310f52fcbb8cf85219d9d6d887)

Fixes #21293.
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) enhancement The issue or pull request is an enhancement estimate-1w iOS Issues affecting iOS macOS Issues affecting macOS
Projects
Status: Done
Development

No branches or pull requests

1 participant