Skip to content

Commit

Permalink
Implement data flow annotations on generic parameters (dotnet#1245)
Browse files Browse the repository at this point in the history 
* Implement data flow annotations on generic parameters

The checks are done when linker calls MarkType(TypeReference) or MarkMethod(MethodReference). The check itself is straightforward as it uses existing functionality.

Added annotations in FlowAnnotations for generic parameters.

Added one parameter to MarkType to better track the location where the type reference comes from. This enables much more precise reporting of warnings.

On test side this makes the recognized/unrecognized attributes applicable to many more things as warnings can come from types, fields and so on.

* Recognize MakeGenericType/Method and warn on them

Data flow on generic parameters makes any generic instantiation potentially problematic. So we need to start marking MakeGenericType and MakeGenericMethod as problematic as well.

MakeGenericType is possible to only warn when there are generic parameters with annotations. For MakeGenericMethod we can't do anything special as we don't track MethodInfo instances.
  • Loading branch information
@vitek-karas
vitek-karas authored Jun 16, 2020
1 parent fd182b0 commit c75e881
Show file tree
Hide file tree
Showing 10 changed files with 1,439 additions and 327 deletions.
110 changes: 101 additions & 9 deletions src/linker/Linker.Dataflow/FlowAnnotations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public bool RequiresDataFlowAnalysis (FieldDefinition field)
return GetAnnotations (field.DeclaringType).TryGetAnnotation (field, out _);
}

public bool RequiresDataFlowAnalysis (GenericParameter genericParameter)
{
return GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None;
}

/// <summary>
/// Retrieves the annotations for the given parameter.
/// </summary>
Expand Down Expand Up @@ -63,6 +68,24 @@ public DynamicallyAccessedMemberTypes GetFieldAnnotation (FieldDefinition field)
return DynamicallyAccessedMemberTypes.None;
}

public DynamicallyAccessedMemberTypes GetGenericParameterAnnotation (GenericParameter genericParameter)
{
TypeDefinition declaringType = genericParameter.DeclaringType?.Resolve ();
if (declaringType != null) {
if (GetAnnotations (declaringType).TryGetAnnotation (genericParameter, out var annotation))
return annotation.Annotation;

return DynamicallyAccessedMemberTypes.None;
}

MethodDefinition declaringMethod = genericParameter.DeclaringMethod?.Resolve ();
if (declaringMethod != null && GetAnnotations (declaringMethod.DeclaringType).TryGetAnnotation (declaringMethod, out var methodTypeAnnotations) &&
methodTypeAnnotations.TryGetAnnotation (genericParameter, out var methodAnnotation))
return methodAnnotation.Annotation;

return DynamicallyAccessedMemberTypes.None;
}

TypeAnnotations GetAnnotations (TypeDefinition type)
{
if (!_annotations.TryGetValue (type, out TypeAnnotations value)) {
Expand Down Expand Up @@ -166,8 +189,18 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)

DynamicallyAccessedMemberTypes returnAnnotation = IsTypeInterestingForDataflow (method.ReturnType) ?
GetMemberTypesForDynamicallyAccessedMemberAttribute (method.MethodReturnType, method) : DynamicallyAccessedMemberTypes.None;
if (returnAnnotation != DynamicallyAccessedMemberTypes.None || paramAnnotations != null) {
annotatedMethods.Add (new MethodAnnotations (method, paramAnnotations, returnAnnotation));

var annotatedMethodGenericParameters = new ArrayBuilder<GenericParameterAnnotation> ();
if (method.HasGenericParameters) {
foreach (var genericParameter in method.GenericParameters) {
var annotation = GetMemberTypesForDynamicallyAccessedMemberAttribute (genericParameter, method);
if (annotation != DynamicallyAccessedMemberTypes.None)
annotatedMethodGenericParameters.Add (new GenericParameterAnnotation (genericParameter, annotation));
}
}

if (returnAnnotation != DynamicallyAccessedMemberTypes.None || paramAnnotations != null || annotatedMethodGenericParameters.Count > 0) {
annotatedMethods.Add (new MethodAnnotations (method, paramAnnotations, returnAnnotation, annotatedMethodGenericParameters.ToArray ()));
}
}
}
Expand Down Expand Up @@ -220,7 +253,7 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)
if (setMethod.Parameters.Count > 0) {
DynamicallyAccessedMemberTypes[] paramAnnotations = new DynamicallyAccessedMemberTypes[setMethod.Parameters.Count + offset];
paramAnnotations[offset] = annotation;
annotatedMethods.Add (new MethodAnnotations (setMethod, paramAnnotations, DynamicallyAccessedMemberTypes.None));
annotatedMethods.Add (new MethodAnnotations (setMethod, paramAnnotations, DynamicallyAccessedMemberTypes.None, null));
}
}
}
Expand All @@ -243,7 +276,7 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)
if (annotatedMethods.Any (a => a.Method == getMethod)) {
_context.LogWarning ($"Trying to propagate DynamicallyAccessedMemberAttribute from property '{property.FullName}' to its getter '{getMethod}', but it already has such attribute on the return value.", 2043, getMethod);
} else {
annotatedMethods.Add (new MethodAnnotations (getMethod, null, annotation));
annotatedMethods.Add (new MethodAnnotations (getMethod, null, annotation, null));
}
}

Expand All @@ -266,7 +299,16 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)
}
}

return new TypeAnnotations (annotatedMethods.ToArray (), annotatedFields.ToArray ());
var annotatedGenericParameters = new ArrayBuilder<GenericParameterAnnotation> ();
if (type.HasGenericParameters) {
foreach (var genericParameter in type.GenericParameters) {
var annotation = GetMemberTypesForDynamicallyAccessedMemberAttribute (genericParameter, type);
if (annotation != DynamicallyAccessedMemberTypes.None)
annotatedGenericParameters.Add (new GenericParameterAnnotation (genericParameter, annotation));
}
}

return new TypeAnnotations (annotatedMethods.ToArray (), annotatedFields.ToArray (), annotatedGenericParameters.ToArray ());
}

bool ScanMethodBodyForFieldAccess (MethodBody body, bool write, out FieldDefinition found)
Expand Down Expand Up @@ -338,9 +380,10 @@ readonly struct TypeAnnotations
{
readonly MethodAnnotations[] _annotatedMethods;
readonly FieldAnnotation[] _annotatedFields;
readonly GenericParameterAnnotation[] _annotatedGenericParameters;

public TypeAnnotations (MethodAnnotations[] annotatedMethods, FieldAnnotation[] annotatedFields)
=> (_annotatedMethods, _annotatedFields) = (annotatedMethods, annotatedFields);
public TypeAnnotations (MethodAnnotations[] annotatedMethods, FieldAnnotation[] annotatedFields, GenericParameterAnnotation[] annotatedGenericParameters)
=> (_annotatedMethods, _annotatedFields, _annotatedGenericParameters) = (annotatedMethods, annotatedFields, annotatedGenericParameters);

public bool TryGetAnnotation (MethodDefinition method, out MethodAnnotations annotations)
{
Expand Down Expand Up @@ -377,16 +420,56 @@ public bool TryGetAnnotation (FieldDefinition field, out FieldAnnotation annotat

return false;
}

public bool TryGetAnnotation (GenericParameter genericParameter, out GenericParameterAnnotation annotation)
{
annotation = default;

if (_annotatedGenericParameters == null)
return false;

foreach (var g in _annotatedGenericParameters) {
if (g.GenericParameter == genericParameter) {
annotation = g;
return true;
}
}

return false;
}
}

readonly struct MethodAnnotations
{
public readonly MethodDefinition Method;
public readonly DynamicallyAccessedMemberTypes[] ParameterAnnotations;
public readonly DynamicallyAccessedMemberTypes ReturnParameterAnnotation;
readonly GenericParameterAnnotation[] _annotatedGenericParameters;

public MethodAnnotations (
MethodDefinition method,
DynamicallyAccessedMemberTypes[] paramAnnotations,
DynamicallyAccessedMemberTypes returnParamAnnotations,
GenericParameterAnnotation[] annotatedGenericParameters)
=> (Method, ParameterAnnotations, ReturnParameterAnnotation, _annotatedGenericParameters) =
(method, paramAnnotations, returnParamAnnotations, annotatedGenericParameters);

public bool TryGetAnnotation (GenericParameter genericParameter, out GenericParameterAnnotation annotation)
{
annotation = default;

if (_annotatedGenericParameters == null)
return false;

foreach (var g in _annotatedGenericParameters) {
if (g.GenericParameter == genericParameter) {
annotation = g;
return true;
}
}

public MethodAnnotations (MethodDefinition method, DynamicallyAccessedMemberTypes[] paramAnnotations, DynamicallyAccessedMemberTypes returnParamAnnotations)
=> (Method, ParameterAnnotations, ReturnParameterAnnotation) = (method, paramAnnotations, returnParamAnnotations);
return false;
}
}

readonly struct FieldAnnotation
Expand All @@ -397,5 +480,14 @@ readonly struct FieldAnnotation
public FieldAnnotation (FieldDefinition field, DynamicallyAccessedMemberTypes annotation)
=> (Field, Annotation) = (field, annotation);
}

readonly struct GenericParameterAnnotation
{
public readonly GenericParameter GenericParameter;
public readonly DynamicallyAccessedMemberTypes Annotation;

public GenericParameterAnnotation (GenericParameter genericParameter, DynamicallyAccessedMemberTypes annotation)
=> (GenericParameter, Annotation) = (genericParameter, annotation);
}
}
}
14 changes: 10 additions & 4 deletions src/linker/Linker.Dataflow/MethodBodyScanner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Collections.Generic;
using System;
using System.Collections.Generic;
using System.Linq;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Collections.Generic;

namespace Mono.Linker.Dataflow
{
Expand Down Expand Up @@ -693,6 +693,12 @@ private void ScanLdtoken (
MethodDefinition thisMethod,
MethodBody methodBody)
{
if (operation.Operand is GenericParameter genericParameter) {
StackSlot slot = new StackSlot (new RuntimeTypeHandleForGenericParameterValue (genericParameter));
currentStack.Push (slot);
return;
}

if (operation.Operand is TypeReference typeReference) {
var resolvedReference = typeReference.Resolve ();
if (resolvedReference != null) {
Expand Down Expand Up @@ -838,7 +844,7 @@ private void HandleCall (

ValueNode newObjValue = null;
ValueNodeList methodParams = PopCallArguments (currentStack, calledMethod, callingMethodBody, isNewObj,
operation.Offset, out newObjValue);
operation.Offset, out newObjValue);

ValueNode methodReturnValue = null;
bool handledFunction = HandleCall (
Expand Down
Loading

0 comments on commit c75e881

Please sign in to comment.