Skip to content

Commit

Permalink
Support running a basic analysis of generic instantiations created th…
Browse files Browse the repository at this point in the history 
…rough reflection. (dotnet#1955)

Enable a basic analysis for arrays of `System.Type` passed to `Type.MakeGenericType` and `MethodInfo.MakeGenericMethod`. This analysis works for arrays created within the method with a constant size where all array elements are known.

This allows for common usages of `Type.MakeGenericType` and `MethodInfo.MakeGenericMethod` that let the compiler implicitly create an array for them (since the parameter is `params`) to get basic analysis.

Also enable analysis for ldelem when it's a known array with a known value at a given known index.

In combination with dotnet#1930, we could also enable this validation for `Expression.Call` on generic types.
  • Loading branch information
@jkoritzinsky
jkoritzinsky authored Apr 19, 2021
1 parent 76bcd90 commit f6ccab5
Show file tree
Hide file tree
Showing 9 changed files with 424 additions and 107 deletions.
152 changes: 117 additions & 35 deletions src/linker/Linker.Dataflow/MethodBodyScanner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,33 +166,33 @@ public int MoveNext (Instruction op)
}
}

public struct ValueBasicBlockPair
{
public ValueNode Value;
public int BasicBlockIndex;
}

private static void StoreMethodLocalValue<KeyType> (
Dictionary<KeyType, ValueBasicBlockPair> valueCollection,
ValueNode valueToStore,
KeyType collectionKey,
int curBasicBlock)
int curBasicBlock,
int? maxTrackedValues = null)
{
ValueBasicBlockPair newValue = new ValueBasicBlockPair { BasicBlockIndex = curBasicBlock };

ValueBasicBlockPair existingValue;
if (valueCollection.TryGetValue (collectionKey, out existingValue)
&& existingValue.BasicBlockIndex == curBasicBlock) {
// If the previous value was stored in the current basic block, then we can safely
// overwrite the previous value with the new one.
if (valueCollection.TryGetValue (collectionKey, out existingValue)) {
if (existingValue.BasicBlockIndex == curBasicBlock) {
// If the previous value was stored in the current basic block, then we can safely
// overwrite the previous value with the new one.
newValue.Value = valueToStore;
} else {
// If the previous value came from a previous basic block, then some other use of
// the local could see the previous value, so we must merge the new value with the
// old value.
newValue.Value = MergePointValue.MergeValues (existingValue.Value, valueToStore);
}
valueCollection[collectionKey] = newValue;
} else if (maxTrackedValues == null || valueCollection.Count < maxTrackedValues) {
// We're not currently tracking a value a this index, so store the value now.
newValue.Value = valueToStore;
} else {
// If the previous value came from a previous basic block, then some other use of
// the local could see the previous value, so we must merge the new value with the
// old value.
newValue.Value = MergePointValue.MergeValues (existingValue.Value, valueToStore);
valueCollection[collectionKey] = newValue;
}
valueCollection[collectionKey] = newValue;
}

public void Scan (MethodBody methodBody)
Expand Down Expand Up @@ -246,22 +246,9 @@ public void Scan (MethodBody methodBody)
case Code.Cgt_Un:
case Code.Clt:
case Code.Clt_Un:
case Code.Ldelem_I:
case Code.Ldelem_I1:
case Code.Ldelem_I2:
case Code.Ldelem_I4:
case Code.Ldelem_I8:
case Code.Ldelem_R4:
case Code.Ldelem_R8:
case Code.Ldelem_U1:
case Code.Ldelem_U2:
case Code.Ldelem_U4:
case Code.Shl:
case Code.Shr:
case Code.Shr_Un:
case Code.Ldelem_Any:
case Code.Ldelem_Ref:
case Code.Ldelema:
case Code.Ceq:
PopUnknown (currentStack, 2, methodBody, operation.Offset);
PushUnknown (currentStack);
Expand Down Expand Up @@ -432,12 +419,10 @@ public void Scan (MethodBody methodBody)

case Code.Newarr: {
StackSlot count = PopUnknown (currentStack, 1, methodBody, operation.Offset);
currentStack.Push (new StackSlot (new ArrayValue (count.Value)));
currentStack.Push (new StackSlot (new ArrayValue (count.Value, (TypeReference) operation.Operand)));
}
break;

case Code.Cpblk:
case Code.Initblk:
case Code.Stelem_I:
case Code.Stelem_I1:
case Code.Stelem_I2:
Expand All @@ -447,6 +432,27 @@ public void Scan (MethodBody methodBody)
case Code.Stelem_R8:
case Code.Stelem_Any:
case Code.Stelem_Ref:
ScanStelem (operation, currentStack, methodBody, curBasicBlock);
break;

case Code.Ldelem_I:
case Code.Ldelem_I1:
case Code.Ldelem_I2:
case Code.Ldelem_I4:
case Code.Ldelem_I8:
case Code.Ldelem_R4:
case Code.Ldelem_R8:
case Code.Ldelem_U1:
case Code.Ldelem_U2:
case Code.Ldelem_U4:
case Code.Ldelem_Any:
case Code.Ldelem_Ref:
case Code.Ldelema:
ScanLdelem (operation, currentStack, methodBody, curBasicBlock);
break;

case Code.Cpblk:
case Code.Initblk:
PopUnknown (currentStack, 3, methodBody, operation.Offset);
break;

Expand Down Expand Up @@ -527,7 +533,7 @@ public void Scan (MethodBody methodBody)
case Code.Call:
case Code.Callvirt:
case Code.Newobj:
HandleCall (methodBody, operation, currentStack);
HandleCall (methodBody, operation, currentStack, curBasicBlock);
break;

case Code.Jmp:
Expand Down Expand Up @@ -852,7 +858,8 @@ private ValueNodeList PopCallArguments (
private void HandleCall (
MethodBody callingMethodBody,
Instruction operation,
Stack<StackSlot> currentStack)
Stack<StackSlot> currentStack,
int curBasicBlock)
{
MethodReference calledMethod = (MethodReference) operation.Operand;

Expand Down Expand Up @@ -886,6 +893,12 @@ private void HandleCall (

if (methodReturnValue != null)
currentStack.Push (new StackSlot (methodReturnValue, calledMethod.ReturnType.IsByRefOrPointer ()));

foreach (var param in methodParams) {
if (param is ArrayValue arr) {
MarkArrayValuesAsUnknown (arr, curBasicBlock);
}
}
}

public abstract bool HandleCall (
Expand All @@ -894,5 +907,74 @@ public abstract bool HandleCall (
Instruction operation,
ValueNodeList methodParams,
out ValueNode methodReturnValue);

// Limit tracking array values to 32 values for performance reasons. There are many arrays much longer than 32 elements in .NET, but the interesting ones for the linker are nearly always less than 32 elements.
private const int MaxTrackedArrayValues = 32;

private static void MarkArrayValuesAsUnknown (ArrayValue arrValue, int curBasicBlock)
{
// Since we can't know the current index we're storing the value at, clear all indices.
// That way we won't accidentally think we know the value at a given index when we cannot.
foreach (var knownIndex in arrValue.IndexValues.Keys) {
// Don't pass MaxTrackedArrayValues since we are only looking at keys we've already seen.
StoreMethodLocalValue (arrValue.IndexValues, UnknownValue.Instance, knownIndex, curBasicBlock);
}
}

private void ScanStelem (
Instruction operation,
Stack<StackSlot> currentStack,
MethodBody methodBody,
int curBasicBlock)
{
StackSlot valueToStore = PopUnknown (currentStack, 1, methodBody, operation.Offset);
StackSlot indexToStoreAt = PopUnknown (currentStack, 1, methodBody, operation.Offset);
StackSlot arrayToStoreIn = PopUnknown (currentStack, 1, methodBody, operation.Offset);
int? indexToStoreAtInt = indexToStoreAt.Value.AsConstInt ();
foreach (var array in arrayToStoreIn.Value.UniqueValues ()) {
if (array is ArrayValue arrValue) {
if (indexToStoreAtInt == null) {
MarkArrayValuesAsUnknown (arrValue, curBasicBlock);
} else {
// When we know the index, we can record the value at that index.
StoreMethodLocalValue (arrValue.IndexValues, valueToStore.Value, indexToStoreAtInt.Value, curBasicBlock, MaxTrackedArrayValues);
}
}
}
}

private void ScanLdelem (
Instruction operation,
Stack<StackSlot> currentStack,
MethodBody methodBody,
int curBasicBlock)
{
StackSlot indexToLoadFrom = PopUnknown (currentStack, 1, methodBody, operation.Offset);
StackSlot arrayToLoadFrom = PopUnknown (currentStack, 1, methodBody, operation.Offset);
if (arrayToLoadFrom.Value is not ArrayValue arr) {
PushUnknown (currentStack);
return;
}
bool isByRef = operation.OpCode.Code == Code.Ldelema;

int? index = indexToLoadFrom.Value.AsConstInt ();
if (index == null) {
PushUnknown (currentStack);
if (isByRef) {
MarkArrayValuesAsUnknown (arr, curBasicBlock);
}
return;
}


ValueBasicBlockPair arrayIndexValue;
arr.IndexValues.TryGetValue (index.Value, out arrayIndexValue);
if (arrayIndexValue.Value != null) {
ValueNode valueToPush = arrayIndexValue.Value;
currentStack.Push (new StackSlot (valueToPush, isByRef));
} else {
currentStack.Push (new StackSlot (null, isByRef));
}
}
}
}
102 changes: 72 additions & 30 deletions src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ protected override ValueNode GetFieldValue (MethodDefinition method, FieldDefini
{
switch (field.Name) {
case "EmptyTypes" when field.DeclaringType.IsTypeOf ("System", "Type"): {
return new ArrayValue (new ConstIntValue (0));
return new ArrayValue (new ConstIntValue (0), field.DeclaringType);
}
case "Empty" when field.DeclaringType.IsTypeOf ("System", "String"): {
return new KnownStringValue (string.Empty);
Expand Down Expand Up @@ -656,7 +656,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
break;

case IntrinsicId.Array_Empty: {
methodReturnValue = new ArrayValue (new ConstIntValue (0));
methodReturnValue = new ArrayValue (new ConstIntValue (0), ((GenericInstanceMethod) calledMethod).GenericArguments[0]);
}
break;

Expand Down Expand Up @@ -702,20 +702,28 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
reflectionContext.AnalyzingPattern ();
foreach (var value in methodParams[0].UniqueValues ()) {
if (value is SystemTypeValue typeValue) {
foreach (var genericParameter in typeValue.TypeRepresented.GenericParameters) {
if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None ||
(genericParameter.HasDefaultConstructorConstraint && !typeValue.TypeRepresented.IsTypeOf ("System", "Nullable`1"))) {
// There is a generic parameter which has some requirements on the input types.
// For now we don't support tracking actual array elements, so we can't validate that the requirements are fulfilled.

// Special case: Nullable<T> where T : struct
// The struct constraint in C# implies new() constraints, but Nullable doesn't make a use of that part.
// There are several places even in the framework where typeof(Nullable<>).MakeGenericType would warn
// without any good reason to do so.
if (AnalyzeGenericInstatiationTypeArray (methodParams[1], ref reflectionContext, calledMethodDefinition, typeValue.TypeRepresented.GenericParameters)) {
reflectionContext.RecordHandledPattern ();
} else {
bool hasUncheckedAnnotation = false;
foreach (var genericParameter in typeValue.TypeRepresented.GenericParameters) {
if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None ||
(genericParameter.HasDefaultConstructorConstraint && !typeValue.TypeRepresented.IsTypeOf ("System", "Nullable`1"))) {
// If we failed to analyze the array, we go through the analyses again
// and intentionally ignore one particular annotation:
// Special case: Nullable<T> where T : struct
// The struct constraint in C# implies new() constraints, but Nullable doesn't make a use of that part.
// There are several places even in the framework where typeof(Nullable<>).MakeGenericType would warn
// without any good reason to do so.
hasUncheckedAnnotation = true;
break;
}
}
if (hasUncheckedAnnotation) {
reflectionContext.RecordUnrecognizedPattern (
2055,
$"Call to '{calledMethodDefinition.GetDisplayName ()}' can not be statically analyzed. " +
$"It's not possible to guarantee the availability of requirements of the generic type.");
2055,
$"Call to '{calledMethodDefinition.GetDisplayName ()}' can not be statically analyzed. " +
$"It's not possible to guarantee the availability of requirements of the generic type.");
}
}

Expand Down Expand Up @@ -811,7 +819,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
foreach (var stringParam in methodParams[1].UniqueValues ()) {
if (stringParam is KnownStringValue stringValue) {
foreach (var method in systemTypeValue.TypeRepresented.GetMethodsOnTypeHierarchy (m => m.Name == stringValue.Contents, bindingFlags)) {
ValidateGenericMethodInstantiation (ref reflectionContext, method, calledMethod);
ValidateGenericMethodInstantiation (ref reflectionContext, method, methodParams[2], calledMethod);
MarkMethod (ref reflectionContext, method);
}

Expand Down Expand Up @@ -1630,8 +1638,7 @@ methodParams[argsParam] is ArrayValue arrayValue &&

foreach (var methodValue in methodParams[0].UniqueValues ()) {
if (methodValue is SystemReflectionMethodBaseValue methodBaseValue) {
ValidateGenericMethodInstantiation (ref reflectionContext, methodBaseValue.MethodRepresented, calledMethod);
reflectionContext.RecordHandledPattern ();
ValidateGenericMethodInstantiation (ref reflectionContext, methodBaseValue.MethodRepresented, methodParams[1], calledMethod);
} else if (methodValue == NullValue.Instance) {
reflectionContext.RecordHandledPattern ();
} else {
Expand Down Expand Up @@ -1712,6 +1719,42 @@ methodParams[argsParam] is ArrayValue arrayValue &&
return true;
}

private bool AnalyzeGenericInstatiationTypeArray (ValueNode arrayParam, ref ReflectionPatternContext reflectionContext, MethodReference calledMethod, IList<GenericParameter> genericParameters)
{
foreach (var typesValue in arrayParam.UniqueValues ()) {
if (typesValue.Kind != ValueNodeKind.Array) {
return false;
}
ArrayValue array = (ArrayValue) typesValue;
int? size = array.Size.AsConstInt ();
if (size == null || size != genericParameters.Count) {
return false;
}
bool allIndicesKnown = true;
for (int i = 0; i < size.Value; i++) {
if (!array.IndexValues.TryGetValue (i, out ValueBasicBlockPair value) || value.Value is null or { Kind: ValueNodeKind.Unknown }) {
allIndicesKnown = false;
break;
}
}

if (!allIndicesKnown) {
return false;
}

for (int i = 0; i < size.Value; i++) {
if (array.IndexValues.TryGetValue (i, out ValueBasicBlockPair value)) {
RequireDynamicallyAccessedMembers (
ref reflectionContext,
_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameters[i]),
value.Value,
calledMethod.Resolve ());
}
}
}
return true;
}

void ProcessCreateInstanceByName (ref ReflectionPatternContext reflectionContext, MethodDefinition calledMethod, ValueNodeList methodParams)
{
reflectionContext.AnalyzingPattern ();
Expand Down Expand Up @@ -2176,20 +2219,19 @@ void MarkEventsOnTypeHierarchy (ref ReflectionPatternContext reflectionContext,
void ValidateGenericMethodInstantiation (
ref ReflectionPatternContext reflectionContext,
MethodDefinition genericMethod,
ValueNode genericParametersArray,
MethodReference reflectionMethod)
{
if (!genericMethod.HasGenericParameters)
return;

foreach (var genericParameter in genericMethod.GenericParameters) {
if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None ||
genericParameter.HasDefaultConstructorConstraint) {
// There is a generic parameter which has some requirements on input types.
// For now we don't support tracking actual array elements, so we can't validate that the requirements are fulfilled.
reflectionContext.RecordUnrecognizedPattern (
2060, string.Format (Resources.Strings.IL2060,
DiagnosticUtilities.GetMethodSignatureDisplayName (reflectionMethod)));
}
if (!genericMethod.HasGenericParameters) {
reflectionContext.RecordHandledPattern ();
}

if (!AnalyzeGenericInstatiationTypeArray (genericParametersArray, ref reflectionContext, reflectionMethod, genericMethod.GenericParameters)) {
reflectionContext.RecordUnrecognizedPattern (
2060,
string.Format (Resources.Strings.IL2060, DiagnosticUtilities.GetMethodSignatureDisplayName (reflectionMethod)));
} else {
reflectionContext.RecordHandledPattern ();
}
}

Expand Down
Loading

0 comments on commit f6ccab5

Please sign in to comment.