diff options
19 files changed, 334 insertions, 91 deletions
diff --git a/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs b/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs index 3cb60e631..d42daee81 100644 --- a/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs +++ b/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs @@ -57,6 +57,14 @@ namespace ILLink.Shared.TypeSystemProxy return namedType.HasName (fullTypeName); } + public ReferenceKind ParameterReferenceKind (int index) + => Method.Parameters[index].RefKind switch { + RefKind.In => ReferenceKind.In, + RefKind.Out => ReferenceKind.Out, + RefKind.Ref => ReferenceKind.Ref, + _ => ReferenceKind.None + }; + public override string ToString () => Method.ToString (); } } diff --git a/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs b/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs index 10d3ba9ed..60b41e750 100644 --- a/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs +++ b/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs @@ -1151,6 +1151,8 @@ namespace ILLink.Shared.TrimAnalysis _requireDynamicallyAccessedMembersAction.Invoke (instanceValue, _annotations.GetMethodThisParameterValue (calledMethod)); } for (int argumentIndex = 0; argumentIndex < argumentValues.Count; argumentIndex++) { + if (calledMethod.ParameterReferenceKind (argumentIndex) == ReferenceKind.Out) + continue; _requireDynamicallyAccessedMembersAction.Invoke (argumentValues[argumentIndex], _annotations.GetMethodParameterValue (calledMethod, argumentIndex)); } } diff --git a/src/ILLink.Shared/TrimAnalysis/ReferenceKind.cs b/src/ILLink.Shared/TrimAnalysis/ReferenceKind.cs new file mode 100644 index 000000000..1c46b2dbe --- /dev/null +++ b/src/ILLink.Shared/TrimAnalysis/ReferenceKind.cs @@ -0,0 +1,14 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + + +namespace ILLink.Shared.TypeSystemProxy +{ + public enum ReferenceKind + { + Ref, + In, + Out, + None + } +}
\ No newline at end of file diff --git a/src/linker/Linker.Dataflow/FieldReferenceValue.cs b/src/linker/Linker.Dataflow/FieldReferenceValue.cs new file mode 100644 index 000000000..de0f23541 --- /dev/null +++ b/src/linker/Linker.Dataflow/FieldReferenceValue.cs @@ -0,0 +1,12 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +using ILLink.Shared.DataFlow; +using Mono.Cecil; + +namespace ILLink.Shared.TrimAnalysis +{ + public partial record FieldReferenceValue (FieldDefinition FieldDefinition) : ReferenceValue + { + public override SingleValue DeepCopy () => this; + } +} diff --git a/src/linker/Linker.Dataflow/LocalVariableReferenceValue.cs b/src/linker/Linker.Dataflow/LocalVariableReferenceValue.cs new file mode 100644 index 000000000..92365210a --- /dev/null +++ b/src/linker/Linker.Dataflow/LocalVariableReferenceValue.cs @@ -0,0 +1,15 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +using ILLink.Shared.DataFlow; +using Mono.Cecil.Cil; + +namespace ILLink.Shared.TrimAnalysis +{ + public partial record LocalVariableReferenceValue (VariableDefinition LocalDefinition) : ReferenceValue + { + public override SingleValue DeepCopy () + { + return this; + } + } +} diff --git a/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/linker/Linker.Dataflow/MethodBodyScanner.cs index b50de8978..6c2f0bbf4 100644 --- a/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -3,13 +3,16 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; +using ILLink.Shared; using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using ILLink.Shared.TypeSystemProxy; using Mono.Cecil; using Mono.Cecil.Cil; using Mono.Collections.Generic; +using LocalVariableStore = System.Collections.Generic.Dictionary<Mono.Cecil.Cil.VariableDefinition, Mono.Linker.Dataflow.ValueBasicBlockPair>; using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>; namespace Mono.Linker.Dataflow @@ -21,27 +24,19 @@ namespace Mono.Linker.Dataflow { public MultiValue Value { get; } - /// <summary> - /// True if the value is on the stack as a byref - /// </summary> - public bool IsByRef { get; } - public StackSlot () { Value = new MultiValue (UnknownValue.Instance); - IsByRef = false; } - public StackSlot (SingleValue value, bool isByRef = false) + public StackSlot (SingleValue value) { Value = new MultiValue (value); - IsByRef = isByRef; } - public StackSlot (MultiValue value, bool isByRef = false) + public StackSlot (MultiValue value) { Value = value; - IsByRef = isByRef; } } @@ -180,7 +175,25 @@ namespace Mono.Linker.Dataflow } } - private static void StoreMethodLocalValue<KeyType> ( + [Conditional ("DEBUG")] + static void ValidateNoReferenceToReference (LocalVariableStore locals, MethodDefinition method, int ilOffset) + { + foreach (var keyValuePair in locals) { + MultiValue localValue = keyValuePair.Value.Value; + VariableDefinition localVariable = keyValuePair.Key; + foreach (var val in localValue) { + if (val is LocalVariableReferenceValue reference + && locals[reference.LocalDefinition].Value.Any (v => v is ReferenceValue)) { + throw new LinkerFatalErrorException (MessageContainer.CreateCustomErrorMessage ( + $"In method {method.FullName}, local variable {localVariable.Index} references variable {reference.LocalDefinition.Index} which is a reference.", + (int) DiagnosticId.LinkerUnexpectedError, + origin: new MessageOrigin (method, ilOffset))); + } + } + } + } + + protected static void StoreMethodLocalValue<KeyType> ( Dictionary<KeyType, ValueBasicBlockPair> valueCollection, in MultiValue valueToStore, KeyType collectionKey, @@ -211,7 +224,7 @@ namespace Mono.Linker.Dataflow { MethodDefinition thisMethod = methodBody.Method; - Dictionary<VariableDefinition, ValueBasicBlockPair> locals = new Dictionary<VariableDefinition, ValueBasicBlockPair> (methodBody.Variables.Count); + LocalVariableStore locals = new (methodBody.Variables.Count); Dictionary<int, Stack<StackSlot>> knownStacks = new Dictionary<int, Stack<StackSlot>> (); Stack<StackSlot>? currentStack = new Stack<StackSlot> (methodBody.MaxStackSize); @@ -222,6 +235,7 @@ namespace Mono.Linker.Dataflow ReturnValue = new (); foreach (Instruction operation in methodBody.Instructions) { + ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset); int curBasicBlock = blockIterator.MoveNext (operation); if (knownStacks.ContainsKey (operation.Offset)) { @@ -486,7 +500,7 @@ namespace Mono.Linker.Dataflow case Code.Stind_R8: case Code.Stind_Ref: case Code.Stobj: - ScanIndirectStore (operation, currentStack, methodBody); + ScanIndirectStore (operation, currentStack, methodBody, locals, curBasicBlock); break; case Code.Initobj: @@ -545,7 +559,7 @@ namespace Mono.Linker.Dataflow case Code.Call: case Code.Callvirt: case Code.Newobj: - HandleCall (methodBody, operation, currentStack, curBasicBlock); + HandleCall (methodBody, operation, currentStack, locals, curBasicBlock); break; case Code.Jmp: @@ -580,7 +594,9 @@ namespace Mono.Linker.Dataflow } if (hasReturnValue) { StackSlot retValue = PopUnknown (currentStack, 1, methodBody, operation.Offset); - ReturnValue = MultiValueLattice.Meet (ReturnValue, retValue.Value); + // If the return value is a reference, treat it as the value itself for now + // We can handle ref return values better later + ReturnValue = MultiValueLattice.Meet (ReturnValue, DereferenceValue (retValue.Value, locals)); } ClearStack (ref currentStack); break; @@ -675,12 +691,16 @@ namespace Mono.Linker.Dataflow paramNum = paramDefinition.Index; } + // This is semantically wrong if it returns true - we would representing a reference parameter as a reference to a parameter - but it should be fine for now isByRef = paramDefinition.ParameterType.IsByRefOrPointer (); } isByRef |= code == Code.Ldarga || code == Code.Ldarga_S; - StackSlot slot = new StackSlot (GetMethodParameterValue (thisMethod, paramNum), isByRef); + StackSlot slot = new StackSlot ( + isByRef + ? new ParameterReferenceValue (thisMethod, paramNum) + : GetMethodParameterValue (thisMethod, paramNum)); currentStack.Push (slot); } @@ -703,7 +723,7 @@ namespace Mono.Linker.Dataflow Instruction operation, Stack<StackSlot> currentStack, MethodBody methodBody, - Dictionary<VariableDefinition, ValueBasicBlockPair> locals) + LocalVariableStore locals) { VariableDefinition localDef = GetLocalDef (operation, methodBody.Variables); if (localDef == null) { @@ -711,13 +731,16 @@ namespace Mono.Linker.Dataflow return; } - bool isByRef = operation.OpCode.Code == Code.Ldloca || operation.OpCode.Code == Code.Ldloca_S - || localDef.VariableType.IsByRefOrPointer (); + bool isByRef = operation.OpCode.Code == Code.Ldloca || operation.OpCode.Code == Code.Ldloca_S; - if (!locals.TryGetValue (localDef, out ValueBasicBlockPair localValue)) - currentStack.Push (new StackSlot (UnknownValue.Instance, isByRef)); + StackSlot newSlot; + if (isByRef) { + newSlot = new StackSlot (new LocalVariableReferenceValue (localDef)); + } else if (locals.TryGetValue (localDef, out ValueBasicBlockPair localValue)) + newSlot = new StackSlot (localValue.Value); else - currentStack.Push (new StackSlot (localValue.Value, isByRef)); + newSlot = new StackSlot (UnknownValue.Instance); + currentStack.Push (newSlot); } void ScanLdtoken (Instruction operation, Stack<StackSlot> currentStack) @@ -763,7 +786,7 @@ namespace Mono.Linker.Dataflow Instruction operation, Stack<StackSlot> currentStack, MethodBody methodBody, - Dictionary<VariableDefinition, ValueBasicBlockPair> locals, + LocalVariableStore locals, int curBasicBlock) { StackSlot valueToStore = PopUnknown (currentStack, 1, methodBody, operation.Offset); @@ -779,16 +802,55 @@ namespace Mono.Linker.Dataflow private void ScanIndirectStore ( Instruction operation, Stack<StackSlot> currentStack, - MethodBody methodBody) + MethodBody methodBody, + LocalVariableStore locals, + int curBasicBlock) { StackSlot valueToStore = PopUnknown (currentStack, 1, methodBody, operation.Offset); StackSlot destination = PopUnknown (currentStack, 1, methodBody, operation.Offset); - foreach (var uniqueDestination in destination.Value) { - if (uniqueDestination is FieldValue fieldDestination) { - HandleStoreField (methodBody.Method, fieldDestination, operation, valueToStore.Value); - } else if (uniqueDestination is MethodParameterValue parameterDestination) { - HandleStoreParameter (methodBody.Method, parameterDestination, operation, valueToStore.Value); + StoreInReference (destination.Value, valueToStore.Value, methodBody.Method, operation, locals, curBasicBlock); + } + + /// <summary> + /// Handles storing the source value in a target <see cref="ReferenceValue"/> or MultiValue of ReferenceValues. + /// </summary> + /// <param name="target">A set of <see cref="ReferenceValue"/> that a value is being stored into</param> + /// <param name="source">The value to store</param> + /// <param name="method">The method body that contains the operation causing the store</param> + /// <param name="operation">The instruction causing the store</param> + /// <exception cref="LinkerFatalErrorException">Throws if <paramref name="target"/> is not a valid target for an indirect store.</exception> + protected void StoreInReference (MultiValue target, MultiValue source, MethodDefinition method, Instruction operation, LocalVariableStore locals, int curBasicBlock) + { + foreach (var value in target) { + switch (value) { + case LocalVariableReferenceValue localReference: + StoreMethodLocalValue (locals, source, localReference.LocalDefinition, curBasicBlock); + break; + case FieldReferenceValue fieldReference + when GetFieldValue (fieldReference.FieldDefinition).AsSingleValue () is FieldValue fieldValue: + HandleStoreField (method, fieldValue, operation, source); + break; + case ParameterReferenceValue parameterReference + when GetMethodParameterValue (parameterReference.MethodDefinition, parameterReference.ParameterIndex) is MethodParameterValue parameterValue: + HandleStoreParameter (method, parameterValue, operation, source); + break; + case ParameterReferenceValue parameterReference + when GetMethodParameterValue (parameterReference.MethodDefinition, parameterReference.ParameterIndex) is MethodThisParameterValue thisParameterValue: + HandleStoreMethodThisParameter (method, thisParameterValue, operation, source); + break; + case MethodReturnValue methodReturnValue: + // Ref returns don't have special ReferenceValue values, so assume if the target here is a MethodReturnValue then it must be a ref return value + HandleStoreMethodReturnValue (method, methodReturnValue, operation, source); + break; + case UnknownValue: + // These cases should only be refs to array elements. + break; + default: + throw new LinkerFatalErrorException (MessageContainer.CreateErrorMessage ( + $"Unhandled StoreReference call. Unhandled attempt to store a value in {value} of type {value.GetType ()}.", + (int) DiagnosticId.LinkerUnexpectedError, + origin: new MessageOrigin (method, operation.Offset))); } } @@ -809,7 +871,10 @@ namespace Mono.Linker.Dataflow FieldDefinition? field = _context.TryResolve ((FieldReference) operation.Operand); if (field != null) { - StackSlot slot = new StackSlot (GetFieldValue (field), isByRef); + MultiValue newValue = isByRef ? + new FieldReferenceValue (field) + : GetFieldValue (field); + StackSlot slot = new (newValue); currentStack.Push (slot); return; } @@ -825,6 +890,14 @@ namespace Mono.Linker.Dataflow { } + protected virtual void HandleStoreMethodThisParameter (MethodDefinition method, MethodThisParameterValue thisParameter, Instruction operation, MultiValue sourceValue) + { + } + + protected virtual void HandleStoreMethodReturnValue (MethodDefinition method, MethodReturnValue thisParameter, Instruction operation, MultiValue sourceValue) + { + } + private void ScanStfld ( Instruction operation, Stack<StackSlot> currentStack, @@ -887,10 +960,67 @@ namespace Mono.Linker.Dataflow return methodParams; } + internal MultiValue DereferenceValue (MultiValue maybeReferenceValue, Dictionary<VariableDefinition, ValueBasicBlockPair> locals) + { + MultiValue dereferencedValue = MultiValueLattice.Top; + foreach (var value in maybeReferenceValue) { + switch (value) { + case FieldReferenceValue fieldReferenceValue: + dereferencedValue = MultiValue.Meet ( + dereferencedValue, + GetFieldValue (fieldReferenceValue.FieldDefinition)); + break; + case ParameterReferenceValue parameterReferenceValue: + dereferencedValue = MultiValue.Meet ( + dereferencedValue, + GetMethodParameterValue (parameterReferenceValue.MethodDefinition, parameterReferenceValue.ParameterIndex)); + break; + case LocalVariableReferenceValue localVariableReferenceValue: + if (locals.TryGetValue (localVariableReferenceValue.LocalDefinition, out var valueBasicBlockPair)) + dereferencedValue = MultiValue.Meet (dereferencedValue, valueBasicBlockPair.Value); + else + dereferencedValue = MultiValue.Meet (dereferencedValue, UnknownValue.Instance); + break; + case ReferenceValue referenceValue: + throw new NotImplementedException ($"Unhandled dereference of ReferenceValue of type {referenceValue.GetType ().FullName}"); + default: + dereferencedValue = MultiValue.Meet (dereferencedValue, value); + break; + } + } + return dereferencedValue; + } + + /// <summary> + /// Assigns a MethodParameterValue to the location of each parameter passed by reference. (i.e. assigns the value to x when passing `ref x` as a parameter) + /// </summary> + protected void AssignRefAndOutParameters ( + MethodBody callingMethodBody, + MethodReference calledMethod, + ValueNodeList methodArguments, + Instruction operation, + LocalVariableStore locals, + int curBasicBlock) + { + MethodDefinition? calledMethodDefinition = _context.Resolve (calledMethod); + bool methodIsResolved = calledMethodDefinition is not null; + int offset = calledMethod.HasImplicitThis () ? 1 : 0; + int parameterIndex = 0; + for (int ilArgumentIndex = offset; ilArgumentIndex < methodArguments.Count; ilArgumentIndex++, parameterIndex++) { + if (calledMethod.ParameterReferenceKind (ilArgumentIndex) is not (ReferenceKind.Ref or ReferenceKind.Out)) + continue; + SingleValue newByRefValue = methodIsResolved + ? _context.Annotations.FlowAnnotations.GetMethodParameterValue (calledMethodDefinition!, parameterIndex) + : UnknownValue.Instance; + StoreInReference (methodArguments[ilArgumentIndex], newByRefValue, callingMethodBody.Method, operation, locals, curBasicBlock); + } + } + private void HandleCall ( MethodBody callingMethodBody, Instruction operation, Stack<StackSlot> currentStack, + LocalVariableStore locals, int curBasicBlock) { MethodReference calledMethod = (MethodReference) operation.Operand; @@ -898,15 +1028,15 @@ namespace Mono.Linker.Dataflow bool isNewObj = operation.OpCode.Code == Code.Newobj; SingleValue? newObjValue; - ValueNodeList methodParams = PopCallArguments (currentStack, calledMethod, callingMethodBody, isNewObj, + ValueNodeList methodArguments = PopCallArguments (currentStack, calledMethod, callingMethodBody, isNewObj, operation.Offset, out newObjValue); - + ValueNodeList dereferencedMethodParams = new (methodArguments.Select (param => DereferenceValue (param, locals)).ToList ()); MultiValue methodReturnValue; bool handledFunction = HandleCall ( callingMethodBody, calledMethod, operation, - methodParams, + dereferencedMethodParams, out methodReturnValue); // Handle the return value or newobj result @@ -924,9 +1054,11 @@ namespace Mono.Linker.Dataflow } if (isNewObj || !calledMethod.ReturnsVoid ()) - currentStack.Push (new StackSlot (methodReturnValue, calledMethod.ReturnType.IsByRefOrPointer ())); + currentStack.Push (new StackSlot (methodReturnValue)); + + AssignRefAndOutParameters (callingMethodBody, calledMethod, methodArguments, operation, locals, curBasicBlock); - foreach (var param in methodParams) { + foreach (var param in methodArguments) { foreach (var v in param) { if (v is ArrayValue arr) { MarkArrayValuesAsUnknown (arr, curBasicBlock); @@ -991,6 +1123,7 @@ namespace Mono.Linker.Dataflow PushUnknown (currentStack); return; } + // We don't yet handle arrays of references or pointers bool isByRef = operation.OpCode.Code == Code.Ldelema; int? index = indexToLoadFrom.Value.AsConstInt (); @@ -999,11 +1132,13 @@ namespace Mono.Linker.Dataflow if (isByRef) { MarkArrayValuesAsUnknown (arr, curBasicBlock); } - return; } - - if (arr.IndexValues.TryGetValue (index.Value, out ValueBasicBlockPair arrayIndexValue)) - currentStack.Push (new StackSlot (arrayIndexValue.Value, isByRef)); + // Don't try to track refs to array elements. Set it as unknown, then push unknown to the stack + else if (isByRef) { + arr.IndexValues[index.Value] = new ValueBasicBlockPair (UnknownValue.Instance, curBasicBlock); + PushUnknown (currentStack); + } else if (arr.IndexValues.TryGetValue (index.Value, out ValueBasicBlockPair arrayIndexValue)) + currentStack.Push (new StackSlot (arrayIndexValue.Value)); else PushUnknown (currentStack); } diff --git a/src/linker/Linker.Dataflow/MethodProxy.cs b/src/linker/Linker.Dataflow/MethodProxy.cs index 6180b2bdb..a903f5b57 100644 --- a/src/linker/Linker.Dataflow/MethodProxy.cs +++ b/src/linker/Linker.Dataflow/MethodProxy.cs @@ -51,5 +51,7 @@ namespace ILLink.Shared.TypeSystemProxy internal partial bool ReturnsVoid () => Method.ReturnsVoid (); public override string ToString () => Method.ToString (); + + public ReferenceKind ParameterReferenceKind (int index) => Method.ParameterReferenceKind (index); } } diff --git a/src/linker/Linker.Dataflow/ParameterReferenceValue.cs b/src/linker/Linker.Dataflow/ParameterReferenceValue.cs new file mode 100644 index 000000000..7f3804791 --- /dev/null +++ b/src/linker/Linker.Dataflow/ParameterReferenceValue.cs @@ -0,0 +1,16 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +using ILLink.Shared.DataFlow; +using Mono.Cecil; + +namespace ILLink.Shared.TrimAnalysis +{ + public partial record ParameterReferenceValue (MethodDefinition MethodDefinition, int ParameterIndex) +: ReferenceValue + { + public override SingleValue DeepCopy () + { + return this; + } + } +} diff --git a/src/linker/Linker.Dataflow/ReferenceValue.cs b/src/linker/Linker.Dataflow/ReferenceValue.cs new file mode 100644 index 000000000..4e43a95cd --- /dev/null +++ b/src/linker/Linker.Dataflow/ReferenceValue.cs @@ -0,0 +1,11 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +using ILLink.Shared.DataFlow; + +namespace ILLink.Shared.TrimAnalysis +{ + /// <summary> + /// Acts as the base class for all values that represent a reference to another value. These should only be held in a ref type or on the stack as a result of a 'load address' instruction (e.g. ldloca). + /// </summary> + public abstract record ReferenceValue : SingleValue { } +} diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 085d57b36..052b0d69d 100644 --- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -108,21 +108,25 @@ namespace Mono.Linker.Dataflow protected override MultiValue GetFieldValue (FieldDefinition field) => _annotations.GetFieldValue (field); - protected override void HandleStoreField (MethodDefinition method, FieldValue field, Instruction operation, MultiValue valueToStore) + private void HandleStoreValueWithDynamicallyAccessedMembers (ValueWithDynamicallyAccessedMembers targetValue, Instruction operation, MultiValue sourceValue) { - if (field.DynamicallyAccessedMemberTypes != 0) { + if (targetValue.DynamicallyAccessedMemberTypes != 0) { _origin = _origin.WithInstructionOffset (operation.Offset); - HandleAssignmentPattern (_origin, valueToStore, field); + HandleAssignmentPattern (_origin, sourceValue, targetValue); } } + protected override void HandleStoreField (MethodDefinition method, FieldValue field, Instruction operation, MultiValue valueToStore) + => HandleStoreValueWithDynamicallyAccessedMembers (field, operation, valueToStore); + protected override void HandleStoreParameter (MethodDefinition method, MethodParameterValue parameter, Instruction operation, MultiValue valueToStore) - { - if (parameter.DynamicallyAccessedMemberTypes != 0) { - _origin = _origin.WithInstructionOffset (operation.Offset); - HandleAssignmentPattern (_origin, valueToStore, parameter); - } - } + => HandleStoreValueWithDynamicallyAccessedMembers (parameter, operation, valueToStore); + + protected override void HandleStoreMethodThisParameter (MethodDefinition method, MethodThisParameterValue thisParameter, Instruction operation, MultiValue valueToStore) + => HandleStoreValueWithDynamicallyAccessedMembers (thisParameter, operation, valueToStore); + + protected override void HandleStoreMethodReturnValue (MethodDefinition method, MethodReturnValue returnValue, Instruction operation, MultiValue valueToStore) + => HandleStoreValueWithDynamicallyAccessedMembers (returnValue, operation, valueToStore); public override bool HandleCall (MethodBody callingMethodBody, MethodReference calledMethod, Instruction operation, ValueNodeList methodParams, out MultiValue methodReturnValue) { diff --git a/src/linker/Linker.Dataflow/ScannerExtensions.cs b/src/linker/Linker.Dataflow/ScannerExtensions.cs index 166a968b6..c445f7d61 100644 --- a/src/linker/Linker.Dataflow/ScannerExtensions.cs +++ b/src/linker/Linker.Dataflow/ScannerExtensions.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using Mono.Cecil; using Mono.Cecil.Cil; namespace Mono.Linker.Dataflow @@ -40,11 +39,6 @@ namespace Mono.Linker.Dataflow } return branchTargets; } - - public static bool IsByRefOrPointer (this TypeReference typeRef) - { - return typeRef.IsByReference || typeRef.IsPointer; - } } } diff --git a/src/linker/Linker/MethodDefinitionExtensions.cs b/src/linker/Linker/MethodDefinitionExtensions.cs index 2404b6202..814d390d8 100644 --- a/src/linker/Linker/MethodDefinitionExtensions.cs +++ b/src/linker/Linker/MethodDefinitionExtensions.cs @@ -98,10 +98,5 @@ namespace Mono.Linker di.Scope = null; } } - - public static bool HasImplicitThis (this MethodDefinition method) - { - return method.HasThis && !method.ExplicitThis; - } } } diff --git a/src/linker/Linker/MethodReferenceExtensions.cs b/src/linker/Linker/MethodReferenceExtensions.cs index be378a015..e38371399 100644 --- a/src/linker/Linker/MethodReferenceExtensions.cs +++ b/src/linker/Linker/MethodReferenceExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using ILLink.Shared.TypeSystemProxy; using Mono.Cecil; namespace Mono.Linker @@ -96,5 +97,30 @@ namespace Mono.Linker { return method.Parameters.Count > parameterIndex && method.Parameters[parameterIndex].ParameterType.IsTypeOf (fullTypeName); } + + public static bool HasImplicitThis (this MethodReference method) + { + return method.HasThis && !method.ExplicitThis; + } + + /// <summary> + /// Returns the ReferenceKind of a parameter (in, out, ref, none) of a method. Uses the IL based index number (i.e. `this` is 0 if there is a `this`, then 1 is the first parameter) + /// </summary> + public static ReferenceKind ParameterReferenceKind (this MethodReference method, int index) + { + if (method.HasImplicitThis ()) { + if (index == 0) + return method.DeclaringType.IsValueType ? ReferenceKind.Ref : ReferenceKind.None; + index--; + } + var param = method.Parameters[index]; + if (!param.ParameterType.IsByReference) + return ReferenceKind.None; + if (param.IsIn) + return ReferenceKind.In; + if (param.IsOut) + return ReferenceKind.Out; + return ReferenceKind.Ref; + } } } diff --git a/src/linker/Linker/TypeReferenceExtensions.cs b/src/linker/Linker/TypeReferenceExtensions.cs index 4a6915fec..04dece0f9 100644 --- a/src/linker/Linker/TypeReferenceExtensions.cs +++ b/src/linker/Linker/TypeReferenceExtensions.cs @@ -405,5 +405,13 @@ namespace Mono.Linker => typeReference is ArrayType ? BCL.FindPredefinedType (WellKnownType.System_Array, context) : context.TryResolve (typeReference); + + public static bool IsByRefOrPointer (this TypeReference typeReference) + { + return typeReference.WithoutModifiers ().MetadataType switch { + MetadataType.Pointer or MetadataType.ByReference => true, + _ => false, + }; + } } } diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs index 7331f81ac..2caf1fc83 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs @@ -226,15 +226,15 @@ namespace Mono.Linker.Tests.Cases.DataFlow // https://github.com/dotnet/linker/issues/2680 - analyzer doesn't reset array in this case [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicFields), ProducedBy = ProducedBy.Trimmer)] - // https://github.com/dotnet/linker/issues/2632 - Ref params don't reset or track. - // [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] + // https://github.com/dotnet/linker/issues/2680 - analyzer doesn't reset array in this case + [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicMethods), ProducedBy = ProducedBy.Trimmer)] static void TestArrayResetGetElementOnByRefArray (int i = 0) { Type[] arr = new Type[] { typeof (TestType), typeof (TestType) }; arr[0].RequiresPublicProperties (); - TakesTypeByRef (ref arr[0]); // Should reset index 0 - linker doesn't - arr[0].RequiresPublicMethods (); // Should warn -- linker doesn't + TakesTypeByRef (ref arr[0]); // Should reset index 0 - analyzer doesn't + arr[0].RequiresPublicMethods (); // Should warn - analyzer doesn't arr[1].RequiresPublicMethods (); // Shouldn't warn TakesTypeByRef (ref arr[i]); // Reset - unknown index diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/ByRefDataflow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/ByRefDataflow.cs index efc270ce1..3d47170d6 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/ByRefDataflow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/ByRefDataflow.cs @@ -2,9 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.Text; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; @@ -44,9 +42,15 @@ namespace Mono.Linker.Tests.Cases.DataFlow // Trimmer and analyzer use different formats for ref parameters: https://github.com/dotnet/linker/issues/2406 [ExpectedWarning ("IL2077", nameof (ByRefDataflow) + "." + nameof (MethodWithRefParameter) + "(Type&)", ProducedBy = ProducedBy.Trimmer)] [ExpectedWarning ("IL2077", nameof (ByRefDataflow) + "." + nameof (MethodWithRefParameter) + "(ref Type)", ProducedBy = ProducedBy.Analyzer)] + [ExpectedWarning ("IL2069", nameof (s_typeWithPublicParameterlessConstructor), "parameter 'type'", nameof (MethodWithRefParameter), ProducedBy = ProducedBy.Trimmer)] + // MethodWithRefParameter (ref x) + [ExpectedWarning ("IL2077", nameof (ByRefDataflow) + "." + nameof (MethodWithRefParameter) + "(Type&)", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2077", nameof (ByRefDataflow) + "." + nameof (MethodWithRefParameter) + "(ref Type)", ProducedBy = ProducedBy.Analyzer)] public static void PassRefToField () { MethodWithRefParameter (ref s_typeWithPublicParameterlessConstructor); + var x = s_typeWithPublicParameterlessConstructor; + MethodWithRefParameter (ref x); } [Kept] diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs index 16e3dca37..2b2726541 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs @@ -87,9 +87,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow typeWithMethods.RequiresPublicMethods (); } - // https://github.com/dotnet/linker/issues/2632 - // This test should generate a warning since there's mismatch on annotations - // [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicFields))] + [ExpectedWarning ("IL2067", nameof (TryGetAnnotatedValue), "RequiresPublicFields")] static void TestReadFromRefParameter_MismatchOnOutput () { Type typeWithMethods = null; @@ -97,9 +95,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow typeWithMethods.RequiresPublicFields (); } - // https://github.com/dotnet/linker/issues/2632 - // This test should generate a warning since there's mismatch on annotations - // [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicFields))] + [ExpectedWarning ("IL2067", nameof (TryGetAnnotatedValue), "RequiresPublicFields")] static void TestReadFromRefParameter_MismatchOnOutput_PassedTwice () { Type typeWithMethods = null; @@ -111,7 +107,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow // https://github.com/dotnet/linker/issues/2632 // This second warning should not be generated, the value of typeWithMethods should have PublicMethods // after the call with out parameter. - [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] + [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods), ProducedBy = ProducedBy.Analyzer)] static void TestReadFromRefParameter_MismatchOnInput () { Type typeWithMethods = GetTypeWithFields (); @@ -124,7 +120,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow // https://github.com/dotnet/linker/issues/2632 // This third warning should not be generated, the value of typeWithMethods should have PublicMethods // after the call with ref parameter. - [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] + [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods), ProducedBy = ProducedBy.Analyzer)] static void TestReadFromRefParameter_MismatchOnInput_PassedTwice () { Type typeWithMethods = GetTypeWithFields (); @@ -138,6 +134,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow } [ExpectedWarning ("IL2067", "typeWithMethods", nameof (TryGetAnnotatedValue))] + [ExpectedWarning ("IL2067", "typeWithMethods", nameof (TryGetAnnotatedValue))] static void TestPassingRefParameter_Mismatch ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] ref Type typeWithMethods) { TryGetAnnotatedValue (ref typeWithMethods); diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs index d47bf1c5e..dc1783482 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs @@ -25,14 +25,14 @@ namespace Mono.Linker.Tests.Cases.DataFlow // This should warn, as assiging to the return ref Type will assign value to the annotated field // but the annotation is not propagated // https://github.com/dotnet/linker/issues/2158 - // [ExpectedWarning("IL????")] static ref Type ReturnAnnotatedTypeReferenceAsUnannotated () { return ref _annotatedField; } [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] static ref Type ReturnAnnotatedTypeReferenceAsAnnotated () { return ref _annotatedField; } - // https://github.com/dotnet/linker/issues/2158 - // [ExpectedWarning("IL2026", "Message for --TestType.Requires--")] + // Correct behavior in the linker, but needs to be added in analyzer + // Bug link: https://github.com/dotnet/linker/issues/2158 + [ExpectedWarning ("IL2026", "Message for --TestType.Requires--", ProducedBy = ProducedBy.Trimmer)] static void AssignToAnnotatedTypeReference () { ref Type typeShouldHaveAllMethods = ref ReturnAnnotatedTypeReferenceAsAnnotated (); diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/MethodOutParameterDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/MethodOutParameterDataFlow.cs index cd9a2c301..86f2f2feb 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/MethodOutParameterDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/MethodOutParameterDataFlow.cs @@ -23,9 +23,15 @@ namespace Mono.Linker.Tests.Cases.DataFlow TestInitializedReadFromOutParameter_MismatchOnOutput_PassedTwice (); TestInitializedReadFromOutParameter_MismatchOnInput (); TestInitializedReadFromOutParameter_MismatchOnInput_PassedTwice (); - TestPassingOutParameter (out t); + // Gets Fields TestPassingOutParameter_Mismatch (out t); + t.RequiresPublicFields (); + // Gets Methods + TestPassingOutParameter (out t); + // Needs Methods and gets Methods TestAssigningToOutParameter (t, out t); + t = typeof (int); + // Needs Fields and gets Methods TestAssigningToOutParameter_Mismatch (t, out t); } @@ -43,10 +49,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow typeWithMethods.RequiresPublicMethods (); } - // https://github.com/dotnet/linker/issues/2632 - // These two warnings should not be generated, the annotations are all correct - [ExpectedWarning ("IL2062", nameof (TryGetAnnotatedValue))] - [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] static void TestUninitializedReadFromOutParameter () { Type typeWithMethods; @@ -54,9 +56,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow typeWithMethods.RequiresPublicMethods (); } - // https://github.com/dotnet/linker/issues/2632 - // This test should generate a warning since there's mismatch on annotations - // [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicFields))] + [ExpectedWarning ("IL2067", nameof (DataFlowTypeExtensions.RequiresPublicFields))] static void TestInitializedReadFromOutParameter_MismatchOnOutput () { Type typeWithMethods = null; @@ -66,7 +66,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow // https://github.com/dotnet/linker/issues/2632 // This test should generate a warning since there's mismatch on annotations - // [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicFields))] + [ExpectedWarning ("IL2067", nameof (DataFlowTypeExtensions.RequiresPublicFields))] static void TestInitializedReadFromOutParameter_MismatchOnOutput_PassedTwice () { Type typeWithMethods = null; @@ -74,27 +74,27 @@ namespace Mono.Linker.Tests.Cases.DataFlow typeWithMethods.RequiresPublicFields (); } - [ExpectedWarning ("IL2072", nameof (TryGetAnnotatedValue))] // https://github.com/dotnet/linker/issues/2632 - // This second warning should not be generated, the value of typeWithMethods should have PublicMethods + // This warning should not be generated, the value of typeWithMethods should have PublicMethods // after the call with out parameter. - [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] + [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods), ProducedBy = ProducedBy.Analyzer)] static void TestInitializedReadFromOutParameter_MismatchOnInput () { Type typeWithMethods = GetTypeWithFields (); + // No warning on out parameter TryGetAnnotatedValue (out typeWithMethods); typeWithMethods.RequiresPublicMethods (); } [ExpectedWarning ("IL2072", nameof (TryGetAnnotatedValueFromValue))] - [ExpectedWarning ("IL2072", nameof (TryGetAnnotatedValueFromValue))] // https://github.com/dotnet/linker/issues/2632 - // This third warning should not be generated, the value of typeWithMethods should have PublicMethods + // This warning should not be generated, the value of typeWithMethods should have PublicMethods // after the call with out parameter. - [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] + [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresPublicMethods), ProducedBy = ProducedBy.Analyzer)] static void TestInitializedReadFromOutParameter_MismatchOnInput_PassedTwice () { Type typeWithMethods = GetTypeWithFields (); + // Warn on first parameter only, not on out parameter TryGetAnnotatedValueFromValue (typeWithMethods, out typeWithMethods); typeWithMethods.RequiresPublicMethods (); } |