diff options
18 files changed, 1049 insertions, 202 deletions
diff --git a/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index a185801d4..1a9adee45 100644 --- a/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -15,7 +15,6 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.FlowAnalysis; -using Microsoft.CodeAnalysis.Operations; namespace ILLink.RoslynAnalyzer { @@ -63,6 +62,24 @@ namespace ILLink.RoslynAnalyzer public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => GetSupportedDiagnostics (); + bool HasCompilerGeneratedCode (IOperation operation) + { + switch (operation.Kind) { + case OperationKind.AnonymousFunction: + case OperationKind.LocalFunction: + case OperationKind.YieldBreak: + case OperationKind.YieldReturn: + return true; + } + + foreach (var child in operation.ChildOperations) { + if (HasCompilerGeneratedCode (child)) + return true; + } + + return false; + } + public override void Initialize (AnalysisContext context) { if (!System.Diagnostics.Debugger.IsAttached) @@ -87,15 +104,8 @@ namespace ILLink.RoslynAnalyzer // Sub optimal way to handle analyzer not to generate warnings until the linker is fixed // Iterators, local functions and lambdas are handled foreach (IOperation blockOperation in context.OperationBlocks) { - if (blockOperation is IBlockOperation blocks) { - foreach (IOperation operation in blocks.Operations) { - if (operation.Kind == OperationKind.AnonymousFunction || - operation.Kind == OperationKind.LocalFunction || - operation.Kind == OperationKind.YieldBreak || - operation.Kind == OperationKind.YieldReturn) - return; - } - } + if (HasCompilerGeneratedCode (blockOperation)) + return; } foreach (var operationBlock in context.OperationBlocks) { diff --git a/src/ILLink.Shared/DataFlow/DefaultValueDictionary.cs b/src/ILLink.Shared/DataFlow/DefaultValueDictionary.cs index 679f70ca6..1d8221463 100644 --- a/src/ILLink.Shared/DataFlow/DefaultValueDictionary.cs +++ b/src/ILLink.Shared/DataFlow/DefaultValueDictionary.cs @@ -27,6 +27,8 @@ namespace ILLink.Shared.DataFlow public DefaultValueDictionary (TValue defaultValue) => (Dictionary, DefaultValue) = (null, defaultValue); + private DefaultValueDictionary (TValue defaultValue, Dictionary<TKey, TValue> dictionary) => (Dictionary, DefaultValue) = (dictionary, defaultValue); + public DefaultValueDictionary (DefaultValueDictionary<TKey, TValue> other) { Dictionary = other.Dictionary == null ? null : new Dictionary<TKey, TValue> (other.Dictionary); @@ -65,6 +67,10 @@ namespace ILLink.Shared.DataFlow return true; } + public override bool Equals (object? obj) => obj is DefaultValueDictionary<TKey, TValue> other && Equals (other); + + public int Count => Dictionary?.Count ?? 0; + public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator () { return Dictionary?.GetEnumerator () ?? Enumerable.Empty<KeyValuePair<TKey, TValue>> ().GetEnumerator (); @@ -84,5 +90,24 @@ namespace ILLink.Shared.DataFlow sb.Append (Environment.NewLine).Append ("}"); return sb.ToString (); } + + public DefaultValueDictionary<TKey, TValue> Clone () + { + var defaultValue = DefaultValue is IDeepCopyValue<TValue> copyDefaultValue ? copyDefaultValue.DeepCopy () : DefaultValue; + if (Dictionary == null) + return new DefaultValueDictionary<TKey, TValue> (defaultValue); + + var dict = new Dictionary<TKey, TValue> (); + foreach (var kvp in Dictionary) { + var key = kvp.Key; + var value = kvp.Value; + dict.Add (key, value is IDeepCopyValue<TValue> copyValue ? copyValue.DeepCopy () : value); + } + return new DefaultValueDictionary<TKey, TValue> (defaultValue, dict); + } + + // Prevent warning CS0659 https://docs.microsoft.com/en-us/dotnet/csharp/misc/cs0659. + // This type should never be used as a dictionary key. + public override int GetHashCode () => throw new NotImplementedException (); } } diff --git a/src/linker/Linker.Dataflow/HoistedLocalKey.cs b/src/linker/Linker.Dataflow/HoistedLocalKey.cs new file mode 100644 index 000000000..b721df288 --- /dev/null +++ b/src/linker/Linker.Dataflow/HoistedLocalKey.cs @@ -0,0 +1,30 @@ +// 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 System; +using System.Diagnostics; +using Mono.Cecil; + +namespace Mono.Linker.Dataflow +{ + // This represents a field which has been generated by the compiler as the + // storage location for a hoisted local (a local variable which is lifted to a + // field on a state machine type, or to a field on a closure accessed by lambdas + // or local functions). + public readonly struct HoistedLocalKey : IEquatable<HoistedLocalKey> + { + readonly FieldDefinition Field; + + public HoistedLocalKey (FieldDefinition field) + { + Debug.Assert (CompilerGeneratedState.IsHoistedLocal (field)); + Field = field; + } + + public bool Equals (HoistedLocalKey other) => Field.Equals (other.Field); + + public override bool Equals (object? obj) => obj is HoistedLocalKey other && Equals (other); + + public override int GetHashCode () => Field.GetHashCode (); + } +}
\ No newline at end of file diff --git a/src/linker/Linker.Dataflow/InterproceduralState.cs b/src/linker/Linker.Dataflow/InterproceduralState.cs new file mode 100644 index 000000000..6e18818bb --- /dev/null +++ b/src/linker/Linker.Dataflow/InterproceduralState.cs @@ -0,0 +1,89 @@ +// 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 System; +using System.Collections.Generic; +using System.Diagnostics; +using ILLink.Shared.DataFlow; +using Mono.Cecil; +using Mono.Cecil.Cil; +using HoistedLocalState = ILLink.Shared.DataFlow.DefaultValueDictionary< + Mono.Linker.Dataflow.HoistedLocalKey, + ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>>; +using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>; + +namespace Mono.Linker.Dataflow +{ + // Wrapper that implements IEquatable for MethodBody. + readonly record struct MethodBodyValue (MethodBody MethodBody); + + // Tracks the set of methods which get analyzer together during interprocedural analysis, + // and the possible states of hoisted locals in state machine methods and lambdas/local functions. + struct InterproceduralState : IEquatable<InterproceduralState> + { + public ValueSet<MethodBodyValue> MethodBodies; + public HoistedLocalState HoistedLocals; + readonly InterproceduralStateLattice lattice; + + public InterproceduralState (ValueSet<MethodBodyValue> methodBodies, HoistedLocalState hoistedLocals, InterproceduralStateLattice lattice) + => (MethodBodies, HoistedLocals, this.lattice) = (methodBodies, hoistedLocals, lattice); + + public bool Equals (InterproceduralState other) + => MethodBodies.Equals (other.MethodBodies) && HoistedLocals.Equals (other.HoistedLocals); + + public InterproceduralState Clone () + => new (MethodBodies.Clone (), HoistedLocals.Clone (), lattice); + + public void TrackMethod (MethodBodyValue methodBody) + { + // Work around the fact that ValueSet is readonly + var methodsList = new List<MethodBodyValue> (MethodBodies); + methodsList.Add (methodBody); + + // For state machine methods, also scan the state machine members. + // Simplification: assume that all generated methods of the state machine type are + // reached at the point where the state machine method is reached. + if (CompilerGeneratedState.TryGetStateMachineType (methodBody.MethodBody.Method, out TypeDefinition? stateMachineType)) { + foreach (var stateMachineMethod in stateMachineType.Methods) { + Debug.Assert (!CompilerGeneratedNames.IsLambdaOrLocalFunction (stateMachineMethod.Name)); + if (stateMachineMethod.Body is MethodBody stateMachineMethodBody) + methodsList.Add (new MethodBodyValue (stateMachineMethodBody)); + } + } + + MethodBodies = new ValueSet<MethodBodyValue> (methodsList); + } + + public void SetHoistedLocal (HoistedLocalKey key, MultiValue value) + { + // For hoisted locals, we track the entire set of assigned values seen + // in the closure of a method, so setting a hoisted local value meets + // it with any existing value. + HoistedLocals.Set (key, + lattice.HoistedLocalsLattice.ValueLattice.Meet ( + HoistedLocals.Get (key), value)); + } + + public MultiValue GetHoistedLocal (HoistedLocalKey key) + => HoistedLocals.Get (key); + } + + struct InterproceduralStateLattice : ILattice<InterproceduralState> + { + public readonly ValueSetLattice<MethodBodyValue> MethodBodyLattice; + public readonly DictionaryLattice<HoistedLocalKey, MultiValue, ValueSetLattice<SingleValue>> HoistedLocalsLattice; + + public InterproceduralStateLattice ( + ValueSetLattice<MethodBodyValue> methodBodyLattice, + DictionaryLattice<HoistedLocalKey, MultiValue, ValueSetLattice<SingleValue>> hoistedLocalsLattice) + => (MethodBodyLattice, HoistedLocalsLattice) = (methodBodyLattice, hoistedLocalsLattice); + + public InterproceduralState Top => new InterproceduralState (MethodBodyLattice.Top, HoistedLocalsLattice.Top, this); + + public InterproceduralState Meet (InterproceduralState left, InterproceduralState right) + => new ( + MethodBodyLattice.Meet (left.MethodBodies, right.MethodBodies), + HoistedLocalsLattice.Meet (left.HoistedLocals, right.HoistedLocals), + this); + } +}
\ No newline at end of file diff --git a/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/linker/Linker.Dataflow/MethodBodyScanner.cs index 39579f3dc..905458a08 100644 --- a/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Linq; using ILLink.Shared; using ILLink.Shared.DataFlow; @@ -13,7 +12,9 @@ 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 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 @@ -45,7 +46,8 @@ namespace Mono.Linker.Dataflow { protected readonly LinkContext _context; protected static ValueSetLattice<SingleValue> MultiValueLattice => default; - protected static ValueSetLattice<MethodProxy> MethodLattice => default; + + static InterproceduralStateLattice InterproceduralStateLattice => default; protected MethodBodyScanner (LinkContext context) { @@ -224,66 +226,42 @@ namespace Mono.Linker.Dataflow // Scans the method as well as any nested functions (local functions or lambdas) and state machines // reachable from it. - public virtual void InterproceduralScan (MethodBody methodBody) + public virtual void InterproceduralScan (MethodBody startingMethodBody) { - var methodsInGroup = new ValueSet<MethodProxy> (methodBody.Method); + // Note that the default value of a hoisted local will be MultiValueLattice.Top, not UnknownValue.Instance. + // This ensures that there are no warnings for the "unassigned state" of a parameter. + // Definite assignment should ensure that there is no way for this to be an analysis hole. + var interproceduralState = InterproceduralStateLattice.Top; - // Optimization to prevent multiple scans of a method. - // Eventually we will need to allow re-scanning in some cases, for example - // when we discover new inputs to a method. But we aren't doing dataflow across - // lambdas and local functions yet, so no need for now. - HashSet<MethodDefinition> scannedMethods = new HashSet<MethodDefinition> (); + var oldInterproceduralState = interproceduralState.Clone (); + interproceduralState.TrackMethod (new MethodBodyValue (startingMethodBody)); - while (true) { - if (!TryGetNextMethodToScan (out MethodDefinition? methodToScan)) - break; + while (!interproceduralState.Equals (oldInterproceduralState)) { + oldInterproceduralState = interproceduralState.Clone (); - scannedMethods.Add (methodToScan); - Scan (methodToScan.Body, ref methodsInGroup); - - // For state machine methods, also scan the state machine members. - // Simplification: assume that all generated methods of the state machine type are - // invoked at the point where the state machine method is called. - if (CompilerGeneratedState.TryGetStateMachineType (methodToScan, out TypeDefinition? stateMachineType)) { - foreach (var method in stateMachineType.Methods) { - Debug.Assert (!CompilerGeneratedNames.IsLambdaOrLocalFunction (method.Name)); - if (method.Body is MethodBody stateMachineBody) - Scan (stateMachineBody, ref methodsInGroup); - } - } + // Flow state through all methods encountered so far, as long as there + // are changes discovered in the hoisted local state on entry to any method. + foreach (var methodBodyValue in oldInterproceduralState.MethodBodies) + Scan (methodBodyValue.MethodBody, ref interproceduralState); } #if DEBUG // Validate that the compiler-generated callees tracked by the compiler-generated state // are the same set of methods that we discovered and scanned above. - if (_context.CompilerGeneratedState.TryGetCompilerGeneratedCalleesForUserMethod (methodBody.Method, out List<IMemberDefinition>? compilerGeneratedCallees)) { + if (_context.CompilerGeneratedState.TryGetCompilerGeneratedCalleesForUserMethod (startingMethodBody.Method, out List<IMemberDefinition>? compilerGeneratedCallees)) { var calleeMethods = compilerGeneratedCallees.OfType<MethodDefinition> (); - // https://github.com/dotnet/linker/issues/2845 // Disabled asserts due to a bug - //Debug.Assert (methodsInGroup.Count () == 1 + calleeMethods.Count ()); - //foreach (var method in calleeMethods) - // Debug.Assert (methodsInGroup.Contains (method)); + // Debug.Assert (interproceduralState.Count == 1 + calleeMethods.Count ()); + // foreach (var method in calleeMethods) + // Debug.Assert (interproceduralState.Any (kvp => kvp.Key.Method == method)); } else { - Debug.Assert (methodsInGroup.Count () == 1); + Debug.Assert (interproceduralState.MethodBodies.Count () == 1); } #endif - - bool TryGetNextMethodToScan ([NotNullWhen (true)] out MethodDefinition? method) - { - foreach (var candidate in methodsInGroup) { - var candidateMethod = candidate.Method; - if (!scannedMethods.Contains (candidateMethod) && candidateMethod.HasBody) { - method = candidateMethod; - return true; - } - } - method = null; - return false; - } } - void TrackNestedFunctionReference (MethodReference referencedMethod, ref ValueSet<MethodProxy> methodsInGroup) + void TrackNestedFunctionReference (MethodReference referencedMethod, ref InterproceduralState interproceduralState) { if (_context.TryResolve (referencedMethod) is not MethodDefinition method) return; @@ -291,10 +269,11 @@ namespace Mono.Linker.Dataflow if (!CompilerGeneratedNames.IsLambdaOrLocalFunction (method.Name)) return; - methodsInGroup = MethodLattice.Meet (methodsInGroup, new (method)); + if (method.Body is MethodBody methodBody) + interproceduralState.TrackMethod (new MethodBodyValue (methodBody)); } - protected virtual void Scan (MethodBody methodBody, ref ValueSet<MethodProxy> methodsInGroup) + protected virtual void Scan (MethodBody methodBody, ref InterproceduralState interproceduralState) { MethodDefinition thisMethod = methodBody.Method; @@ -411,7 +390,7 @@ namespace Mono.Linker.Dataflow break; case Code.Ldftn: - TrackNestedFunctionReference ((MethodReference) operation.Operand, ref methodsInGroup); + TrackNestedFunctionReference ((MethodReference) operation.Operand, ref interproceduralState); PushUnknown (currentStack); break; @@ -518,7 +497,7 @@ namespace Mono.Linker.Dataflow case Code.Ldsfld: case Code.Ldflda: case Code.Ldsflda: - ScanLdfld (operation, currentStack, methodBody); + ScanLdfld (operation, currentStack, methodBody, ref interproceduralState); break; case Code.Newarr: { @@ -562,7 +541,7 @@ namespace Mono.Linker.Dataflow case Code.Stfld: case Code.Stsfld: - ScanStfld (operation, currentStack, thisMethod, methodBody); + ScanStfld (operation, currentStack, thisMethod, methodBody, ref interproceduralState); break; case Code.Cpobj: @@ -637,8 +616,8 @@ namespace Mono.Linker.Dataflow case Code.Call: case Code.Callvirt: case Code.Newobj: - TrackNestedFunctionReference ((MethodReference) operation.Operand, ref methodsInGroup); - HandleCall (methodBody, operation, currentStack, locals, curBasicBlock); + TrackNestedFunctionReference ((MethodReference) operation.Operand, ref interproceduralState); + HandleCall (methodBody, operation, currentStack, locals, ref interproceduralState, curBasicBlock); break; case Code.Jmp: @@ -675,7 +654,7 @@ namespace Mono.Linker.Dataflow StackSlot retValue = PopUnknown (currentStack, 1, methodBody, operation.Offset); // 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)); + ReturnValue = MultiValueLattice.Meet (ReturnValue, DereferenceValue (retValue.Value, locals, ref interproceduralState)); } ClearStack (ref currentStack); break; @@ -944,7 +923,8 @@ namespace Mono.Linker.Dataflow private void ScanLdfld ( Instruction operation, Stack<StackSlot> currentStack, - MethodBody methodBody) + MethodBody methodBody, + ref InterproceduralState interproceduralState) { Code code = operation.OpCode.Code; if (code == Code.Ldfld || code == Code.Ldflda) @@ -953,16 +933,20 @@ namespace Mono.Linker.Dataflow bool isByRef = code == Code.Ldflda || code == Code.Ldsflda; FieldDefinition? field = _context.TryResolve ((FieldReference) operation.Operand); - if (field != null) { - MultiValue newValue = isByRef ? - new FieldReferenceValue (field) - : GetFieldValue (field); - StackSlot slot = new (newValue); - currentStack.Push (slot); + if (field == null) { + PushUnknown (currentStack); return; } - PushUnknown (currentStack); + MultiValue value; + if (isByRef) { + value = new FieldReferenceValue (field); + } else if (CompilerGeneratedState.IsHoistedLocal (field)) { + value = interproceduralState.GetHoistedLocal (new HoistedLocalKey (field)); + } else { + value = GetFieldValue (field); + } + currentStack.Push (new StackSlot (value)); } protected virtual void HandleStoreField (MethodDefinition method, FieldValue field, Instruction operation, MultiValue valueToStore) @@ -985,7 +969,8 @@ namespace Mono.Linker.Dataflow Instruction operation, Stack<StackSlot> currentStack, MethodDefinition thisMethod, - MethodBody methodBody) + MethodBody methodBody, + ref InterproceduralState interproceduralState) { StackSlot valueToStoreSlot = PopUnknown (currentStack, 1, methodBody, operation.Offset); if (operation.OpCode.Code == Code.Stfld) @@ -993,6 +978,11 @@ namespace Mono.Linker.Dataflow FieldDefinition? field = _context.TryResolve ((FieldReference) operation.Operand); if (field != null) { + if (CompilerGeneratedState.IsHoistedLocal (field)) { + interproceduralState.SetHoistedLocal (new HoistedLocalKey (field), valueToStoreSlot.Value); + return; + } + foreach (var value in GetFieldValue (field)) { // GetFieldValue may return different node types, in which case they can't be stored to. // At least not yet. @@ -1043,7 +1033,7 @@ namespace Mono.Linker.Dataflow return methodParams; } - internal MultiValue DereferenceValue (MultiValue maybeReferenceValue, Dictionary<VariableDefinition, ValueBasicBlockPair> locals) + internal MultiValue DereferenceValue (MultiValue maybeReferenceValue, Dictionary<VariableDefinition, ValueBasicBlockPair> locals, ref InterproceduralState interproceduralState) { MultiValue dereferencedValue = MultiValueLattice.Top; foreach (var value in maybeReferenceValue) { @@ -1051,7 +1041,9 @@ namespace Mono.Linker.Dataflow case FieldReferenceValue fieldReferenceValue: dereferencedValue = MultiValue.Meet ( dereferencedValue, - GetFieldValue (fieldReferenceValue.FieldDefinition)); + CompilerGeneratedState.IsHoistedLocal (fieldReferenceValue.FieldDefinition) + ? interproceduralState.GetHoistedLocal (new HoistedLocalKey (fieldReferenceValue.FieldDefinition)) + : GetFieldValue (fieldReferenceValue.FieldDefinition)); break; case ParameterReferenceValue parameterReferenceValue: dereferencedValue = MultiValue.Meet ( @@ -1104,6 +1096,7 @@ namespace Mono.Linker.Dataflow Instruction operation, Stack<StackSlot> currentStack, LocalVariableStore locals, + ref InterproceduralState interproceduralState, int curBasicBlock) { MethodReference calledMethod = (MethodReference) operation.Operand; @@ -1113,13 +1106,15 @@ namespace Mono.Linker.Dataflow SingleValue? newObjValue; ValueNodeList methodArguments = PopCallArguments (currentStack, calledMethod, callingMethodBody, isNewObj, operation.Offset, out newObjValue); - ValueNodeList dereferencedMethodParams = new (methodArguments.Select (param => DereferenceValue (param, locals)).ToList ()); + var dereferencedMethodParams = new List<MultiValue> (); + foreach (var argument in methodArguments) + dereferencedMethodParams.Add (DereferenceValue (argument, locals, ref interproceduralState)); MultiValue methodReturnValue; bool handledFunction = HandleCall ( callingMethodBody, calledMethod, operation, - dereferencedMethodParams, + new ValueNodeList (dereferencedMethodParams), out methodReturnValue); // Handle the return value or newobj result diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 4e60e2b1a..02a512806 100644 --- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -7,7 +7,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; using ILLink.Shared; -using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using ILLink.Shared.TypeSystemProxy; using Mono.Cecil; @@ -59,7 +58,7 @@ namespace Mono.Linker.Dataflow _origin = origin; _annotations = context.Annotations.FlowAnnotations; _reflectionMarker = new ReflectionMarker (context, parent, enabled: false); - TrimAnalysisPatterns = new TrimAnalysisPatternStore (context); + TrimAnalysisPatterns = new TrimAnalysisPatternStore (MultiValueLattice, context); } public override void InterproceduralScan (MethodBody methodBody) @@ -70,10 +69,10 @@ namespace Mono.Linker.Dataflow TrimAnalysisPatterns.MarkAndProduceDiagnostics (reflectionMarker, _markStep); } - protected override void Scan (MethodBody methodBody, ref ValueSet<MethodProxy> methodsInGroup) + protected override void Scan (MethodBody methodBody, ref InterproceduralState interproceduralState) { _origin = new MessageOrigin (methodBody.Method); - base.Scan (methodBody, ref methodsInGroup); + base.Scan (methodBody, ref interproceduralState); if (!methodBody.Method.ReturnsVoid ()) { var method = methodBody.Method; diff --git a/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs b/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs index 401330e55..81010a3a6 100644 --- a/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs +++ b/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs @@ -2,6 +2,8 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Diagnostics; +using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>; @@ -20,6 +22,16 @@ namespace Mono.Linker.Dataflow Origin = origin; } + public TrimAnalysisAssignmentPattern Merge (ValueSetLattice<SingleValue> lattice, TrimAnalysisAssignmentPattern other) + { + Debug.Assert (Origin == other.Origin); + + return new TrimAnalysisAssignmentPattern ( + lattice.Meet (Source, other.Source), + lattice.Meet (Target, other.Target), + Origin); + } + public void MarkAndProduceDiagnostics (ReflectionMarker reflectionMarker, LinkContext context) { bool diagnosticsEnabled = !context.Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (Origin.Provider); diff --git a/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs b/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs index 1f71d7e49..be4219ae6 100644 --- a/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs +++ b/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs @@ -3,11 +3,11 @@ using System.Collections.Immutable; using System.Diagnostics; +using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using Mono.Cecil; using Mono.Cecil.Cil; using Mono.Linker.Steps; - using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>; namespace Mono.Linker.Dataflow @@ -42,6 +42,25 @@ namespace Mono.Linker.Dataflow Origin = origin; } + public TrimAnalysisMethodCallPattern Merge (ValueSetLattice<SingleValue> lattice, TrimAnalysisMethodCallPattern other) + { + Debug.Assert (Operation == other.Operation); + Debug.Assert (Origin == other.Origin); + Debug.Assert (CalledMethod == other.CalledMethod); + Debug.Assert (Arguments.Length == other.Arguments.Length); + + var argumentsBuilder = ImmutableArray.CreateBuilder<MultiValue> (); + for (int i = 0; i < Arguments.Length; i++) + argumentsBuilder.Add (lattice.Meet (Arguments[i], other.Arguments[i])); + + return new TrimAnalysisMethodCallPattern ( + Operation, + CalledMethod, + lattice.Meet (Instance, other.Instance), + argumentsBuilder.ToImmutable (), + Origin); + } + public void MarkAndProduceDiagnostics (ReflectionMarker reflectionMarker, MarkStep markStep, LinkContext context) { bool diagnosticsEnabled = !context.Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (Origin.Provider); diff --git a/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs b/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs index 2e7796ef3..ee88e9d55 100644 --- a/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs +++ b/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using Mono.Linker.Steps; @@ -11,12 +12,14 @@ namespace Mono.Linker.Dataflow { readonly Dictionary<(MessageOrigin, bool), TrimAnalysisAssignmentPattern> AssignmentPatterns; readonly Dictionary<MessageOrigin, TrimAnalysisMethodCallPattern> MethodCallPatterns; + readonly ValueSetLattice<SingleValue> Lattice; readonly LinkContext _context; - public TrimAnalysisPatternStore (LinkContext context) + public TrimAnalysisPatternStore (ValueSetLattice<SingleValue> lattice, LinkContext context) { AssignmentPatterns = new Dictionary<(MessageOrigin, bool), TrimAnalysisAssignmentPattern> (); MethodCallPatterns = new Dictionary<MessageOrigin, TrimAnalysisMethodCallPattern> (); + Lattice = lattice; _context = context; } @@ -27,12 +30,23 @@ namespace Mono.Linker.Dataflow // https://github.com/dotnet/linker/issues/2778 // For now, work around it with a separate bit. bool isReturnValue = pattern.Target.AsSingleValue () is MethodReturnValue; - AssignmentPatterns.Add ((pattern.Origin, isReturnValue), pattern); + + if (!AssignmentPatterns.TryGetValue ((pattern.Origin, isReturnValue), out var existingPattern)) { + AssignmentPatterns.Add ((pattern.Origin, isReturnValue), pattern); + return; + } + + AssignmentPatterns[(pattern.Origin, isReturnValue)] = pattern.Merge (Lattice, existingPattern); } public void Add (TrimAnalysisMethodCallPattern pattern) { - MethodCallPatterns.Add (pattern.Origin, pattern); + if (!MethodCallPatterns.TryGetValue (pattern.Origin, out var existingPattern)) { + MethodCallPatterns.Add (pattern.Origin, pattern); + return; + } + + MethodCallPatterns[pattern.Origin] = pattern.Merge (Lattice, existingPattern); } public void MarkAndProduceDiagnostics (ReflectionMarker reflectionMarker, MarkStep markStep) diff --git a/src/linker/Linker.Dataflow/ValueNode.cs b/src/linker/Linker.Dataflow/ValueNode.cs index 5a0df8f34..81d5e0581 100644 --- a/src/linker/Linker.Dataflow/ValueNode.cs +++ b/src/linker/Linker.Dataflow/ValueNode.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using ILLink.Shared; using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>; namespace Mono.Linker.Dataflow @@ -47,7 +48,7 @@ namespace Mono.Linker.Dataflow } } - public struct ValueBasicBlockPair + public struct ValueBasicBlockPair : IEquatable<ValueBasicBlockPair> { public ValueBasicBlockPair (MultiValue value, int basicBlockIndex) { @@ -57,5 +58,11 @@ namespace Mono.Linker.Dataflow public MultiValue Value { get; } public int BasicBlockIndex { get; } + + public bool Equals (ValueBasicBlockPair other) => Value.Equals (other.Value) && BasicBlockIndex.Equals (other.BasicBlockIndex); + + public override bool Equals (object? obj) => obj is ValueBasicBlockPair other && Equals (other); + + public override int GetHashCode () => HashUtils.Combine (Value.GetHashCode (), BasicBlockIndex); } } diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index df91e3347..e4391dc6f 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -1628,32 +1628,43 @@ namespace Mono.Linker.Steps if (reportOnMember) origin = new MessageOrigin (member); - // Warn on reflection access to compiler-generated code. This includes MoveNext methods. - if (member is MethodDefinition method) { - if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (method)) { - var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase; - Context.LogWarning (origin, id, type.GetDisplayName (), method.GetDisplayName ()); - } - // All override methods should have the same annotations as their base methods - // (else we will produce warning IL2046 or IL2092 or some other warning). - // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. - if (method.IsVirtual && Annotations.GetBaseMethods (method) != null) - return; - } + // All override methods should have the same annotations as their base methods + // (else we will produce warning IL2046 or IL2092 or some other warning). + // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. + bool skipWarningsForOverride = member is MethodDefinition m && m.IsVirtual && Annotations.GetBaseMethods (m) != null; - if (Annotations.DoesMemberRequireUnreferencedCode (member, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCodeAttribute)) { + bool isReflectionAccessCoveredByRUC = Annotations.DoesMemberRequireUnreferencedCode (member, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCodeAttribute); + if (isReflectionAccessCoveredByRUC && !skipWarningsForOverride) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithRequiresUnreferencedCode : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithRequiresUnreferencedCode; Context.LogWarning (origin, id, type.GetDisplayName (), ((MemberReference) member).GetDisplayName (), // The cast is valid since it has to be a method or field - MessageFormat.FormatRequiresAttributeMessageArg (requiresUnreferencedCodeAttribute.Message), - MessageFormat.FormatRequiresAttributeMessageArg (requiresUnreferencedCodeAttribute.Url)); + MessageFormat.FormatRequiresAttributeMessageArg (requiresUnreferencedCodeAttribute!.Message), + MessageFormat.FormatRequiresAttributeMessageArg (requiresUnreferencedCodeAttribute!.Url)); } - if (Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (member)) { + bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (member); + if (isReflectionAccessCoveredByDAM && !skipWarningsForOverride) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithDynamicallyAccessedMembers : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers; Context.LogWarning (origin, id, type.GetDisplayName (), ((MemberReference) member).GetDisplayName ()); } + + // Warn on reflection access to compiler-generated methods, if the method isn't already unsafe to access via reflection + // due to annotations. For the annotation-based warnings, we skip virtual overrides since those will produce warnings on + // the base, but for unannotated compiler-generated methods this is not the case, so we must produce these warnings even + // for virtual overrides. This ensures that we include the unannotated MoveNext state machine method. Lambdas and local + // functions should never be virtual overrides in the first place. + bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM; + if (member is MethodDefinition method && ShouldWarnForReflectionAccessToCompilerGeneratedCode (method, isCoveredByAnnotations)) { + var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase; + Context.LogWarning (origin, id, type.GetDisplayName (), method.GetDisplayName ()); + } + + // Warn on reflection access to compiler-generated fields. + if (member is FieldDefinition field && ShouldWarnForReflectionAccessToCompilerGeneratedCode (field, isCoveredByAnnotations)) { + var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase; + Context.LogWarning (origin, id, type.GetDisplayName (), field.GetDisplayName ()); + } } void MarkField (FieldDefinition field, in DependencyInfo reason, in MessageOrigin origin) @@ -1669,25 +1680,7 @@ namespace Mono.Linker.Steps Annotations.Mark (field, reason, origin); } - if (reason.Kind != DependencyKind.DynamicallyAccessedMemberOnType && - Annotations.DoesFieldRequireUnreferencedCode (field, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCodeAttribute) && - !Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider)) - ReportRequiresUnreferencedCode (field.GetDisplayName (), requiresUnreferencedCodeAttribute, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); - - switch (reason.Kind) { - case DependencyKind.AccessedViaReflection: - case DependencyKind.DynamicDependency: - case DependencyKind.DynamicallyAccessedMember: - case DependencyKind.InteropMethodDependency: - if (Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (field) && - !Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider)) - Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersFieldAccessedViaReflection, field.GetDisplayName ()); - - break; - case DependencyKind.DynamicallyAccessedMemberOnType: - ReportWarningsForTypeHierarchyReflectionAccess (field, origin); - break; - } + ProcessAnalysisAnnotationsForField (field, reason.Kind, in origin); if (CheckProcessed (field)) return; @@ -1735,6 +1728,65 @@ namespace Mono.Linker.Steps } } + bool ShouldWarnForReflectionAccessToCompilerGeneratedCode (FieldDefinition field, bool isCoveredByAnnotations) + { + // No need to warn if it's already covered by the Requires attribute or explicit annotations on the field. + if (isCoveredByAnnotations) + return false; + + if (!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (field)) + return false; + + // Only warn for types which are interesting for dataflow. Note that this does + // not include integer types, even though we track integers in the dataflow analysis. + // Technically we should also warn for integer types, but this leads to more warnings + // for example about the compiler-generated "state" field for state machine methods. + // This should be ok because in most cases the state machine types will also have other + // hoisted locals that produce warnings anyway when accessed via reflection. + return Annotations.FlowAnnotations.IsTypeInterestingForDataflow (field.FieldType); + } + + void ProcessAnalysisAnnotationsForField (FieldDefinition field, DependencyKind dependencyKind, in MessageOrigin origin) + { + if (origin.Provider != ScopeStack.CurrentScope.Origin.Provider) { + Debug.Assert (dependencyKind == DependencyKind.DynamicallyAccessedMemberOnType || + (origin.Provider is MethodDefinition originMethod && CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (originMethod))); + } + + if (dependencyKind == DependencyKind.DynamicallyAccessedMemberOnType) { + ReportWarningsForTypeHierarchyReflectionAccess (field, origin); + return; + } + + if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider)) + return; + + bool isReflectionAccessCoveredByRUC; + if (isReflectionAccessCoveredByRUC = Annotations.DoesFieldRequireUnreferencedCode (field, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCodeAttribute)) + ReportRequiresUnreferencedCode (field.GetDisplayName (), requiresUnreferencedCodeAttribute!, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); + + bool isReflectionAccessCoveredByDAM = false; + switch (dependencyKind) { + case DependencyKind.AccessedViaReflection: + case DependencyKind.DynamicDependency: + case DependencyKind.DynamicallyAccessedMember: + case DependencyKind.InteropMethodDependency: + if (isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (field)) + Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersFieldAccessedViaReflection, field.GetDisplayName ()); + + break; + } + + switch (dependencyKind) { + case DependencyKind.AccessedViaReflection: + case DependencyKind.DynamicallyAccessedMember: + bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM; + if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (field, isCoveredByAnnotations)) + Context.LogWarning (origin, DiagnosticId.CompilerGeneratedMemberAccessedViaReflection, field.GetDisplayName ()); + break; + } + } + /// <summary> /// Returns true if the assembly of the <paramref name="scope"></paramref> is not set to link (i.e. action=copy is set for that assembly) /// </summary> @@ -2820,13 +2872,13 @@ namespace Mono.Linker.Steps return method; } - bool ShouldWarnForReflectionAccessToCompilerGeneratedCode (MethodDefinition method) + bool ShouldWarnForReflectionAccessToCompilerGeneratedCode (MethodDefinition method, bool isCoveredByAnnotations) { - if (!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (method) || method.Body == null) + // No need to warn if it's already covered by the Requires attribute or explicit annotations on the method. + if (isCoveredByAnnotations) return false; - // No need to warn if it's already covered by the Requires attribute or explicit annotations on the method. - if (Annotations.DoesMethodRequireUnreferencedCode (method, out _) || Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (method)) + if (!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (method) || method.Body == null) return false; // Warn only if it has potential dataflow issues, as approximated by our check to see if it requires @@ -2917,41 +2969,41 @@ namespace Mono.Linker.Steps if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider)) return; - // Warn about reflection access to compiler-generated code. - // This must happen before the check for virtual methods, because it should include the - // virtual MoveNext method of state machines. - switch (dependencyKind) { - case DependencyKind.AccessedViaReflection: - case DependencyKind.DynamicallyAccessedMember: - if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (method)) - Context.LogWarning (origin, DiagnosticId.CompilerGeneratedMemberAccessedViaReflection, method.GetDisplayName ()); - break; - } - - if (dependencyKind == DependencyKind.DynamicallyAccessedMember) { - // All override methods should have the same annotations as their base methods - // (else we will produce warning IL2046 or IL2092 or some other warning). - // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. - if (method.IsVirtual && Annotations.GetBaseMethods (method) != null) - return; - } + // All override methods should have the same annotations as their base methods + // (else we will produce warning IL2046 or IL2092 or some other warning). + // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. + bool skipWarningsForOverride = dependencyKind == DependencyKind.DynamicallyAccessedMember && method.IsVirtual && Annotations.GetBaseMethods (method) != null; - if (Annotations.DoesMethodRequireUnreferencedCode (method, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCode)) - ReportRequiresUnreferencedCode (method.GetDisplayName (), requiresUnreferencedCode, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); + bool isReflectionAccessCoveredByRUC = Annotations.DoesMethodRequireUnreferencedCode (method, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCode); + if (isReflectionAccessCoveredByRUC && !skipWarningsForOverride) + ReportRequiresUnreferencedCode (method.GetDisplayName (), requiresUnreferencedCode!, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); - if (Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (method)) { + bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (method); + if (isReflectionAccessCoveredByDAM && !skipWarningsForOverride) { // ReflectionMethodBodyScanner handles more cases for data flow annotations // so don't warn for those. switch (dependencyKind) { case DependencyKind.AttributeConstructor: case DependencyKind.AttributeProperty: - return; - + break; default: + Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, method.GetDisplayName ()); break; } + } - Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, method.GetDisplayName ()); + // Warn on reflection access to compiler-generated methods, if the method isn't already unsafe to access via reflection + // due to annotations. For the annotation-based warnings, we skip virtual overrides since those will produce warnings on + // the base, but for unannotated compiler-generated methods this is not the case, so we must produce these warnings even + // for virtual overrides. This ensures that we include the unannotated MoveNext state machine method. Lambdas and local + // functions should never be virtual overrides in the first place. + bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM; + switch (dependencyKind) { + case DependencyKind.AccessedViaReflection: + case DependencyKind.DynamicallyAccessedMember: + if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (method, isCoveredByAnnotations)) + Context.LogWarning (origin, DiagnosticId.CompilerGeneratedMemberAccessedViaReflection, method.GetDisplayName ()); + break; } } diff --git a/src/linker/Linker/CompilerGeneratedState.cs b/src/linker/Linker/CompilerGeneratedState.cs index 349470e3a..c019d73fe 100644 --- a/src/linker/Linker/CompilerGeneratedState.cs +++ b/src/linker/Linker/CompilerGeneratedState.cs @@ -53,6 +53,14 @@ namespace Mono.Linker } } + public static bool IsHoistedLocal (FieldDefinition field) + { + // Treat all fields on compiler-generated types as hoisted locals. + // This avoids depending on the name mangling scheme for hoisted locals. + var declaringTypeName = field.DeclaringType.Name; + return CompilerGeneratedNames.IsLambdaDisplayClass (declaringTypeName) || CompilerGeneratedNames.IsStateMachineType (declaringTypeName); + } + // "Nested function" refers to lambdas and local functions. public static bool IsNestedFunctionOrStateMachineMember (IMemberDefinition member) { diff --git a/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index 2ac1bbcf6..73a0260f5 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -59,6 +59,12 @@ namespace ILLink.RoslynAnalyzer.Tests } [Fact] + public Task CompilerGeneratedCodeDataflow () + { + return RunTest (); + } + + [Fact] public Task CompilerGeneratedTypes () { return RunTest (); diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs index 1b5f65c7e..804f2a370 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs @@ -30,8 +30,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow class BaseTypeWithIteratorStateMachines { - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] public static IEnumerable<int> BaseIteratorWithCorrectDataflow () { var t = GetAll (); @@ -42,6 +40,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow [ExpectedWarning ("IL2120", "<" + nameof (BaseIteratorWithCorrectDataflow) + ">", "MoveNext", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2120", "<" + nameof (BaseIteratorWithCorrectDataflow) + ">", "<t>", + ProducedBy = ProducedBy.Trimmer)] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] class IteratorStateMachines : BaseTypeWithIteratorStateMachines { @@ -63,25 +63,33 @@ namespace Mono.Linker.Tests.Cases.DataFlow } [ExpectedWarning ("IL2119", "<" + nameof (IteratorWithCorrectDataflow) + ">", "MoveNext", CompilerGeneratedCode = true)] - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, - ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2119", "<t_IteratorWithCorrectDataflow>", CompilerGeneratedCode = true)] public static IEnumerable<int> IteratorWithCorrectDataflow () { - var t = GetAll (); + var t_IteratorWithCorrectDataflow = GetAll (); yield return 0; - t.RequiresAll (); + t_IteratorWithCorrectDataflow.RequiresAll (); + } + + [ExpectedWarning ("IL2119", "<" + nameof (IteratorWithIntegerDataflow) + ">", "MoveNext", CompilerGeneratedCode = true)] + [ExpectedWarning ("IL2119", "<types>", CompilerGeneratedCode = true)] + public static IEnumerable<int> IteratorWithIntegerDataflow () + { + int integerLocal = 0; + yield return 0; + var types = new Type[] { GetWithPublicMethods (), GetWithPublicFields () }; + types[integerLocal].RequiresPublicMethods (); } [ExpectedWarning ("IL2119", "<" + nameof (IteratorWithProblematicDataflow) + ">", "MoveNext", CompilerGeneratedCode = true)] - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, + [ExpectedWarning ("IL2119", "<t_IteratorWithProblematicDataflow>", CompilerGeneratedCode = true)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, ProducedBy = ProducedBy.Trimmer)] public static IEnumerable<int> IteratorWithProblematicDataflow () { - var t = GetWithPublicMethods (); + var t_IteratorWithProblematicDataflow = GetWithPublicMethods (); yield return 0; - t.RequiresAll (); + t_IteratorWithProblematicDataflow.RequiresAll (); } [ExpectedWarning ("IL2112", nameof (RUCTypeWithIterators) + "()", "--RUCTypeWithIterators--", CompilerGeneratedCode = true)] @@ -121,6 +129,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow ProducedBy = ProducedBy.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithCorrectDataflow) + ">", "MoveNext", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithIntegerDataflow) + ">", "MoveNext", + ProducedBy = ProducedBy.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (BaseIteratorWithCorrectDataflow) + ">", "MoveNext", ProducedBy = ProducedBy.Trimmer)] [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators) + "()", "--RUCTypeWithIterators--")] @@ -134,6 +144,18 @@ namespace Mono.Linker.Tests.Cases.DataFlow // With that, the IL2118 warning should also go away. [ExpectedWarning ("IL2118", "<" + nameof (RUCTypeWithIterators.InstanceIteratorCallsMethodWithRequires) + ">", "MoveNext", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithCorrectDataflow) + ">", "<t_IteratorWithCorrectDataflow>", + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithProblematicDataflow) + ">", "<t_IteratorWithProblematicDataflow>", + ProducedBy = ProducedBy.Trimmer)] + // Technically the access to IteratorWithIntegerDataflow should warn about access to the integer + // field integerLocal, but our heuristics only warn if the field type satisfies the + // "IsTypeInterestingForDatafllow" check. This is likely good enough because in most cases the + // compiler-generated code will have other hoisted fields with types that _are_ interesting for dataflow. + [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithIntegerDataflow) + ">", "<types>", + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (BaseIteratorWithCorrectDataflow) + ">", "<t>", + ProducedBy = ProducedBy.Trimmer)] public static void Test (IteratorStateMachines test = null) { typeof (IteratorStateMachines).RequiresAll (); @@ -156,22 +178,18 @@ namespace Mono.Linker.Tests.Cases.DataFlow MethodWithRequires (); } - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, - ProducedBy = ProducedBy.Trimmer)] public static async Task AsyncWithCorrectDataflow () { - var t = GetAll (); - t.RequiresAll (); + var t_AsyncWithCorrectDataflow = GetAll (); + t_AsyncWithCorrectDataflow.RequiresAll (); } - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, ProducedBy = ProducedBy.Trimmer)] public static async Task AsyncWithProblematicDataflow () { - var t = GetWithPublicMethods (); - t.RequiresAll (); + var t_AsyncWithProblematicDataflow = GetWithPublicMethods (); + t_AsyncWithProblematicDataflow.RequiresAll (); } [ExpectedWarning ("IL2118", "<" + nameof (AsyncWithProblematicDataflow) + ">", "MoveNext", @@ -180,6 +198,10 @@ namespace Mono.Linker.Tests.Cases.DataFlow ProducedBy = ProducedBy.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (AsyncWithCorrectDataflow) + ">", "MoveNext", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (AsyncWithCorrectDataflow) + ">", "<t_AsyncWithCorrectDataflow>", + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (AsyncWithProblematicDataflow) + ">", "<t_AsyncWithProblematicDataflow>", + ProducedBy = ProducedBy.Trimmer)] public static void Test () { typeof (AsyncStateMachines).RequiresAll (); @@ -202,9 +224,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow MethodWithRequires (); } - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, - ProducedBy = ProducedBy.Trimmer)] public static async IAsyncEnumerable<int> AsyncIteratorWithCorrectDataflow () { var t = GetAll (); @@ -212,9 +231,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow t.RequiresAll (); } - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), - nameof (AsyncIteratorWithProblematicDataflow), CompilerGeneratedCode = true, + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, ProducedBy = ProducedBy.Trimmer)] public static async IAsyncEnumerable<int> AsyncIteratorWithProblematicDataflow () { @@ -229,6 +246,10 @@ namespace Mono.Linker.Tests.Cases.DataFlow ProducedBy = ProducedBy.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (AsyncIteratorWithCorrectDataflow) + ">", "MoveNext", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (AsyncIteratorWithCorrectDataflow) + ">", "<t>", + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (AsyncIteratorWithProblematicDataflow) + ">", "<t>", + ProducedBy = ProducedBy.Trimmer)] public static void Test () { typeof (AsyncIteratorStateMachines).RequiresAll (); @@ -259,7 +280,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow static void LambdaWithCorrectDataflow () { var lambda = - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 [ExpectedWarning ("IL2119", "<" + nameof (LambdaWithCorrectDataflow) + ">", ProducedBy = ProducedBy.Trimmer)] () => { @@ -302,8 +322,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow var lambda = [ExpectedWarning ("IL2119", "<" + nameof (LambdaWithCapturedTypeToDAM) + ">", ProducedBy = ProducedBy.Trimmer)] - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), ProducedBy = ProducedBy.Trimmer)] () => { t.RequiresAll (); @@ -424,9 +443,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow var t = GetAll (); [ExpectedWarning ("IL2119", "<" + nameof (LocalFunctionWithCapturedTypeToDAM) + ">", ProducedBy = ProducedBy.Trimmer)] - // Annotations aren't propagated to hoisted locals: https://github.com/dotnet/linker/issues/2001 - [ExpectedWarning ("IL2077", nameof (DataFlowTypeExtensions.RequiresAll), - ProducedBy = ProducedBy.Trimmer)] void LocalFunction () { t.RequiresAll (); @@ -517,6 +533,9 @@ namespace Mono.Linker.Tests.Cases.DataFlow [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] static Type GetWithPublicMethods () => null; + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + static Type GetWithPublicFields () => null; + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] static Type GetAll () => null; } diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeDataflow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeDataflow.cs new file mode 100644 index 000000000..f42464e54 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeDataflow.cs @@ -0,0 +1,562 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.DataFlow +{ + [SkipKeptItemsValidation] + [SetupCompileArgument ("/unsafe")] + [ExpectedNoWarnings] + public class CompilerGeneratedCodeDataflow + { + public static void Main () + { + Iterator.Test (); + Async.Test (); + AsyncIterator.Test (); + LocalFunction.Test (); + Lambda.Test (); + Complex.Test (); + } + + class Iterator + { + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + CompilerGeneratedCode = true)] + static IEnumerable<int> FlowAcrossYieldReturn () + { + Type t = GetWithPublicMethods (); + yield return 0; + t.RequiresAll (); + } + + // Linker tracks all assignments of hoisted locals, so this produces warnings. + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresPublicFields), CompilerGeneratedCode = true)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresPublicMethods), CompilerGeneratedCode = true)] + static IEnumerable<int> NoFlowAcrossYieldReturn () + { + Type t = GetWithPublicMethods (); + t.RequiresPublicMethods (); + yield return 0; + t = GetWithPublicFields (); + t.RequiresPublicFields (); + } + + [ExpectedWarning ("IL2067", "publicMethodsType", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + static IEnumerable<int> UseParameterBeforeYieldReturn ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type publicMethodsType = null) + { + publicMethodsType.RequiresAll (); + yield return 0; + } + + [ExpectedWarning ("IL2067", "unknownType", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + static IEnumerable<int> UseUnannotatedParameterBeforeYieldReturn (Type unknownType = null) + { + unknownType.RequiresAll (); + yield return 0; + } + + [ExpectedWarning ("IL2067", "publicMethodsType", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + static IEnumerable<int> FlowParameterAcrossYieldReturn ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type publicMethodsType = null) + { + yield return 0; + publicMethodsType.RequiresAll (); + } + + [ExpectedWarning ("IL2067", "unknownType", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + static IEnumerable<int> FlowUnannotatedParameterAcrossYieldReturn (Type unknownType = null) + { + yield return 0; + unknownType.RequiresAll (); + } + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + // Linker includes backwards branches for hoisted locals, by virtue of tracking all assignments. + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + static IEnumerable<int> FlowAcrossYieldReturnWithBackwardsBranch (int n = 0) + { + Type t = GetWithPublicMethods (); + for (int i = 0; i < n; i++) { + yield return 0; + t.RequiresAll (); + yield return 1; + t = GetWithPublicFields (); + } + } + + public static void Test () + { + FlowAcrossYieldReturn (); + NoFlowAcrossYieldReturn (); + NoFlowAcrossYieldReturn (); + UseParameterBeforeYieldReturn (); + UseUnannotatedParameterBeforeYieldReturn (); + FlowParameterAcrossYieldReturn (); + FlowUnannotatedParameterAcrossYieldReturn (); + FlowAcrossYieldReturnWithBackwardsBranch (); + } + } + + class Async + { + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + CompilerGeneratedCode = true)] + static async void FlowAcrossAwait () + { + Type t = GetWithPublicMethods (); + await MethodAsync (); + t.RequiresAll (); + } + + // Linker tracks all assignments of hoisted locals, so this produces warnings. + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresPublicFields), CompilerGeneratedCode = true)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresPublicMethods), CompilerGeneratedCode = true)] + static async void NoFlowAcrossAwait () + { + Type t = GetWithPublicMethods (); + t.RequiresPublicMethods (); + await MethodAsync (); + t = GetWithPublicFields (); + t.RequiresPublicFields (); + } + + [ExpectedWarning ("IL2067", "publicMethodsType", nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + static async void FlowParameterAcrossAwait ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type publicMethodsType = null) + { + await MethodAsync (); + publicMethodsType.RequiresAll (); + } + + public static void Test () + { + FlowAcrossAwait (); + NoFlowAcrossAwait (); + FlowParameterAcrossAwait (); + } + } + + class AsyncIterator + { + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, + ProducedBy = ProducedBy.Trimmer)] + static async IAsyncEnumerable<int> FlowAcrossAwaitAndYieldReturn () + { + Type t = GetWithPublicMethods (); + await MethodAsync (); + yield return 0; + t.RequiresAll (); + } + + public static void Test () + { + FlowAcrossAwaitAndYieldReturn (); + } + } + + class LocalFunction + { + static void WarningsInBody () + { + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + static void LocalFunction () + { + Type t = GetWithPublicMethods (); + t.RequiresAll (); + } + + LocalFunction (); + } + + static void WarningsInBodyUnused () + { + // Trimmer doesn't warn because this is unused code. + static void LocalFunction () + { + Type t = GetWithPublicMethods (); + t.RequiresAll (); + } + } + + static void ReadCapturedVariable () + { + Type t = GetWithPublicMethods (); + LocalFunction (); + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () + { + t.RequiresAll (); + } + } + + static void ReadMergedCapturedVariable (bool b = false) + { + Type t; + if (b) { + t = GetWithPublicMethods (); + } else { + t = GetWithPublicFields (); + } + + LocalFunction (); + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => t.RequiresAll (); + } + + static void ReadCapturedVariableInMultipleBranches (bool b = false) + { + Type t; + if (b) { + t = GetWithPublicMethods (); + LocalFunction (); + } else { + t = GetWithPublicFields (); + LocalFunction (); + } + + LocalFunction (); + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => t.RequiresAll (); + } + + static void ReadCapturedVariableInMultipleBranchesDistinct (bool b = false) + { + Type t; + if (b) { + t = GetWithPublicMethods (); + LocalFunctionRequiresMethods (); + } else { + t = GetWithPublicFields (); + LocalFunctionRequiresFields (); + } + + // We include all writes, including ones that can't reach the local function invocation. + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresPublicFields), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunctionRequiresFields () => t.RequiresPublicFields (); + // We include all writes, including ones that can't reach the local function invocation. + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresPublicMethods), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunctionRequiresMethods () => t.RequiresPublicMethods (); + } + + static void ReadCapturedVariableWithBackwardsBranch (int i = 0) + { + Type t = GetWithPublicMethods (); + while (true) { + LocalFunction (); + if (i++ == 5) + break; + t = GetWithPublicFields (); + } + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + // Linker includes backwards branches for hoisted locals, by virtue of tracking all assignments. + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => t.RequiresAll (); + } + + static void ReadCapturedVariableInMultipleFunctions () + { + Type t = GetWithPublicMethods (); + LocalFunction (); + + CallerOfLocalFunction (); + + void CallerOfLocalFunction () + { + t = GetWithPublicFields (); + LocalFunction (); + } + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => t.RequiresAll (); + } + + static void ReadCapturedVariableInCallGraphCycle () + { + Type t = GetUnknownType (); + A (); + + void A () + { + t = GetWithPublicMethods (); + LocalFunction (); + B (); + } + + void B () + { + t = GetWithPublicFields (); + LocalFunction (); + A (); + } + + [ExpectedWarning ("IL2072", nameof (GetUnknownType), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => t.RequiresAll (); + } + + public static void ReadCapturedParameter (Type tParameter) + { + LocalFunction (); + + [ExpectedWarning ("IL2067", nameof (ReadCapturedParameter), "tParameter", nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => tParameter.RequiresAll (); + } + + public static void ReadCapturedParameterAfterWrite (Type tParameter) + { + tParameter = GetWithPublicMethods (); + LocalFunction (); + + [ExpectedWarning ("IL2067", "tParameter", nameof (DataFlowTypeExtensions.RequiresPublicMethods), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => tParameter.RequiresPublicMethods (); + } + + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + // analyzer clears all local state, but trimmer doesn't + ProducedBy = ProducedBy.Trimmer)] + static void ReadCapturedVariableWithUnhoistedLocals () + { + Type t = GetWithPublicMethods (); + Type notCaptured = GetWithPublicFields (); + LocalFunction (); + notCaptured.RequiresAll (); + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => t.RequiresAll (); + } + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + // We include all writes, including ones that can't reach the local function invocation. + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + static void WriteCapturedVariable () + { + Type t = GetWithPublicFields (); + LocalFunction (); + t.RequiresAll (); + + void LocalFunction () => t = GetWithPublicMethods (); + } + + public static void Test () + { + WarningsInBody (); + WarningsInBodyUnused (); + ReadCapturedVariable (); + ReadMergedCapturedVariable (); + ReadCapturedVariableInMultipleBranches (); + ReadCapturedVariableInMultipleBranchesDistinct (); + ReadCapturedVariableInMultipleFunctions (); + ReadCapturedVariableInCallGraphCycle (); + ReadCapturedVariableWithBackwardsBranch (); + ReadCapturedParameter (null); + ReadCapturedParameterAfterWrite (null); + ReadCapturedVariableWithUnhoistedLocals (); + WriteCapturedVariable (); + } + } + + class Lambda + { + static void WarningsInBody () + { + var lambda = + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + () => { + Type t = GetWithPublicMethods (); + t.RequiresAll (); + }; + + lambda (); + } + + static void WarningsInBodyUnused () + { + var lambda = + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + () => { + Type t = GetWithPublicMethods (); + t.RequiresAll (); + }; + } + + static void ReadCapturedVariable () + { + Type t = GetWithPublicMethods (); + Action lambda = + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + () => t.RequiresAll (); + lambda (); + } + + static void ReadCapturedVariableAfterWriteAfterDefinition () + { + Type t = GetWithPublicFields (); + + Action lambda = + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + () => t.RequiresAll (); + + t = GetWithPublicMethods (); + lambda (); + } + + public static void ReadCapturedParameter (Type tParameter) + { + var lambda = + [ExpectedWarning ("IL2067", nameof (ReadCapturedParameter), "tParameter", nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + () => tParameter.RequiresAll (); + + lambda (); + } + + public static void ReadCapturedParameterAfterWrite (Type tParameter) + { + tParameter = GetWithPublicMethods (); + var lambda = + // We produce dataflow warnings for the unknown parameter even though it has been overwritten + // with a value that satisfies the requirement. + [ExpectedWarning ("IL2067", "tParameter", nameof (DataFlowTypeExtensions.RequiresPublicMethods), + ProducedBy = ProducedBy.Trimmer)] + () => tParameter.RequiresPublicMethods (); + lambda (); + } + + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + static void ReadCapturedVariableWithUnhoistedLocals () + { + Type t = GetWithPublicMethods (); + Type notCaptured = GetWithPublicFields (); + Action lambda = + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + () => t.RequiresAll (); + lambda (); + notCaptured.RequiresAll (); + } + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + // We include all writes, including ones that can't reach the local function invocation. + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + static void WriteCapturedVariable () + { + Type t = GetWithPublicFields (); + Action lambda = () => t = GetWithPublicMethods (); + lambda (); + t.RequiresAll (); + } + + public static void Test () + { + WarningsInBody (); + WarningsInBodyUnused (); + ReadCapturedVariable (); + ReadCapturedVariableAfterWriteAfterDefinition (); + ReadCapturedParameter (null); + ReadCapturedParameterAfterWrite (null); + ReadCapturedVariableWithUnhoistedLocals (); + WriteCapturedVariable (); + } + } + + class Complex + { + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, + ProducedBy = ProducedBy.Trimmer)] + // Linker merges branches going forward + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true, + ProducedBy = ProducedBy.Trimmer)] + static IEnumerable<int> IteratorWithLocalFunctions () + { + Type t = GetWithPublicMethods (); + LocalFunction (); + + yield return 0; + + LocalFunction (); + t = GetWithPublicFields (); + LocalFunction (); + t.RequiresAll (); + + [ExpectedWarning ("IL2072", nameof (GetWithPublicMethods), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), + ProducedBy = ProducedBy.Trimmer)] + void LocalFunction () => t.RequiresAll (); + } + + public static void Test () + { + IteratorWithLocalFunctions (); + } + } + + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] + static Type GetAll () => null; + + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + static Type GetWithPublicMethods () => null; + + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + static Type GetWithPublicFields () => null; + + static Type GetUnknownType () => null; + + static async Task<int> MethodAsync () + { + return await Task.FromResult (0); + } + + [RequiresUnreferencedCode ("RUC")] + static void RUCMethod () { } + + struct TestStruct + { + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + public Type TypeWithPublicFields => null; + } + } +}
\ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Reflection/ExpressionPropertyMethodInfo.cs b/test/Mono.Linker.Tests.Cases/Reflection/ExpressionPropertyMethodInfo.cs index 91a1b345f..95447aa6a 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/ExpressionPropertyMethodInfo.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/ExpressionPropertyMethodInfo.cs @@ -89,11 +89,11 @@ namespace Mono.Linker.Tests.Cases.Reflection [Kept] // https://github.com/dotnet/linker/issues/2669 [ExpectedWarning ("IL2026", nameof (StaticPropertyExpressionAccess), ProducedBy = ProducedBy.Trimmer)] - [ExpectedWarning ("IL2026", nameof (StaticPropertyViaReflection))] - [ExpectedWarning ("IL2026", nameof (StaticPropertyViaRuntimeMethod))] + [ExpectedWarning ("IL2026", nameof (StaticPropertyViaReflection), ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2026", nameof (StaticPropertyViaRuntimeMethod), ProducedBy = ProducedBy.Trimmer)] // https://github.com/dotnet/linker/issues/2669 [ExpectedWarning ("IL2026", nameof (InstancePropertyExpressionAccess), ProducedBy = ProducedBy.Trimmer)] - [ExpectedWarning ("IL2026", nameof (InstancePropertyViaReflection))] + [ExpectedWarning ("IL2026", nameof (InstancePropertyViaReflection), ProducedBy = ProducedBy.Trimmer)] public static void Test () { Expression<Func<int>> staticGetter = () => StaticPropertyExpressionAccess; diff --git a/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/SuppressWarningsInCompilerGeneratedCode.cs b/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/SuppressWarningsInCompilerGeneratedCode.cs index 0c9bd06b1..c0f89607b 100644 --- a/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/SuppressWarningsInCompilerGeneratedCode.cs +++ b/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/SuppressWarningsInCompilerGeneratedCode.cs @@ -55,8 +55,7 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression yield return 0; } - // See https://github.com/dotnet/linker/issues/2587 - [UnconditionalSuppressMessage ("Linker sees the compiler generated property", "IL2077")] + [UnconditionalSuppressMessage ("Test", "IL2067")] static IEnumerable<int> TestMethodParameterWithRequirements (Type unknownType = null) { unknownType.RequiresNonPublicMethods (); @@ -122,8 +121,7 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression await MethodAsync (); } - // See https://github.com/dotnet/linker/issues/2587 - [UnconditionalSuppressMessage ("Linker sees the compiler generated property", "IL2077")] + [UnconditionalSuppressMessage ("Test", "IL2067")] static async void TestMethodParameterWithRequirements (Type unknownType = null) { unknownType.RequiresNonPublicMethods (); @@ -199,7 +197,7 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression void LocalFunction () => typeof (TypeWithRUCMethod).RequiresNonPublicMethods (); } - [UnconditionalSuppressMessage ("Test", "IL2077")] + [UnconditionalSuppressMessage ("Test", "IL2067")] static void TestMethodParameterWithRequirements (Type unknownType = null) { LocalFunction (); @@ -403,7 +401,7 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression () => typeof (TypeWithRUCMethod).RequiresNonPublicMethods (); } - [UnconditionalSuppressMessage ("Test", "IL2077")] + [UnconditionalSuppressMessage ("Test", "IL2067")] static void TestMethodParameterWithRequirements (Type unknownType = null) { Action _ = diff --git a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs index fad77080a..bd611721c 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs @@ -776,11 +776,10 @@ namespace Mono.Linker.Tests.TestCasesRunner continue; } } else if (isCompilerGeneratedCode == true) { - if (loggedMessage.Origin?.Provider is MethodDefinition methodDefinition) { + if (loggedMessage.Origin?.Provider is IMemberDefinition memberDefinition) { if (attrProvider is not IMemberDefinition expectedMember) continue; - - string actualName = methodDefinition.DeclaringType.FullName + "." + methodDefinition.Name; + string actualName = memberDefinition.DeclaringType.FullName + "." + memberDefinition.Name; if (actualName.StartsWith (expectedMember.DeclaringType.FullName) && actualName.Contains ("<" + expectedMember.Name + ">")) { @@ -788,14 +787,16 @@ namespace Mono.Linker.Tests.TestCasesRunner loggedMessages.Remove (loggedMessage); break; } + if (memberDefinition is not MethodDefinition) + continue; if (actualName.StartsWith (expectedMember.DeclaringType.FullName) && actualName.Contains (".cctor") && (expectedMember is FieldDefinition || expectedMember is PropertyDefinition)) { expectedWarningFound = true; loggedMessages.Remove (loggedMessage); break; } - if (methodDefinition.Name == ".ctor" && - methodDefinition.DeclaringType.FullName == expectedMember.FullName) { + if (memberDefinition.Name == ".ctor" && + memberDefinition.DeclaringType.FullName == expectedMember.FullName) { expectedWarningFound = true; loggedMessages.Remove (loggedMessage); break; @@ -833,13 +834,14 @@ namespace Mono.Linker.Tests.TestCasesRunner } break; - case nameof (ExpectedNoWarningsAttribute): - // Postpone processing of negative checks, to make it possible to mark some warnings as expected (will be removed from the list above) - // and then do the negative check on the rest. - var memberDefinition = attrProvider as IMemberDefinition; - Assert.NotNull (memberDefinition); - expectedNoWarningsAttributes.Add ((memberDefinition, attr)); - break; + case nameof (ExpectedNoWarningsAttribute): { + // Postpone processing of negative checks, to make it possible to mark some warnings as expected (will be removed from the list above) + // and then do the negative check on the rest. + var memberDefinition = attrProvider as IMemberDefinition; + Assert.NotNull (memberDefinition); + expectedNoWarningsAttributes.Add ((memberDefinition, attr)); + break; + } } } } |