Skip to content

Commit c5cb147

Browse files
authored
Trim analyzer: Implement intrinsics handling of object.GetType (#93732)
This fixes #86921. Analyzer so far didn't handle correct data flow around `object.GetType` and `DynamicallyAccessedMembersAttribute` on types. This change implements that behavior. Main changes: * Move `IValueWithStaticType` to the shared code and refactor its existing usage in trimmer/AOT to use the shared code instead. Also implement it for the analyzer. * Refactor method call handling in the analyzer to a single static method which is called both from the visitor and from the patterns. * In order to get same behavior, start tracking values for all fields and method parameters. Outside of the actual fix, the other main change is that analyzer now tracks values for all fields and method parameters, regardless if their type is interesting to analysis. This is necessary because the static type now matterns, even if it's something else than `System.Type`. The effect of that is that the analyzer now recognizes lot more invalid cases because it can determine if the value is something unrecognizable. Before the change such values where tracked as "empty", and thus anslysis ignored them. Now they're track as "value of a field, without annotations" which can lead to producing more warnings. That means this effectively fixes dotnet/linker#2755. At least all the test cases which were added because of that bug, or which expected different behavior because of it now produce consistent behavior with trimmer/NativeAOT.
1 parent f5e6c01 commit c5cb147

31 files changed

+211
-148
lines changed

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FieldValue.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Collections.Generic;
55
using System.Diagnostics.CodeAnalysis;
66
using ILCompiler;
7-
using ILCompiler.Dataflow;
87
using ILLink.Shared.DataFlow;
98
using Internal.TypeSystem;
109

@@ -16,7 +15,7 @@ namespace ILLink.Shared.TrimAnalysis
1615
/// <summary>
1716
/// A representation of a field. Typically a result of ldfld.
1817
/// </summary>
19-
internal sealed partial record FieldValue : IValueWithStaticType
18+
internal sealed partial record FieldValue
2019
{
2120
public FieldValue(FieldDesc field, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
2221
{
@@ -32,8 +31,6 @@ public FieldValue(FieldDesc field, DynamicallyAccessedMemberTypes dynamicallyAcc
3231
public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch()
3332
=> new string[] { Field.GetDisplayName() };
3433

35-
public TypeDesc? StaticType { get; }
36-
3734
public override SingleValue DeepCopy() => this; // This value is immutable
3835

3936
public override string ToString() => this.ValueToString(Field, DynamicallyAccessedMemberTypes);

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/IValueWithStaticType.cs

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

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1041,7 +1041,7 @@ when GetMethodParameterValue(parameterReference.Parameter) is MethodParameterVal
10411041
HandleStoreField(method, offset, fieldValue, DereferenceValue(method, offset, source, locals, ref ipState));
10421042
break;
10431043
case IValueWithStaticType valueWithStaticType:
1044-
if (valueWithStaticType.StaticType is not null && FlowAnnotations.IsTypeInterestingForDataflow(valueWithStaticType.StaticType))
1044+
if (valueWithStaticType.StaticType is not null && FlowAnnotations.IsTypeInterestingForDataflow(valueWithStaticType.StaticType.Value.Type))
10451045
throw new InvalidOperationException(MessageContainer.CreateErrorMessage(
10461046
$"Unhandled StoreReference call. Unhandled attempt to store a value in {value} of type {value.GetType()}.",
10471047
(int)DiagnosticId.LinkerUnexpectedError,

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodParameterValue.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics.CodeAnalysis;
5-
using ILCompiler.Dataflow;
65
using ILLink.Shared.TypeSystemProxy;
7-
using Internal.TypeSystem;
86

97
#nullable enable
108

@@ -14,7 +12,7 @@ namespace ILLink.Shared.TrimAnalysis
1412
/// <summary>
1513
/// A value that came from a method parameter - such as the result of a ldarg.
1614
/// </summary>
17-
internal partial record MethodParameterValue : IValueWithStaticType
15+
internal partial record MethodParameterValue
1816
{
1917
public MethodParameterValue(ParameterProxy param, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, bool overrideIsThis = false)
2018
{
@@ -25,7 +23,5 @@ public MethodParameterValue(ParameterProxy param, DynamicallyAccessedMemberTypes
2523
}
2624

2725
public override DynamicallyAccessedMemberTypes DynamicallyAccessedMemberTypes { get; }
28-
29-
public TypeDesc? StaticType { get; }
3026
}
3127
}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace ILLink.Shared.TrimAnalysis
1414
/// <summary>
1515
/// Return value from a method
1616
/// </summary>
17-
internal partial record MethodReturnValue : IValueWithStaticType
17+
internal partial record MethodReturnValue
1818
{
1919
public MethodReturnValue(MethodDesc method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
2020
{
@@ -30,8 +30,6 @@ public MethodReturnValue(MethodDesc method, DynamicallyAccessedMemberTypes dynam
3030
public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch()
3131
=> new string[] { DiagnosticUtilities.GetMethodSignatureDisplayName(Method) };
3232

33-
public TypeDesc? StaticType { get; }
34-
3533
public override SingleValue DeepCopy() => this; // This value is immutable
3634

3735
public override string ToString() => this.ValueToString(Method, DynamicallyAccessedMemberTypes);

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ public static bool HandleCall(
538538
// In this case to get correct results, trimmer would have to mark all public methods on Derived. Which
539539
// currently it won't do.
540540

541-
TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType;
541+
TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type;
542542
if (staticType is null || (!staticType.IsDefType && !staticType.IsArray))
543543
{
544544
// We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations"

src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,6 @@
377377
<Compile Include="Compiler\Dataflow\HandleCallAction.cs" />
378378
<Compile Include="Compiler\Dataflow\HoistedLocalKey.cs" />
379379
<Compile Include="Compiler\Dataflow\InterproceduralState.cs" />
380-
<Compile Include="Compiler\Dataflow\IValueWithStaticType.cs" />
381380
<Compile Include="Compiler\Dataflow\LocalVariableReferenceValue.cs" />
382381
<Compile Include="Compiler\Dataflow\MethodBodyScanner.cs" />
383382
<Compile Include="Compiler\Dataflow\MethodParameterValue.cs" />

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FieldValue.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ namespace ILLink.Shared.TrimAnalysis
1111
{
1212
internal partial record FieldValue
1313
{
14-
public FieldValue (IFieldSymbol fieldSymbol) => FieldSymbol = fieldSymbol;
14+
public FieldValue (IFieldSymbol fieldSymbol)
15+
{
16+
FieldSymbol = fieldSymbol;
17+
StaticType = new (fieldSymbol.Type);
18+
}
1519

1620
public readonly IFieldSymbol FieldSymbol;
1721

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ public static DynamicallyAccessedMemberTypes GetMethodReturnValueAnnotation (IMe
7878
return returnDamt;
7979
}
8080

81+
public static DynamicallyAccessedMemberTypes GetTypeAnnotation(ITypeSymbol type)
82+
{
83+
var typeAnnotation = type.GetDynamicallyAccessedMemberTypes ();
84+
85+
ITypeSymbol? baseType = type.BaseType;
86+
while (baseType != null) {
87+
typeAnnotation |= baseType.GetDynamicallyAccessedMemberTypes ();
88+
baseType = baseType.BaseType;
89+
}
90+
91+
foreach (var interfaceType in type.AllInterfaces) {
92+
typeAnnotation |= interfaceType.GetDynamicallyAccessedMemberTypes ();
93+
}
94+
95+
return typeAnnotation;
96+
}
97+
8198
#pragma warning disable CA1822 // Mark members as static - the other partial implementations might need to be instance methods
8299

83100
// TODO: This is relatively expensive on the analyzer since it doesn't cache the annotation information

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodParameterValue.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public MethodParameterValue (ParameterProxy parameter, DynamicallyAccessedMember
2121
{
2222
Parameter = parameter;
2323
DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes;
24+
StaticType = parameter.ParameterType;
2425
_overrideIsThis = overrideIsThis;
2526
}
2627

0 commit comments

Comments
 (0)