Skip to content

Commit

Permalink
Share intrinsic handling of GetMember and similar APIs (dotnet/linker…
Browse files Browse the repository at this point in the history 
…#2639)

Other than sharing more code and adapting it so that it works on both linker and analyzer, this change brings simple analysis of constant integer values in the analyzer. This is necessary to make most reflection API calls recognize binding flags. For example `GetMethods(BindingFlags.Public | BindingFlags.Static)`. So this change adds analysis of constant values (as recognized by the Roslyn's operation tree) and the OR binary operator for integers and enums.

Added some new tests for the binding flags handling.

Reenabled some disabled tests for analyzer.
Moved the main affected tests from the generated source files to the hardcoded one and force them to exact match of warnings for both linker and analyzer.

Commit migrated from dotnet/linker@f0bd2ae
  • Loading branch information
@vitek-karas
vitek-karas authored Feb 28, 2022
1 parent 97fb195 commit 8b9fae7
Show file tree
Hide file tree
Showing 18 changed files with 795 additions and 570 deletions.
37 changes: 36 additions & 1 deletion src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using ILLink.RoslynAnalyzer;
using ILLink.RoslynAnalyzer.DataFlow;
using ILLink.RoslynAnalyzer.TrimAnalysis;
using ILLink.Shared.TypeSystemProxy;
using Microsoft.CodeAnalysis;
Expand All @@ -16,13 +19,15 @@ partial struct HandleCallAction

readonly ISymbol _owningSymbol;
readonly IOperation _operation;
readonly ReflectionAccessAnalyzer _reflectionAccessAnalyzer;

public HandleCallAction (in DiagnosticContext diagnosticContext, ISymbol owningSymbol, IOperation operation)
{
_owningSymbol = owningSymbol;
_operation = operation;
_diagnosticContext = diagnosticContext;
_requireDynamicallyAccessedMembersAction = new (diagnosticContext, new ReflectionAccessAnalyzer ());
_reflectionAccessAnalyzer = new ReflectionAccessAnalyzer ();
_requireDynamicallyAccessedMembersAction = new (diagnosticContext, _reflectionAccessAnalyzer);
}

// TODO: This is relatively expensive on the analyzer since it doesn't cache the annotation information
Expand All @@ -41,9 +46,39 @@ private partial GenericParameterValue GetGenericParameterValue (GenericParameter
private partial MethodThisParameterValue GetMethodThisParameterValue (MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
=> new (method.Method, dynamicallyAccessedMemberTypes);

private partial MethodParameterValue GetMethodParameterValue (MethodProxy method, int parameterIndex, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
=> new (method.Method.Parameters[parameterIndex], dynamicallyAccessedMemberTypes);

private partial IEnumerable<SystemReflectionMethodBaseValue> GetMethodsOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags)
{
foreach (var method in type.Type.GetMethodsOnTypeHierarchy (m => m.Name == name, bindingFlags))
yield return new SystemReflectionMethodBaseValue (new MethodProxy (method));
}

private partial IEnumerable<SystemTypeValue> GetNestedTypesOnType (TypeProxy type, string name, BindingFlags? bindingFlags)
{
foreach (var nestedType in type.Type.GetNestedTypesOnType (t => t.Name == name, bindingFlags))
yield return new SystemTypeValue (new TypeProxy (nestedType));
}

// TODO: Does the analyzer need to do something here?
private partial void MarkStaticConstructor (TypeProxy type) { }

private partial void MarkEventsOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForEventsOnTypeHierarchy (_diagnosticContext, type.Type, name, bindingFlags);

private partial void MarkFieldsOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForFieldsOnTypeHierarchy (_diagnosticContext, type.Type, name, bindingFlags);

private partial void MarkPropertiesOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForPropertiesOnTypeHierarchy (_diagnosticContext, type.Type, name, bindingFlags);

private partial void MarkMethod (MethodProxy method)
=> ReflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForMethod (_diagnosticContext, method.Method);

// TODO: Does the analyzer need to do something here?
private partial void MarkType (TypeProxy type) { }

private partial string GetContainingSymbolDisplayName () => _operation.FindContainingSymbol (_owningSymbol).GetDisplayName ();
}
}
8 changes: 6 additions & 2 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodParameterValue.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ namespace ILLink.Shared.TrimAnalysis
{
partial record MethodParameterValue
{
public MethodParameterValue (IParameterSymbol parameterSymbol) => ParameterSymbol = parameterSymbol;
public MethodParameterValue (IParameterSymbol parameterSymbol)
: this (parameterSymbol, FlowAnnotations.GetMethodParameterAnnotation (parameterSymbol)) { }

public MethodParameterValue (IParameterSymbol parameterSymbol, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
=> (ParameterSymbol, DynamicallyAccessedMemberTypes) = (parameterSymbol, dynamicallyAccessedMemberTypes);

public readonly IParameterSymbol ParameterSymbol;

public override DynamicallyAccessedMemberTypes DynamicallyAccessedMemberTypes => FlowAnnotations.GetMethodParameterAnnotation (ParameterSymbol);
public override DynamicallyAccessedMemberTypes DynamicallyAccessedMemberTypes { get; }

public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch ()
=> new string[] { ParameterSymbol.GetDisplayName (), ParameterSymbol.ContainingSymbol.GetDisplayName () };
Expand Down
33 changes: 26 additions & 7 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ReflectionAccessAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using ILLink.RoslynAnalyzer.DataFlow;
using ILLink.Shared;
using ILLink.Shared.TrimAnalysis;
Expand All @@ -17,7 +18,7 @@ internal void GetReflectionAccessDiagnostics (in DiagnosticContext diagnosticCon
foreach (var member in typeSymbol.GetDynamicallyAccessedMembers (requiredMemberTypes, declaredOnly)) {
switch (member) {
case IMethodSymbol method:
GetDiagnosticsForMethod (diagnosticContext, method);
GetReflectionAccessDiagnosticsForMethod (diagnosticContext, method);
break;
case IFieldSymbol field:
GetDiagnosticsForField (diagnosticContext, field);
Expand All @@ -40,14 +41,32 @@ internal void GetReflectionAccessDiagnostics (in DiagnosticContext diagnosticCon
}
}

internal void GetReflectionAccessDiagnosticsForEventsOnTypeHierarchy (in DiagnosticContext diagnosticContext, ITypeSymbol typeSymbol, string name, BindingFlags? bindingFlags)
{
foreach (var @event in typeSymbol.GetEventsOnTypeHierarchy (e => e.Name == name, bindingFlags))
GetDiagnosticsForEvent (diagnosticContext, @event);
}

internal void GetReflectionAccessDiagnosticsForFieldsOnTypeHierarchy (in DiagnosticContext diagnosticContext, ITypeSymbol typeSymbol, string name, BindingFlags? bindingFlags)
{
foreach (var field in typeSymbol.GetFieldsOnTypeHierarchy (f => f.Name == name, bindingFlags))
GetDiagnosticsForField (diagnosticContext, field);
}

internal void GetReflectionAccessDiagnosticsForPropertiesOnTypeHierarchy (in DiagnosticContext diagnosticContext, ITypeSymbol typeSymbol, string name, BindingFlags? bindingFlags)
{
foreach (var prop in typeSymbol.GetPropertiesOnTypeHierarchy (p => p.Name == name, bindingFlags))
GetDiagnosticsForProperty (diagnosticContext, prop);
}

static void ReportRequiresUnreferencedCodeDiagnostic (in DiagnosticContext diagnosticContext, AttributeData requiresAttributeData, ISymbol member)
{
var message = RequiresUnreferencedCodeUtils.GetMessageFromAttribute (requiresAttributeData);
var url = RequiresAnalyzerBase.GetUrlFromAttribute (requiresAttributeData);
diagnosticContext.AddDiagnostic (DiagnosticId.RequiresUnreferencedCode, member.GetDisplayName (), message, url);
}

static void GetDiagnosticsForMethod (in DiagnosticContext diagnosticContext, IMethodSymbol methodSymbol)
internal static void GetReflectionAccessDiagnosticsForMethod (in DiagnosticContext diagnosticContext, IMethodSymbol methodSymbol)
{
if (methodSymbol.TryGetRequiresUnreferencedCodeAttribute (out var requiresAttributeData))
ReportRequiresUnreferencedCodeDiagnostic (diagnosticContext, requiresAttributeData, methodSymbol);
Expand All @@ -69,19 +88,19 @@ static void GetDiagnosticsForMethod (in DiagnosticContext diagnosticContext, IMe
static void GetDiagnosticsForProperty (in DiagnosticContext diagnosticContext, IPropertySymbol propertySymbol)
{
if (propertySymbol.SetMethod is not null)
GetDiagnosticsForMethod (diagnosticContext, propertySymbol.SetMethod);
GetReflectionAccessDiagnosticsForMethod (diagnosticContext, propertySymbol.SetMethod);
if (propertySymbol.GetMethod is not null)
GetDiagnosticsForMethod (diagnosticContext, propertySymbol.GetMethod);
GetReflectionAccessDiagnosticsForMethod (diagnosticContext, propertySymbol.GetMethod);
}

static void GetDiagnosticsForEvent (in DiagnosticContext diagnosticContext, IEventSymbol eventSymbol)
{
if (eventSymbol.AddMethod is not null)
GetDiagnosticsForMethod (diagnosticContext, eventSymbol.AddMethod);
GetReflectionAccessDiagnosticsForMethod (diagnosticContext, eventSymbol.AddMethod);
if (eventSymbol.RemoveMethod is not null)
GetDiagnosticsForMethod (diagnosticContext, eventSymbol.RemoveMethod);
GetReflectionAccessDiagnosticsForMethod (diagnosticContext, eventSymbol.RemoveMethod);
if (eventSymbol.RaiseMethod is not null)
GetDiagnosticsForMethod (diagnosticContext, eventSymbol.RaiseMethod);
GetReflectionAccessDiagnosticsForMethod (diagnosticContext, eventSymbol.RaiseMethod);
}

static void GetDiagnosticsForField (in DiagnosticContext diagnosticContext, IFieldSymbol fieldSymbol)
Expand Down
67 changes: 58 additions & 9 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ public class TrimAnalysisVisitor : LocalDataFlowVisitor<MultiValue, ValueSetLatt
{
public readonly TrimAnalysisPatternStore TrimAnalysisPatterns;

readonly ValueSetLattice<SingleValue> _multiValueLattice;

public TrimAnalysisVisitor (
LocalStateLattice<MultiValue, ValueSetLattice<SingleValue>> lattice,
OperationBlockAnalysisContext context
) : base (lattice, context)
{
TrimAnalysisPatterns = new TrimAnalysisPatternStore (lattice.Lattice.ValueLattice);
_multiValueLattice = lattice.Lattice.ValueLattice;
TrimAnalysisPatterns = new TrimAnalysisPatternStore (_multiValueLattice);
}

// Override visitor methods to create tracked values when visiting operations
Expand All @@ -36,6 +39,26 @@ OperationBlockAnalysisContext context
// - 'this' parameter (for annotated methods)
// - field reference

public override MultiValue Visit (IOperation? operation, StateValue argument)
{
var returnValue = base.Visit (operation, argument);

// If the return value is empty (TopValue basically) and the Operation tree
// reports it as having a constant value, use that as it will automatically cover
// cases we don't need/want to handle.
if (operation != null && returnValue.IsEmpty () && operation.ConstantValue.HasValue) {
object? constantValue = operation.ConstantValue.Value;
if (constantValue == null)
return NullValue.Instance;
else if (operation.Type?.SpecialType == SpecialType.System_String && constantValue is string stringConstantValue)
return new KnownStringValue (stringConstantValue);
else if (operation.Type?.TypeKind == TypeKind.Enum && constantValue is int intConstantValue)
return new ConstIntValue (intConstantValue);
}

return returnValue;
}

public override MultiValue VisitConversion (IConversionOperation operation, StateValue state)
{
var value = base.VisitConversion (operation, state);
Expand Down Expand Up @@ -70,14 +93,15 @@ public override MultiValue VisitInstanceReference (IInstanceReferenceOperation i

public override MultiValue VisitFieldReference (IFieldReferenceOperation fieldRef, StateValue state)
{
if (!fieldRef.Field.Type.IsTypeInterestingForDataflow ())
return TopValue;
if (fieldRef.Field.Type.IsTypeInterestingForDataflow ()) {
var field = fieldRef.Field;
if (field.Name is "Empty" && field.ContainingType.HasName ("System.String"))
return new KnownStringValue (string.Empty);

var field = fieldRef.Field;
if (field.Name is "Empty" && field.ContainingType.HasName ("System.String"))
return new KnownStringValue (string.Empty);
return new FieldValue (fieldRef.Field);
}

return new FieldValue (fieldRef.Field);
return TopValue;
}

public override MultiValue VisitTypeOf (ITypeOfOperation typeOfOperation, StateValue state)
Expand All @@ -90,9 +114,34 @@ public override MultiValue VisitTypeOf (ITypeOfOperation typeOfOperation, StateV
return TopValue;
}

public override MultiValue VisitLiteral (ILiteralOperation literalOperation, StateValue state)
public override MultiValue VisitBinaryOperator (IBinaryOperation operation, StateValue argument)
{
return literalOperation.ConstantValue.Value == null ? NullValue.Instance : TopValue;
if (!operation.ConstantValue.HasValue && // Optimization - if there is already a constant value available, rely on the Visit(IOperation) instead
operation.OperatorKind == BinaryOperatorKind.Or &&
operation.OperatorMethod is null &&
(operation.Type?.TypeKind == TypeKind.Enum || operation.Type?.SpecialType == SpecialType.System_Int32)) {
MultiValue leftValue = Visit (operation.LeftOperand, argument);
MultiValue rightValue = Visit (operation.RightOperand, argument);

MultiValue result = TopValue;
foreach (var left in leftValue) {
if (left is UnknownValue)
result = _multiValueLattice.Meet (result, left);
else if (left is ConstIntValue leftConstInt) {
foreach (var right in rightValue) {
if (right is UnknownValue)
result = _multiValueLattice.Meet (result, right);
else if (right is ConstIntValue rightConstInt) {
result = _multiValueLattice.Meet (result, new ConstIntValue (leftConstInt.Value | rightConstInt.Value));
}
}
}
}

return result;
}

return base.VisitBinaryOperator (operation, argument);
}

// Override handlers for situations where annotated locations may be involved in reflection access:
Expand Down
Loading

0 comments on commit 8b9fae7

Please sign in to comment.