Skip to content

Commit 65f04b9

Browse files
eerhardteerhardt
authored
Resolve MakeGenericType ILLink warning in DependencyInjection (#55102)
* Resolve MakeGenericType ILLink warning in DependencyInjection Resolve the ILLink warning in DependencyInjection by adding a runtime check that is behind a new AppContext switch 'Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability'. The runtime check ensures the trimming annotations on the open generic types are compatible between the service and implementation types. The check is enabled by default when PublishTrimmed=true. * Make VerifyOpenGenericServiceTrimmability a full feature switch
1 parent 3ec2f67 commit 65f04b9

File tree

10 files changed

+171
-18
lines changed

10 files changed

+171
-18
lines changed

‎docs/workflow/trimming/feature-switches.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif
2525
| EnableCppCLIHostActivation | System.Runtime.InteropServices.EnableCppCLIHostActivation | C++/CLI host activation code is disabled when set to false and related functionality can be trimmed. |
2626
| MetadataUpdaterSupport | System.Reflection.Metadata.MetadataUpdater.IsSupported | Metadata update related code to be trimmed when set to false |
2727
| _EnableConsumingManagedCodeFromNativeHosting | System.Runtime.InteropServices.EnableConsumingManagedCodeFromNativeHosting | Getting a managed function from native hosting is disabled when set to false and related functionality can be trimmed. |
28+
| VerifyDependencyInjectionOpenGenericServiceTrimmability | Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability | When set to true, DependencyInjection will verify trimming annotations applied to open generic services are correct |
2829
| NullabilityInfoContextSupport | System.Reflection.NullabilityInfoContext.IsSupported | Nullable attributes can be trimmed when set to false |
2930
| _AggressiveAttributeTrimming | System.AggressiveAttributeTrimming | When set to true, aggressively trims attributes to allow for the most size savings possible, even if it could result in runtime behavior changes |
3031

‎src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/Microsoft.Extensions.DependencyInjection.Abstractions.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.1;netstandard2.0;net461</TargetFrameworks>

‎src/libraries/Microsoft.Extensions.DependencyInjection/src/ILLink/ILLink.Substitutions.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<linker>
2+
<assembly fullname="Microsoft.Extensions.DependencyInjection">
3+
<type fullname="Microsoft.Extensions.DependencyInjection.ServiceProvider">
4+
<method signature="System.Boolean get_VerifyOpenGenericServiceTrimmability()" body="stub" value="true" feature="Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability" featurevalue="true" />
5+
</type>
6+
</assembly>
7+
</linker>

‎src/libraries/Microsoft.Extensions.DependencyInjection/src/ILLink/ILLink.Suppressions.xml‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎src/libraries/Microsoft.Extensions.DependencyInjection/src/Microsoft.Extensions.DependencyInjection.csproj‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.1;netstandard2.0;net461</TargetFrameworks>
@@ -19,6 +19,10 @@
1919
<DefineConstants Condition="$(TargetFramework.StartsWith('net4')) and '$(ILEmitBackendSaveAssemblies)' == 'True'">$(DefineConstants);SAVE_ASSEMBLIES</DefineConstants>
2020
</PropertyGroup>
2121

22+
<ItemGroup>
23+
<ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.xml" />
24+
</ItemGroup>
25+
2226
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
2327
<Compile Include="$(CommonPath)Extensions\ParameterDefaultValue\ParameterDefaultValue.netcoreapp.cs"
2428
Link="Common\src\Extensions\ParameterDefaultValue\ParameterDefaultValue.netcoreapp.cs" />
@@ -32,7 +36,7 @@
3236
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMemberTypes.cs" />
3337
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs" />
3438
</ItemGroup>
35-
39+
3640
<ItemGroup>
3741
<Compile Include="**\*.cs" />
3842
<Compile Remove="ServiceLookup\ILEmit\**\*.cs" />
@@ -41,7 +45,6 @@
4145
Link="Common\src\Extensions\ParameterDefaultValue\ParameterDefaultValue.cs" />
4246
<Compile Include="$(CommonPath)Extensions\TypeNameHelper\TypeNameHelper.cs"
4347
Link="Common\src\Extensions\TypeNameHelper\TypeNameHelper.cs" />
44-
4548
</ItemGroup>
4649

4750
<ItemGroup>

‎src/libraries/Microsoft.Extensions.DependencyInjection/src/Resources/Strings.resx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,4 +174,10 @@
174174
<data name="CallSiteTypeNotSupported" xml:space="preserve">
175175
<value>Call site type {0} is not supported</value>
176176
</data>
177-
</root>
177+
<data name="TrimmingAnnotationsDoNotMatch" xml:space="preserve">
178+
<value>Generic implementation type '{0}' has a DynamicallyAccessedMembers attribute applied to a generic argument type, but the service type '{1}' doesn't have a matching DynamicallyAccessedMembers attribute on its generic argument type.</value>
179+
</data>
180+
<data name="TrimmingAnnotationsDoNotMatch_NewConstraint" xml:space="preserve">
181+
<value>Generic implementation type '{0}' has a DefaultConstructorConstraint ('new()' constraint), but the generic service type '{1}' doesn't.</value>
182+
</data>
183+
</root>

‎src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,18 @@ private void Populate()
5151
SR.Format(SR.TypeCannotBeActivated, implementationType, serviceType));
5252
}
5353

54-
if (serviceType.GetGenericArguments().Length != implementationType.GetGenericArguments().Length)
54+
Type[] serviceTypeGenericArguments = serviceType.GetGenericArguments();
55+
Type[] implementationTypeGenericArguments = implementationType.GetGenericArguments();
56+
if (serviceTypeGenericArguments.Length != implementationTypeGenericArguments.Length)
5557
{
5658
throw new ArgumentException(
5759
SR.Format(SR.ArityOfOpenGenericServiceNotEqualArityOfOpenGenericImplementation, serviceType, implementationType), "descriptors");
5860
}
61+
62+
if (ServiceProvider.VerifyOpenGenericServiceTrimmability)
63+
{
64+
ValidateTrimmingAnnotations(serviceType, serviceTypeGenericArguments, implementationType, implementationTypeGenericArguments);
65+
}
5966
}
6067
else if (descriptor.ImplementationInstance == null && descriptor.ImplementationFactory == null)
6168
{
@@ -77,6 +84,68 @@ private void Populate()
7784
}
7885
}
7986

87+
/// <summary>
88+
/// Validates that two generic type definitions have compatible trimming annotations on their generic arguments.
89+
/// </summary>
90+
/// <remarks>
91+
/// When open generic types are used in DI, there is an error when the concrete implementation type
92+
/// has [DynamicallyAccessedMembers] attributes on a generic argument type, but the interface/service type
93+
/// doesn't have matching annotations. The problem is that the trimmer doesn't see the members that need to
94+
/// be preserved on the type being passed to the generic argument. But when the interface/service type also has
95+
/// the annotations, the trimmer will see which members need to be preserved on the closed generic argument type.
96+
/// </remarks>
97+
private static void ValidateTrimmingAnnotations(
98+
Type serviceType,
99+
Type[] serviceTypeGenericArguments,
100+
Type implementationType,
101+
Type[] implementationTypeGenericArguments)
102+
{
103+
Debug.Assert(serviceTypeGenericArguments.Length == implementationTypeGenericArguments.Length);
104+
105+
for (int i = 0; i < serviceTypeGenericArguments.Length; i++)
106+
{
107+
Type serviceGenericType = serviceTypeGenericArguments[i];
108+
Type implementationGenericType = implementationTypeGenericArguments[i];
109+
110+
DynamicallyAccessedMemberTypes serviceDynamicallyAccessedMembers = GetDynamicallyAccessedMemberTypes(serviceGenericType);
111+
DynamicallyAccessedMemberTypes implementationDynamicallyAccessedMembers = GetDynamicallyAccessedMemberTypes(implementationGenericType);
112+
113+
if (!AreCompatible(serviceDynamicallyAccessedMembers, implementationDynamicallyAccessedMembers))
114+
{
115+
throw new ArgumentException(SR.Format(SR.TrimmingAnnotationsDoNotMatch, implementationType.FullName, serviceType.FullName));
116+
}
117+
118+
bool serviceHasNewConstraint = serviceGenericType.GenericParameterAttributes.HasFlag(GenericParameterAttributes.DefaultConstructorConstraint);
119+
bool implementationHasNewConstraint = implementationGenericType.GenericParameterAttributes.HasFlag(GenericParameterAttributes.DefaultConstructorConstraint);
120+
if (implementationHasNewConstraint && !serviceHasNewConstraint)
121+
{
122+
throw new ArgumentException(SR.Format(SR.TrimmingAnnotationsDoNotMatch_NewConstraint, implementationType.FullName, serviceType.FullName));
123+
}
124+
}
125+
}
126+
127+
private static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes(Type serviceGenericType)
128+
{
129+
foreach (CustomAttributeData attributeData in serviceGenericType.GetCustomAttributesData())
130+
{
131+
if (attributeData.AttributeType.FullName == "System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute" &&
132+
attributeData.ConstructorArguments.Count == 1 &&
133+
attributeData.ConstructorArguments[0].ArgumentType.FullName == "System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes")
134+
{
135+
return (DynamicallyAccessedMemberTypes)(int)attributeData.ConstructorArguments[0].Value;
136+
}
137+
}
138+
139+
return DynamicallyAccessedMemberTypes.None;
140+
}
141+
142+
private static bool AreCompatible(DynamicallyAccessedMemberTypes serviceDynamicallyAccessedMembers, DynamicallyAccessedMemberTypes implementationDynamicallyAccessedMembers)
143+
{
144+
// The DynamicallyAccessedMemberTypes don't need to exactly match.
145+
// The service type needs to preserve a superset of the members required by the implementation type.
146+
return serviceDynamicallyAccessedMembers.HasFlag(implementationDynamicallyAccessedMembers);
147+
}
148+
80149
// For unit testing
81150
internal int? GetSlot(ServiceDescriptor serviceDescriptor)
82151
{
@@ -273,6 +342,10 @@ private ServiceCallSite TryCreateExact(ServiceDescriptor descriptor, Type servic
273342
return null;
274343
}
275344

345+
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:MakeGenericType",
346+
Justification = "MakeGenericType here is used to create a closed generic implementation type given the closed service type. " +
347+
"Trimming annotations on the generic types are verified when 'Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability' is set, which is set by default when PublishTrimmed=true. " +
348+
"That check informs developers when these generic types don't have compatible trimming annotations.")]
276349
private ServiceCallSite TryCreateOpenGeneric(ServiceDescriptor descriptor, Type serviceType, CallSiteChain callSiteChain, int slot, bool throwOnConstraintViolation)
277350
{
278351
if (serviceType.IsConstructedGenericType &&

‎src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ public sealed class ServiceProvider : IServiceProvider, IDisposable, IAsyncDispo
3030

3131
internal ServiceProviderEngineScope Root { get; }
3232

33+
internal static bool VerifyOpenGenericServiceTrimmability { get; } =
34+
AppContext.TryGetSwitch("Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability", out bool verifyOpenGenerics) ? verifyOpenGenerics : false;
35+
3336
internal ServiceProvider(IEnumerable<ServiceDescriptor> serviceDescriptors, ServiceProviderOptions options)
3437
{
3538
// note that Root needs to be set before calling GetEngine(), because the engine may need to access Root

‎src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<TargetFrameworks>$(NetCoreAppCurrent);net461</TargetFrameworks>
55
<EnableDefaultItems>true</EnableDefaultItems>
6+
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
7+
<!-- CS0436: DynamicallyAccessedMemberTypes conflicts with imported type -->
8+
<NoWarn Condition="'$(TargetFramework)' == 'net461'">$(NoWarn);CS0436</NoWarn>
69
</PropertyGroup>
710

811
<ItemGroup>
@@ -11,6 +14,12 @@
1114
Link="Shared\SingleThreadedSynchronizationContext.cs" />
1215
</ItemGroup>
1316

17+
<ItemGroup Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net5.0'))">
18+
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs" />
19+
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMembersAttribute.cs" />
20+
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMemberTypes.cs" />
21+
</ItemGroup>
22+
1423
<ItemGroup>
1524
<PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
1625
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.DependencyInjection.Abstractions\src\Microsoft.Extensions.DependencyInjection.Abstractions.csproj" SkipUseReferenceAssembly="true" />

‎src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics.CodeAnalysis;
67
using System.Linq;
78
using System.Reflection;
89
using System.Threading.Tasks;
10+
using Microsoft.DotNet.RemoteExecutor;
911
using Microsoft.Extensions.DependencyInjection.Extensions;
1012
using Microsoft.Extensions.DependencyInjection.Specification.Fakes;
1113
using Xunit;
@@ -849,6 +851,51 @@ public void CallSitesAreUniquePerServiceTypeAndSlotWithOpenGenericInGraph()
849851
}
850852
}
851853

854+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
855+
[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] // RuntimeConfigurationOptions are not supported on .NET Framework (and neither is trimming)
856+
public void VerifyOpenGenericTrimmabilityChecks()
857+
{
858+
RemoteInvokeOptions options = new RemoteInvokeOptions();
859+
options.RuntimeConfigurationOptions.Add("Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability", "true");
860+
861+
using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(() =>
862+
{
863+
(Type, Type)[] invalidTestCases = new[]
864+
{
865+
(typeof(IFakeOpenGenericService<>), typeof(ClassWithNewConstraint<>)),
866+
(typeof(IServiceWithoutTrimmingAnnotations<>), typeof(ServiceWithTrimmingAnnotations<>)),
867+
(typeof(IServiceWithPublicConstructors<>), typeof(ServiceWithPublicProperties<>)),
868+
(typeof(IServiceWithTwoGenerics<,>), typeof(ServiceWithTwoGenericsInvalid<,>)),
869+
};
870+
foreach ((Type serviceType, Type implementationType) in invalidTestCases)
871+
{
872+
ServiceDescriptor[] serviceDescriptors = new[]
873+
{
874+
new ServiceDescriptor(serviceType, implementationType, ServiceLifetime.Singleton)
875+
};
876+
877+
Assert.Throws<ArgumentException>(() => new CallSiteFactory(serviceDescriptors));
878+
}
879+
880+
(Type, Type)[] validTestCases = new[]
881+
{
882+
(typeof(IFakeOpenGenericService<>), typeof(FakeOpenGenericService<>)),
883+
(typeof(IServiceWithPublicConstructors<>), typeof(ServiceWithPublicConstructors<>)),
884+
(typeof(IServiceWithTwoGenerics<,>), typeof(ServiceWithTwoGenericsValid<,>)),
885+
(typeof(IServiceWithMoreMemberTypes<>), typeof(ServiceWithLessMemberTypes<>)),
886+
};
887+
foreach ((Type serviceType, Type implementationType) in validTestCases)
888+
{
889+
ServiceDescriptor[] serviceDescriptors = new[]
890+
{
891+
new ServiceDescriptor(serviceType, implementationType, ServiceLifetime.Singleton)
892+
};
893+
894+
Assert.NotNull(new CallSiteFactory(serviceDescriptors));
895+
}
896+
}, options);
897+
}
898+
852899
private static Func<Type, ServiceCallSite> GetCallSiteFactory(params ServiceDescriptor[] descriptors)
853900
{
854901
var collection = new ServiceCollection();
@@ -887,5 +934,20 @@ private class ClassB { public ClassB(ClassC<object> cc) { } }
887934
private class ClassC<T> { }
888935
private class ClassD { public ClassD(ClassC<string> cd) { } }
889936
private class ClassE { public ClassE(ClassB cb) { } }
937+
938+
// Open generic with trimming annotations
939+
private interface IServiceWithoutTrimmingAnnotations<T> { }
940+
private class ServiceWithTrimmingAnnotations<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T> : IServiceWithoutTrimmingAnnotations<T> { }
941+
942+
private interface IServiceWithPublicConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T> { }
943+
private class ServiceWithPublicProperties<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>: IServiceWithPublicConstructors<T> { }
944+
private class ServiceWithPublicConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>: IServiceWithPublicConstructors<T> { }
945+
946+
private interface IServiceWithTwoGenerics<T1, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T2> { }
947+
private class ServiceWithTwoGenericsInvalid<T1, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T2> : IServiceWithTwoGenerics<T1, T2> { }
948+
private class ServiceWithTwoGenericsValid<T1, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T2> : IServiceWithTwoGenerics<T1, T2> { }
949+
950+
private interface IServiceWithMoreMemberTypes<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties)] T> { }
951+
private class ServiceWithLessMemberTypes<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T> : IServiceWithMoreMemberTypes<T> { }
890952
}
891953
}

0 commit comments

Comments
 (0)