From 23d4cde14a322a881f22910648b065faf0bb47bb Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 27 Nov 2023 14:21:47 -0800 Subject: [PATCH] Fix ILLink/ILC hang when tracking too many hoisted local values (#95041) https://github.com/dotnet/runtime/pull/87634 introduced a limit on the number of values tracked in dataflow analysis, but this approach was incorrect because resetting the set of tracked values was effectively moving up the dataflow lattice, breaking the invariant and causing potential hangs. The hang was observed in https://github.com/dotnet/runtime/issues/94831, where (as found by @vitek-karas) the hoisted local `state` field of an async method with many await points was getting a large number of tracked values, hitting the limit, being reset to "empty", but then growing again, causing the analysis not to converge. The fix is to instead introduce a special value `ValueSet.Unknown` that is used for this case. It acts like "bottom" in the lattice, so that it destroys any other inputs on meet ("bottom" meet anything else is "bottom"). In this testcase the `state` field isn't involved in dataflow warnings, so this actually allows the method to be analyzed correctly, producing the expected warning for the `Type` local that flows across all of the await points. Fixes https://github.com/dotnet/runtime/issues/94831 --- .../Compiler/Dataflow/ArrayValue.cs | 6 +- .../Compiler/Dataflow/InterproceduralState.cs | 3 +- .../Compiler/Dataflow/MethodBodyScanner.cs | 18 ++-- .../Dataflow/ReflectionMethodBodyScanner.cs | 10 +- .../Dataflow/TrimAnalysisAssignmentPattern.cs | 4 +- .../DataFlow/InterproceduralState.cs | 4 +- .../DataFlow/LocalDataFlowAnalysis.cs | 3 +- .../DataFlow/LocalDataFlowVisitor.cs | 10 +- .../TrimAnalysis/ArrayValue.cs | 4 +- .../TrimAnalysisAssignmentPattern.cs | 4 +- .../TrimAnalysis/TrimAnalysisVisitor.cs | 12 +-- .../src/ILLink.Shared/DataFlow/ValueSet.cs | 97 ++++++++++++++----- .../ILLink.Shared/TrimAnalysis/ArrayValue.cs | 4 +- .../TrimAnalysis/HandleCallAction.cs | 70 ++++++------- ...RequireDynamicallyAccessedMembersAction.cs | 2 +- .../TrimAnalysis/ValueExtensions.cs | 16 ++- .../src/linker/Linker.Dataflow/ArrayValue.cs | 6 +- .../Linker.Dataflow/InterproceduralState.cs | 3 +- .../Linker.Dataflow/MethodBodyScanner.cs | 17 ++-- .../ReflectionMethodBodyScanner.cs | 4 +- .../TrimAnalysisAssignmentPattern.cs | 4 +- .../DataFlow/ExponentialDataFlow.cs | 82 +++++++++++++++- 22 files changed, 267 insertions(+), 116 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ArrayValue.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ArrayValue.cs index 2676873572dc1..beb0d60d4799a 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ArrayValue.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ArrayValue.cs @@ -19,7 +19,7 @@ internal partial record ArrayValue public static MultiValue Create(MultiValue size, TypeDesc elementType) { MultiValue result = MultiValueLattice.Top; - foreach (var sizeValue in size) + foreach (var sizeValue in size.AsEnumerable ()) { result = MultiValueLattice.Meet(result, new MultiValue(new ArrayValue(sizeValue, elementType))); } @@ -92,7 +92,7 @@ public override SingleValue DeepCopy() // Since it's possible to store a reference to array as one of its own elements // simple deep copy could lead to endless recursion. // So instead we simply disallow arrays as element values completely - and treat that case as "too complex to analyze". - foreach (SingleValue v in kvp.Value.Value) + foreach (SingleValue v in kvp.Value.Value.AsEnumerable ()) { System.Diagnostics.Debug.Assert(v is not ArrayValue); } @@ -123,7 +123,7 @@ public override string ToString() result.Append(element.Key); result.Append(",("); bool firstValue = true; - foreach (var v in element.Value.Value) + foreach (var v in element.Value.Value.AsEnumerable ()) { if (firstValue) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/InterproceduralState.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/InterproceduralState.cs index 6a322caa148db..7627312e81550 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/InterproceduralState.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/InterproceduralState.cs @@ -67,7 +67,8 @@ public void TrackMethod(MethodIL methodBody) methodBody = GetInstantiatedMethodIL(methodBody); // Work around the fact that ValueSet is readonly - var methodsList = new List(MethodBodies); + Debug.Assert (!MethodBodies.IsUnknown ()); + var methodsList = new List(MethodBodies.GetKnownValues ()); methodsList.Add(new MethodBodyValue(methodBody)); // For state machine methods, also scan the state machine members. diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs index deb9474117f47..fa6378f736131 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs @@ -210,7 +210,7 @@ private static void ValidateNoReferenceToReference(ValueBasicBlockPair?[] locals continue; MultiValue localValue = localVariable.Value.Value; - foreach (var val in localValue) + foreach (var val in localValue.AsEnumerable ()) { if (val is LocalVariableReferenceValue localReference && localReference.ReferencedType.IsByRefOrPointer()) { @@ -309,7 +309,8 @@ public virtual void InterproceduralScan(MethodIL startingMethodBody) // 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) + Debug.Assert (!oldInterproceduralState.MethodBodies.IsUnknown ()); + foreach (var methodBodyValue in oldInterproceduralState.MethodBodies.GetKnownValues ()) Scan(methodBodyValue.MethodBody, ref interproceduralState); } @@ -327,7 +328,8 @@ public virtual void InterproceduralScan(MethodIL startingMethodBody) } else { - Debug.Assert(interproceduralState.MethodBodies.Count() == 1); + Debug.Assert (!interproceduralState.MethodBodies.IsUnknown ()); + Debug.Assert(interproceduralState.MethodBodies.GetKnownValues ().Count() == 1); } #endif } @@ -1018,7 +1020,7 @@ private void ScanIndirectStore( /// Throws if is not a valid target for an indirect store. protected void StoreInReference(MultiValue target, MultiValue source, MethodIL method, int offset, ValueBasicBlockPair?[] locals, int curBasicBlock, ref InterproceduralState ipState) { - foreach (var value in target) + foreach (var value in target.AsEnumerable ()) { switch (value) { @@ -1137,7 +1139,7 @@ private void ScanStfld( return; } - foreach (var value in HandleGetField(methodBody, offset, field)) + foreach (var value in HandleGetField(methodBody, offset, field).AsEnumerable ()) { // GetFieldValue may return different node types, in which case they can't be stored to. // At least not yet. @@ -1189,7 +1191,7 @@ internal MultiValue DereferenceValue( ref InterproceduralState interproceduralState) { MultiValue dereferencedValue = MultiValueLattice.Top; - foreach (var value in maybeReferenceValue) + foreach (var value in maybeReferenceValue.AsEnumerable ()) { switch (value) { @@ -1324,7 +1326,7 @@ private void HandleCall( foreach (var param in methodArguments) { - foreach (var v in param) + foreach (var v in param.AsEnumerable ()) { if (v is ArrayValue arr) { @@ -1366,7 +1368,7 @@ private void ScanStelem( StackSlot indexToStoreAt = PopUnknown(currentStack, 1, methodBody, offset); StackSlot arrayToStoreIn = PopUnknown(currentStack, 1, methodBody, offset); int? indexToStoreAtInt = indexToStoreAt.Value.AsConstInt(); - foreach (var array in arrayToStoreIn.Value) + foreach (var array in arrayToStoreIn.Value.AsEnumerable ()) { if (array is ArrayValue arrValue) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs index d11569a98d568..9c1fe23f8db18 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -431,7 +431,7 @@ public static bool HandleCall( // type instead). // // At least until we have shared enum code, this needs extra handling to get it right. - foreach (var value in argumentValues[0]) + foreach (var value in argumentValues[0].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue && !systemTypeValue.RepresentedType.Type.IsGenericDefinition @@ -466,7 +466,7 @@ public static bool HandleCall( ? 0 : 1; // We need the data to do struct marshalling. - foreach (var value in argumentValues[paramIndex]) + foreach (var value in argumentValues[paramIndex].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue && !systemTypeValue.RepresentedType.Type.IsGenericDefinition @@ -497,7 +497,7 @@ public static bool HandleCall( case IntrinsicId.Marshal_GetDelegateForFunctionPointer: { // We need the data to do delegate marshalling. - foreach (var value in argumentValues[1]) + foreach (var value in argumentValues[1].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue && !systemTypeValue.RepresentedType.Type.IsGenericDefinition @@ -521,7 +521,7 @@ public static bool HandleCall( // case IntrinsicId.Object_GetType: { - foreach (var valueNode in instanceValue) + foreach (var valueNode in instanceValue.AsEnumerable ()) { // Note that valueNode can be statically typed in IL as some generic argument type. // For example: @@ -619,7 +619,7 @@ public static bool HandleCall( // Validate that the return value has the correct annotations as per the method return value annotations if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0) { - foreach (var uniqueValue in methodReturnValue) + foreach (var uniqueValue in methodReturnValue.AsEnumerable ()) { if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisAssignmentPattern.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisAssignmentPattern.cs index e9ce2078896cc..cbf83266e144e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisAssignmentPattern.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisAssignmentPattern.cs @@ -48,9 +48,9 @@ public void MarkAndProduceDiagnostics(ReflectionMarker reflectionMarker, Logger logger.ShouldSuppressAnalysisWarningsForRequires(Origin.MemberDefinition, DiagnosticUtilities.RequiresAssemblyFilesAttribute), logger); - foreach (var sourceValue in Source) + foreach (var sourceValue in Source.AsEnumerable ()) { - foreach (var targetValue in Target) + foreach (var targetValue in Target.AsEnumerable ()) { if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers) throw new NotImplementedException(); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/InterproceduralState.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/InterproceduralState.cs index 875f4e1e0acf9..2fac8f0540adc 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/InterproceduralState.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/InterproceduralState.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using ILLink.Shared.DataFlow; @@ -50,7 +51,8 @@ public InterproceduralState Clone () public void TrackMethod (MethodBodyValue method) { - var methodsList = new List (Methods); + Debug.Assert (!Methods.IsUnknown ()); + var methodsList = new List (Methods.GetKnownValues ()); methodsList.Add (method); Methods = new ValueSet (methodsList); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs index 80629aee5b10f..741b0a31a49c1 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs @@ -62,7 +62,8 @@ public void InterproceduralAnalyze () while (!interproceduralState.Equals (oldInterproceduralState)) { oldInterproceduralState = interproceduralState.Clone (); - foreach (var method in oldInterproceduralState.Methods) { + Debug.Assert (!oldInterproceduralState.Methods.IsUnknown ()); + foreach (var method in oldInterproceduralState.Methods.GetKnownValues ()) { if (method.Method.IsInRequiresUnreferencedCodeAttributeScope (out _)) continue; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index 80ce7a66f4d23..2c6c74db55488 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -253,7 +253,7 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, ISimpleAssignm // Single captured reference. Treat this as an overwriting assignment, // unless the caller already told us to merge values because this is an // assignment to one of multiple captured array element references. - var enumerator = capturedReferences.GetEnumerator (); + var enumerator = capturedReferences.GetKnownValues ().GetEnumerator (); enumerator.MoveNext (); var capture = enumerator.Current; arrayRef = Visit (capture.Reference, state); @@ -266,7 +266,8 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, ISimpleAssignm // We treat this as possible write to each of the captured references, // which requires merging with the previous values of each. - foreach (var capture in state.Current.CapturedReferences.Get (captureReference.Id)) { + Debug.Assert (!capturedReferences.IsUnknown ()); + foreach (var capture in capturedReferences.GetKnownValues ()) { arrayRef = Visit (capture.Reference, state); HandleArrayElementWrite (arrayRef, index, value, operation, merge: true); } @@ -330,9 +331,10 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati Debug.Assert (IsLValueFlowCapture (flowCaptureReference.Id)); Debug.Assert (!flowCaptureReference.GetValueUsageInfo (Method).HasFlag (ValueUsageInfo.Read)); var capturedReferences = state.Current.CapturedReferences.Get (flowCaptureReference.Id); + Debug.Assert (!capturedReferences.IsUnknown ()); if (!capturedReferences.HasMultipleValues) { // Single captured reference. Treat this as an overwriting assignment. - var enumerator = capturedReferences.GetEnumerator (); + var enumerator = capturedReferences.GetKnownValues ().GetEnumerator (); enumerator.MoveNext (); targetOperation = enumerator.Current.Reference; return ProcessSingleTargetAssignment (targetOperation, operation, state, merge: false); @@ -349,7 +351,7 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati // if the RHS has dataflow warnings. TValue value = TopValue; - foreach (var capturedReference in capturedReferences) { + foreach (var capturedReference in capturedReferences.GetKnownValues ()) { targetOperation = capturedReference.Reference; var singleValue = ProcessSingleTargetAssignment (targetOperation, operation, state, merge: true); value = LocalStateLattice.Lattice.ValueLattice.Meet (value, singleValue); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ArrayValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ArrayValue.cs index 58ebbff0bc1f0..5a4bb0f555965 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ArrayValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ArrayValue.cs @@ -14,7 +14,7 @@ internal partial record ArrayValue public static MultiValue Create (MultiValue size) { MultiValue result = MultiValueLattice.Top; - foreach (var sizeValue in size) { + foreach (var sizeValue in size.AsEnumerable ()) { result = MultiValueLattice.Meet (result, new MultiValue (new ArrayValue (sizeValue))); } @@ -73,7 +73,7 @@ public override SingleValue DeepCopy () // Since it's possible to store a reference to array as one of its own elements // simple deep copy could lead to endless recursion. // So instead we simply disallow arrays as element values completely - and treat that case as "too complex to analyze". - foreach (SingleValue v in kvp.Value) { + foreach (SingleValue v in kvp.Value.AsEnumerable ()) { System.Diagnostics.Debug.Assert (v is not ArrayValue); } #endif diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs index 9a41f47cd9a52..2c2f94d8b2aa0 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs @@ -38,8 +38,8 @@ public TrimAnalysisAssignmentPattern Merge (ValueSetLattice lattice public IEnumerable CollectDiagnostics () { var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); - foreach (var sourceValue in Source) { - foreach (var targetValue in Target) { + foreach (var sourceValue in Source.AsEnumerable ()) { + foreach (var targetValue in Target.AsEnumerable ()) { // The target should always be an annotated value, but the visitor design currently prevents // declaring this in the type system. if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 04c7b06fa048e..b960e2a461f97 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -77,7 +77,7 @@ public override MultiValue VisitArrayCreation (IArrayCreationOperation operation var arrayValue = ArrayValue.Create (Visit (operation.DimensionSizes[0], state)); var elements = operation.Initializer?.ElementValues.Select (val => Visit (val, state)).ToArray () ?? System.Array.Empty (); - foreach (var array in arrayValue.Cast ()) { + foreach (var array in arrayValue.AsEnumerable ().Cast ()) { for (int i = 0; i < elements.Length; i++) { array.IndexValues.Add (i, ArrayValue.SanitizeArrayElementValue(elements[i])); } @@ -155,11 +155,11 @@ operation.OperatorMethod is null && MultiValue rightValue = Visit (operation.RightOperand, argument); MultiValue result = TopValue; - foreach (var left in leftValue) { + foreach (var left in leftValue.AsEnumerable ()) { if (left is UnknownValue) result = _multiValueLattice.Meet (result, left); else if (left is ConstIntValue leftConstInt) { - foreach (var right in rightValue) { + foreach (var right in rightValue.AsEnumerable ()) { if (right is UnknownValue) result = _multiValueLattice.Meet (result, right); else if (right is ConstIntValue rightConstInt) { @@ -210,7 +210,7 @@ public override MultiValue HandleArrayElementRead (MultiValue arrayValue, MultiV return UnknownValue.Instance; MultiValue result = TopValue; - foreach (var value in arrayValue) { + foreach (var value in arrayValue.AsEnumerable ()) { if (value is ArrayValue arr && arr.TryGetValueByIndex (index, out var elementValue)) result = _multiValueLattice.Meet (result, elementValue); else @@ -222,7 +222,7 @@ public override MultiValue HandleArrayElementRead (MultiValue arrayValue, MultiV public override void HandleArrayElementWrite (MultiValue arrayValue, MultiValue indexValue, MultiValue valueToWrite, IOperation operation, bool merge) { int? index = indexValue.AsConstInt (); - foreach (var arraySingleValue in arrayValue) { + foreach (var arraySingleValue in arrayValue.AsEnumerable ()) { if (arraySingleValue is ArrayValue arr) { if (index == null) { // Reset the array to all unknowns - since we don't know which index is being assigned @@ -282,7 +282,7 @@ public override MultiValue HandleMethodCall (IMethodSymbol calledMethod, MultiVa Method)); foreach (var argument in arguments) { - foreach (var argumentValue in argument) { + foreach (var argumentValue in argument.AsEnumerable ()) { if (argumentValue is ArrayValue arrayValue) arrayValue.IndexValues.Clear (); } diff --git a/src/tools/illink/src/ILLink.Shared/DataFlow/ValueSet.cs b/src/tools/illink/src/ILLink.Shared/DataFlow/ValueSet.cs index d538d358e8cea..f8229a4173e9f 100644 --- a/src/tools/illink/src/ILLink.Shared/DataFlow/ValueSet.cs +++ b/src/tools/illink/src/ILLink.Shared/DataFlow/ValueSet.cs @@ -4,6 +4,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Text; @@ -12,11 +13,21 @@ namespace ILLink.Shared.DataFlow { - public readonly struct ValueSet : IEquatable>, IEnumerable, IDeepCopyValue> + public readonly struct ValueSet : IEquatable>, IDeepCopyValue> where TValue : notnull { const int MaxValuesInSet = 256; + public static readonly ValueSet Empty; + + private sealed class ValueSetSentinel + { + } + + private static readonly ValueSetSentinel UnknownSentinel = new (); + + public static readonly ValueSet Unknown = new (UnknownSentinel); + // Since we're going to do lot of type checks for this class a lot, it is much more efficient // if the class is sealed (as then the runtime can do a simple method table pointer comparison) private sealed class EnumerableValues : HashSet @@ -117,11 +128,25 @@ public void Reset () } } + public readonly struct Enumerable : IEnumerable + { + private readonly object? _values; + + public Enumerable (object? values) => _values = values; + + public Enumerator GetEnumerator () => new (_values); + + IEnumerator IEnumerable.GetEnumerator () => GetEnumerator (); + + IEnumerator IEnumerable.GetEnumerator () => GetEnumerator (); + } + // This stores the values. By far the most common case will be either no values, or a single value. // Cases where there are multiple values stored are relatively very rare. // null - no values (empty set) // TValue - single value itself // EnumerableValues typed object - multiple values, stored in the hashset + // ValueSetSentinel.Unknown - unknown value, or "any possible value" private readonly object? _values; public ValueSet (TValue value) => _values = value; @@ -130,8 +155,11 @@ public void Reset () private ValueSet (EnumerableValues values) => _values = values; + private ValueSet (ValueSetSentinel sentinel) => _values = sentinel; + public static implicit operator ValueSet (TValue value) => new (value); + // Note: returns false for Unknown public bool HasMultipleValues => _values is EnumerableValues; public override bool Equals (object? obj) => obj is ValueSet other && Equals (other); @@ -146,14 +174,24 @@ public bool Equals (ValueSet other) if (_values is EnumerableValues enumerableValues) { if (other._values is EnumerableValues otherValuesSet) { return enumerableValues.Equals (otherValuesSet); - } else - return enumerableValues.Equals ((TValue) other._values); - } else { + } else if (other._values is TValue otherValue) { + return enumerableValues.Equals (otherValue); + } else { + Debug.Assert (other._values == UnknownSentinel); + return false; + } + } else if (_values is TValue value) { if (other._values is EnumerableValues otherEnumerableValues) { - return otherEnumerableValues.Equals ((TValue) _values); + return otherEnumerableValues.Equals (value); + } else if (other._values is TValue otherValue) { + return EqualityComparer.Default.Equals (value, otherValue); + } else { + Debug.Assert (other._values == UnknownSentinel); + return false; } - - return EqualityComparer.Default.Equals ((TValue) _values, (TValue) other._values); + } else { + Debug.Assert (_values == UnknownSentinel); + return other._values == UnknownSentinel; } } @@ -171,17 +209,20 @@ public override int GetHashCode () return _values.GetHashCode (); } - public Enumerator GetEnumerator () => new (_values); - - IEnumerator IEnumerable.GetEnumerator () => GetEnumerator (); - - IEnumerator IEnumerable.GetEnumerator () => GetEnumerator (); + public Enumerable GetKnownValues () => new Enumerable (_values == UnknownSentinel ? null : _values); - public bool Contains (TValue value) => _values is null - ? false - : _values is EnumerableValues valuesSet - ? valuesSet.Contains (value) - : EqualityComparer.Default.Equals (value, (TValue) _values); + // Note: returns false for Unknown + public bool Contains (TValue value) + { + if (_values is null) + return false; + if (_values is EnumerableValues valuesSet) + return valuesSet.Contains (value); + if (_values is TValue thisValue) + return EqualityComparer.Default.Equals (value, thisValue); + Debug.Assert (_values == UnknownSentinel); + return false; + } internal static ValueSet Meet (ValueSet left, ValueSet right) { @@ -190,28 +231,35 @@ internal static ValueSet Meet (ValueSet left, ValueSet r if (right._values == null) return left.DeepCopy (); + if (left._values == UnknownSentinel || right._values == UnknownSentinel) + return Unknown; + if (left._values is not EnumerableValues && right.Contains ((TValue) left._values)) return right.DeepCopy (); if (right._values is not EnumerableValues && left.Contains ((TValue) right._values)) return left.DeepCopy (); - var values = new EnumerableValues (left.DeepCopy ()); - values.UnionWith (right.DeepCopy ()); + var values = new EnumerableValues (left.DeepCopy ().GetKnownValues ()); + values.UnionWith (right.DeepCopy ().GetKnownValues ()); // Limit the number of values we track, to prevent hangs in case of patterns that - // create exponentially many possible values. This will result in analysis holes. + // create exponentially many possible values. if (values.Count > MaxValuesInSet) - return default; + return Unknown; return new ValueSet (values); } public bool IsEmpty () => _values == null; + public bool IsUnknown () => _values == UnknownSentinel; + public override string ToString () { + if (IsUnknown ()) + return "Unknown"; StringBuilder sb = new (); sb.Append ('{'); - sb.Append (string.Join (",", this.Select (v => v.ToString ()))); + sb.Append (string.Join (",", GetKnownValues ().Select (v => v.ToString ()))); sb.Append ('}'); return sb.ToString (); } @@ -223,6 +271,9 @@ public ValueSet DeepCopy () if (_values is null) return this; + if (_values == UnknownSentinel) + return this; + // Optimize for the most common case with only a single value if (_values is not EnumerableValues) { if (_values is IDeepCopyValue copyValue) @@ -231,7 +282,7 @@ public ValueSet DeepCopy () return this; } - return new ValueSet (this.Select (value => value is IDeepCopyValue copyValue ? copyValue.DeepCopy () : value)); + return new ValueSet (GetKnownValues ().Select (value => value is IDeepCopyValue copyValue ? copyValue.DeepCopy () : value)); } } } diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayValue.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayValue.cs index 9c699baea9125..417091915dfef 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayValue.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayValue.cs @@ -34,7 +34,7 @@ public static MultiValue SanitizeArrayElementValue (MultiValue input) // So we will simply treat array value as an element value as "too complex to analyze" and give up by storing Unknown instead bool needSanitization = false; - foreach (var v in input) { + foreach (var v in input.AsEnumerable ()) { if (v is ArrayValue) needSanitization = true; } @@ -42,7 +42,7 @@ public static MultiValue SanitizeArrayElementValue (MultiValue input) if (!needSanitization) return input; - return new(input.Select (v => v is ArrayValue ? UnknownValue.Instance : v)); + return new(input.AsEnumerable ().Select (v => v is ArrayValue ? UnknownValue.Instance : v)); } } } diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs index 1191e3cc1c304..6e2f42a2dba1e 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs @@ -68,7 +68,7 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl break; } - foreach (var value in argumentValues[0]) { + foreach (var value in argumentValues[0].AsEnumerable ()) { AddReturnValue (value switch { RuntimeTypeHandleForNullableSystemTypeValue nullableSystemType => new NullableSystemTypeValue (nullableSystemType.NullableType, nullableSystemType.UnderlyingTypeValue), @@ -94,7 +94,7 @@ RuntimeTypeHandleForGenericParameterValue genericParam break; } - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value != NullValue.Instance) AddReturnValue (value switch { NullableSystemTypeValue nullableSystemType @@ -123,7 +123,7 @@ GenericParameterValue genericParam } // Infrastructure piece to support "ldtoken method -> GetMethodFromHandle" - foreach (var value in argumentValues[0]) { + foreach (var value in argumentValues[0].AsEnumerable ()) { if (value is RuntimeMethodHandleValue methodHandle) AddReturnValue (new SystemReflectionMethodBaseValue (methodHandle.RepresentedMethod)); else @@ -138,7 +138,7 @@ GenericParameterValue genericParam break; } - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is SystemReflectionMethodBaseValue methodBaseValue) AddReturnValue (new RuntimeMethodHandleValue (methodBaseValue.RepresentedMethod)); else @@ -166,8 +166,8 @@ GenericParameterValue genericParam } var targetValue = _annotations.GetMethodThisParameterValue (calledMethod, DynamicallyAccessedMemberTypes.Interfaces); - foreach (var value in instanceValue) { - foreach (var interfaceName in argumentValues[0]) { + foreach (var value in instanceValue.AsEnumerable ()) { + foreach (var interfaceName in argumentValues[0].AsEnumerable ()) { if (interfaceName == NullValue.Instance) { // Throws on null string, so no return value. AddReturnValue (MultiValueLattice.Top); @@ -204,7 +204,7 @@ GenericParameterValue genericParam break; } - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers) { // Currently we don't need to track the difference between Type and String annotated values // that only matters when we use them, so Type.GetType is the difference really. @@ -232,7 +232,7 @@ GenericParameterValue genericParam break; } - foreach (var typeHandleValue in argumentValues[0]) { + foreach (var typeHandleValue in argumentValues[0].AsEnumerable ()) { if (typeHandleValue is RuntimeTypeHandleValue runtimeTypeHandleValue) { MarkStaticConstructor (runtimeTypeHandleValue.RepresentedType); } else { @@ -330,9 +330,9 @@ GenericParameterValue genericParam }; var targetValue = _annotations.GetMethodThisParameterValue (calledMethod, memberTypes); - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { - foreach (var stringParam in argumentValues[0]) { + foreach (var stringParam in argumentValues[0].AsEnumerable ()) { if (stringParam is KnownStringValue stringValue && !BindingFlagsAreUnsupported (bindingFlags)) { switch (intrinsicId) { case IntrinsicId.Type_GetEvent: @@ -396,7 +396,7 @@ GenericParameterValue genericParam var targetValue = _annotations.GetMethodThisParameterValue (calledMethod, requiredMemberTypes); // Go over all types we've seen - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { // Mark based on bitfield requirements _requireDynamicallyAccessedMembersAction.Invoke (value, targetValue); } @@ -432,9 +432,9 @@ GenericParameterValue genericParam bindingFlags = BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public; var targetValue = _annotations.GetMethodThisParameterValue (calledMethod, GetDynamicallyAccessedMemberTypesFromBindingFlagsForMethods (bindingFlags)); - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { - foreach (var stringParam in argumentValues[0]) { + foreach (var stringParam in argumentValues[0].AsEnumerable ()) { if (stringParam is KnownStringValue stringValue && !BindingFlagsAreUnsupported (bindingFlags)) { AddReturnValue (MultiValueLattice.Top); ; // Initialize return value (so that it's not autofilled if there are no matching methods) foreach (var methodValue in ProcessGetMethodByName (systemTypeValue.RepresentedType, stringValue.Contents, bindingFlags)) @@ -478,9 +478,9 @@ GenericParameterValue genericParam bindingFlags = BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public; var targetValue = _annotations.GetMethodThisParameterValue (calledMethod, GetDynamicallyAccessedMemberTypesFromBindingFlagsForNestedTypes (bindingFlags)); - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { - foreach (var stringParam in argumentValues[0]) { + foreach (var stringParam in argumentValues[0].AsEnumerable ()) { if (stringParam is KnownStringValue stringValue && !BindingFlagsAreUnsupported (bindingFlags)) { AddReturnValue (MultiValueLattice.Top); foreach (var nestedTypeValue in GetNestedTypesOnType (systemTypeValue.RepresentedType, stringValue.Contents, bindingFlags)) { @@ -546,9 +546,9 @@ GenericParameterValue genericParam var targetValue = _annotations.GetMethodParameterValue (new (calledMethod, (ParameterIndex) 1), requiredMemberTypes); - foreach (var value in argumentValues[0]) { + foreach (var value in argumentValues[0].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { - foreach (var stringParam in argumentValues[1]) { + foreach (var stringParam in argumentValues[1].AsEnumerable ()) { if (stringParam is KnownStringValue stringValue) { switch (intrinsicId) { case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeEvent: @@ -594,7 +594,7 @@ GenericParameterValue genericParam // case IntrinsicId.Expression_New: { var targetValue = _annotations.GetMethodParameterValue (new (calledMethod, (ParameterIndex) 0), DynamicallyAccessedMemberTypes.PublicParameterlessConstructor); - foreach (var value in argumentValues[0]) { + foreach (var value in argumentValues[0].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { MarkConstructorsOnType (systemTypeValue.RepresentedType, BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, parameterCount: null); } else { @@ -615,7 +615,7 @@ GenericParameterValue genericParam break; } - foreach (var value in argumentValues[1]) { + foreach (var value in argumentValues[1].AsEnumerable ()) { if (value is SystemReflectionMethodBaseValue methodBaseValue) { // We have one of the accessors for the property. The Expression.Property will in this case search // for the matching PropertyInfo and store that. So to be perfectly correct we need to mark the @@ -652,9 +652,9 @@ GenericParameterValue genericParam } var targetValue = _annotations.GetMethodParameterValue (new (calledMethod, (ParameterIndex) 1), memberTypes); - foreach (var value in argumentValues[1]) { + foreach (var value in argumentValues[1].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { - foreach (var stringParam in argumentValues[2]) { + foreach (var stringParam in argumentValues[2].AsEnumerable ()) { if (stringParam is KnownStringValue stringValue) { BindingFlags bindingFlags = argumentValues[0].AsSingleValue () is NullValue ? BindingFlags.Static : BindingFlags.Default; if (intrinsicId == IntrinsicId.Expression_Property) { @@ -689,9 +689,9 @@ GenericParameterValue genericParam // This is true even if we "don't know" - so it's only false if we're sure that there are no type arguments bool hasTypeArguments = (argumentValues[2].AsSingleValue () as ArrayValue)?.Size.AsConstInt () != 0; - foreach (var value in argumentValues[0]) { + foreach (var value in argumentValues[0].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { - foreach (var stringParam in argumentValues[1]) { + foreach (var stringParam in argumentValues[1].AsEnumerable ()) { if (stringParam is KnownStringValue stringValue) { foreach (var method in GetMethodsOnTypeHierarchy (systemTypeValue.RepresentedType, stringValue.Contents, bindingFlags)) { ValidateGenericMethodInstantiation (method.RepresentedMethod, argumentValues[2], calledMethod); @@ -769,7 +769,7 @@ GenericParameterValue genericParam break; } - foreach (var typeNameValue in argumentValues[0]) { + foreach (var typeNameValue in argumentValues[0].AsEnumerable ()) { if (typeNameValue is KnownStringValue knownStringValue) { if (!_requireDynamicallyAccessedMembersAction.TryResolveTypeNameAndMark (knownStringValue.Contents, false, out TypeProxy foundType)) { // Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back. @@ -804,7 +804,7 @@ GenericParameterValue genericParam break; } - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is SystemTypeValue typeValue) { // Special case Nullable // Nullables without a type argument are considered SystemTypeValues @@ -815,9 +815,9 @@ GenericParameterValue genericParam // There are several places even in the framework where typeof(Nullable<>).MakeGenericType would warn // without any good reason to do so. - foreach (var argumentValue in argumentValues[0]) { + foreach (var argumentValue in argumentValues[0].AsEnumerable ()) { if ((argumentValue as ArrayValue)?.TryGetValueByIndex (0, out var underlyingMultiValue) == true) { - foreach (var underlyingValue in underlyingMultiValue) { + foreach (var underlyingValue in underlyingMultiValue.AsEnumerable ()) { switch (underlyingValue) { // Don't warn on these types - it will throw instead case NullableValueWithDynamicallyAccessedMembers: @@ -879,7 +879,7 @@ GenericParameterValue genericParam break; } - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers) { DynamicallyAccessedMemberTypes propagatedMemberTypes = DynamicallyAccessedMemberTypes.None; if (valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.All) @@ -953,7 +953,7 @@ GenericParameterValue genericParam }; // Go over all types we've seen - foreach (var value in instanceValue) { + foreach (var value in instanceValue.AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue && !BindingFlagsAreUnsupported (bindingFlags)) { if (HasBindingFlag (bindingFlags, BindingFlags.Public) && !HasBindingFlag (bindingFlags, BindingFlags.NonPublic) && ctorParameterCount == 0) { @@ -986,7 +986,7 @@ GenericParameterValue genericParam break; } - foreach (var methodValue in instanceValue) { + foreach (var methodValue in instanceValue.AsEnumerable ()) { if (methodValue is SystemReflectionMethodBaseValue methodBaseValue) { ValidateGenericMethodInstantiation (methodBaseValue.RepresentedMethod, argumentValues[0], calledMethod); } else if (methodValue == NullValue.Instance) { @@ -1054,7 +1054,7 @@ GenericParameterValue genericParam } // Go over all types we've seen - foreach (var value in argumentValues[0]) { + foreach (var value in argumentValues[0].AsEnumerable ()) { if (value is SystemTypeValue systemTypeValue) { // Special case known type values as we can do better by applying exact binding flags and parameter count. MarkConstructorsOnType (systemTypeValue.RepresentedType, bindingFlags, ctorParameterCount); @@ -1172,7 +1172,7 @@ GenericParameterValue genericParam // Validate that the return value has the correct annotations as per the method return value annotations if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None) { - foreach (var uniqueValue in returnValue.Value) { + foreach (var uniqueValue in returnValue.Value.AsEnumerable ()) { if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes) { if (!methodReturnValueWithMemberTypes.DynamicallyAccessedMemberTypes.HasFlag (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes)) throw new InvalidOperationException ($"Internal ILLink error: in {GetContainingSymbolDisplayName ()} processing call to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); @@ -1228,7 +1228,7 @@ private bool AnalyzeGenericInstantiationTypeArray (in MultiValue arrayParam, in if (!hasRequirements) return true; - foreach (var typesValue in arrayParam) { + foreach (var typesValue in arrayParam.AsEnumerable ()) { if (typesValue is not ArrayValue array) { return false; } @@ -1318,13 +1318,13 @@ private void ProcessCreateInstanceByName (MethodProxy calledMethod, IReadOnlyLis bindingFlags |= BindingFlags.Public | BindingFlags.NonPublic; } - foreach (var assemblyNameValue in argumentValues[0]) { + foreach (var assemblyNameValue in argumentValues[0].AsEnumerable ()) { if (assemblyNameValue is KnownStringValue assemblyNameStringValue) { if (assemblyNameStringValue.Contents is string assemblyName && assemblyName.Length == 0) { // Throws exception for zero-length assembly name. continue; } - foreach (var typeNameValue in argumentValues[1]) { + foreach (var typeNameValue in argumentValues[1].AsEnumerable ()) { if (typeNameValue is NullValue) { // Throws exception for null type name. continue; diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/RequireDynamicallyAccessedMembersAction.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/RequireDynamicallyAccessedMembersAction.cs index aa24361bf5530..50166c803be30 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/RequireDynamicallyAccessedMembersAction.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/RequireDynamicallyAccessedMembersAction.cs @@ -23,7 +23,7 @@ public void Invoke (in MultiValue value, ValueWithDynamicallyAccessedMembers tar if (targetValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None) return; - foreach (var uniqueValue in value) { + foreach (var uniqueValue in value.AsEnumerable ()) { if (targetValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.PublicParameterlessConstructor && uniqueValue is GenericParameterValue genericParam && genericParam.GenericParameter.HasDefaultConstructorConstraint ()) { diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ValueExtensions.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ValueExtensions.cs index 0e7643a8d5d43..f9cd1b7ee1e60 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ValueExtensions.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ValueExtensions.cs @@ -50,10 +50,22 @@ internal static string ValueToString (this SingleValue value, params object[] ar internal static SingleValue? AsSingleValue (this in MultiValue node) { - if (node.Count () != 1) + var values = node.AsEnumerable (); + if (values.Count () != 1) return null; - return node.Single (); + return values.Single (); + } + + private static ValueSet.Enumerable Unknown = new ValueSet.Enumerable (UnknownValue.Instance); + + // ValueSet is not enumerable. This helper translates ValueSet.Unknown + // into a ValueSet whose sole element is UnknownValue.Instance. + internal static ValueSet.Enumerable AsEnumerable (this MultiValue multiValue) + { + return multiValue.IsUnknown () + ? Unknown + : multiValue.GetKnownValues (); } } } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ArrayValue.cs b/src/tools/illink/src/linker/Linker.Dataflow/ArrayValue.cs index 6b914c16f70d3..a428dfc4468ae 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ArrayValue.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ArrayValue.cs @@ -17,7 +17,7 @@ internal partial record ArrayValue public static MultiValue Create (MultiValue size, TypeReference elementType) { MultiValue result = MultiValueLattice.Top; - foreach (var sizeValue in size) { + foreach (var sizeValue in size.AsEnumerable ()) { result = MultiValueLattice.Meet (result, new MultiValue (new ArrayValue (sizeValue, elementType))); } @@ -87,7 +87,7 @@ public override SingleValue DeepCopy () // Since it's possible to store a reference to array as one of its own elements // simple deep copy could lead to endless recursion. // So instead we simply disallow arrays as element values completely - and treat that case as "too complex to analyze". - foreach (SingleValue v in kvp.Value.Value) { + foreach (SingleValue v in kvp.Value.Value.AsEnumerable ()) { System.Diagnostics.Debug.Assert (v is not ArrayValue); } #endif @@ -116,7 +116,7 @@ public override string ToString () result.Append (element.Key); result.Append (",("); bool firstValue = true; - foreach (var v in element.Value.Value) { + foreach (var v in element.Value.Value.AsEnumerable ()) { if (firstValue) { result.Append (','); firstValue = false; diff --git a/src/tools/illink/src/linker/Linker.Dataflow/InterproceduralState.cs b/src/tools/illink/src/linker/Linker.Dataflow/InterproceduralState.cs index 23667336eb8ce..8ab34d8019195 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/InterproceduralState.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/InterproceduralState.cs @@ -53,7 +53,8 @@ public void TrackMethod (MethodBody methodBody) public void TrackMethod (MethodIL methodIL) { // Work around the fact that ValueSet is readonly - var methodsList = new List (MethodBodies); + Debug.Assert (!MethodBodies.IsUnknown ()); + var methodsList = new List (MethodBodies.GetKnownValues ()); methodsList.Add (methodIL); // For state machine methods, also scan the state machine members. diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs index fe140b82d23d5..fac77cce8fd6c 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -185,7 +185,7 @@ static void ValidateNoReferenceToReference (LocalVariableStore locals, MethodDef foreach (var keyValuePair in locals) { MultiValue localValue = keyValuePair.Value.Value; VariableDefinition localVariable = keyValuePair.Key; - foreach (var val in localValue) { + foreach (var val in localValue.AsEnumerable ()) { if (val is LocalVariableReferenceValue localReference && localReference.ReferencedType.IsByReference) { string displayName = $"local variable V_{localReference.LocalDefinition.Index}"; throw new LinkerFatalErrorException (MessageContainer.CreateErrorMessage ( @@ -243,7 +243,8 @@ public virtual void InterproceduralScan (MethodIL startingMethodIL) // 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 methodIL in oldInterproceduralState.MethodBodies) + Debug.Assert (!oldInterproceduralState.MethodBodies.IsUnknown ()); + foreach (var methodIL in oldInterproceduralState.MethodBodies.GetKnownValues ()) Scan (methodIL, ref interproceduralState); } @@ -258,7 +259,7 @@ public virtual void InterproceduralScan (MethodIL startingMethodIL) // foreach (var method in calleeMethods) // Debug.Assert (interproceduralState.Any (kvp => kvp.Key.Method == method)); } else { - Debug.Assert (interproceduralState.MethodBodies.Count () == 1); + Debug.Assert (interproceduralState.MethodBodies.GetKnownValues().Count () == 1); } #endif } @@ -857,7 +858,7 @@ private void ScanIndirectStore ( /// Throws if is not a valid target for an indirect store. protected void StoreInReference (MultiValue target, MultiValue source, MethodDefinition method, Instruction operation, LocalVariableStore locals, int curBasicBlock, ref InterproceduralState ipState) { - foreach (var value in target) { + foreach (var value in target.AsEnumerable ()) { switch (value) { case LocalVariableReferenceValue localReference: StoreMethodLocalValue (locals, source, localReference.LocalDefinition, curBasicBlock); @@ -956,7 +957,7 @@ private void ScanStfld ( return; } - foreach (var value in GetFieldValue (field)) { + foreach (var value in GetFieldValue (field).AsEnumerable ()) { // GetFieldValue may return different node types, in which case they can't be stored to. // At least not yet. if (value is not FieldValue fieldValue) @@ -1012,7 +1013,7 @@ private ValueNodeList PopCallArguments ( internal MultiValue DereferenceValue (MultiValue maybeReferenceValue, LocalVariableStore locals, ref InterproceduralState interproceduralState) { MultiValue dereferencedValue = MultiValueLattice.Top; - foreach (var value in maybeReferenceValue) { + foreach (var value in maybeReferenceValue.AsEnumerable ()) { switch (value) { case FieldReferenceValue fieldReferenceValue: dereferencedValue = MultiValue.Meet ( @@ -1123,7 +1124,7 @@ private void HandleCall ( AssignRefAndOutParameters (callingMethodBody, calledMethod, methodArguments, operation, locals, curBasicBlock, ref interproceduralState); foreach (var param in methodArguments) { - foreach (var v in param) { + foreach (var v in param.AsEnumerable ()) { if (v is ArrayValue arr) { MarkArrayValuesAsUnknown (arr, curBasicBlock); } @@ -1163,7 +1164,7 @@ private void ScanStelem ( StackSlot indexToStoreAt = PopUnknown (currentStack, 1, methodBody, operation.Offset); StackSlot arrayToStoreIn = PopUnknown (currentStack, 1, methodBody, operation.Offset); int? indexToStoreAtInt = indexToStoreAt.Value.AsConstInt (); - foreach (var array in arrayToStoreIn.Value) { + foreach (var array in arrayToStoreIn.Value.AsEnumerable ()) { if (array is ArrayValue arrValue) { if (indexToStoreAtInt == null) { MarkArrayValuesAsUnknown (arrValue, curBasicBlock); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 28780d1123915..7eca458bc81d7 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -283,7 +283,7 @@ public static bool HandleCall ( // GetType() // case IntrinsicId.Object_GetType: { - foreach (var valueNode in instanceValue) { + foreach (var valueNode in instanceValue.AsEnumerable ()) { // Note that valueNode can be statically typed in IL as some generic argument type. // For example: // void Method(T instance) { instance.GetType().... } @@ -359,7 +359,7 @@ public static bool HandleCall ( // Validate that the return value has the correct annotations as per the method return value annotations if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0) { - foreach (var uniqueValue in methodReturnValue) { + foreach (var uniqueValue in methodReturnValue.AsEnumerable ()) { if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes) { if (!methodReturnValueWithMemberTypes.DynamicallyAccessedMemberTypes.HasFlag (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes)) throw new InvalidOperationException ($"Internal trimming error: processing of call from {callingMethodDefinition.GetDisplayName ()} to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs index 829fefdb2f545..ccc91c04fba4c 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisAssignmentPattern.cs @@ -37,8 +37,8 @@ public void MarkAndProduceDiagnostics (ReflectionMarker reflectionMarker, LinkCo bool diagnosticsEnabled = !context.Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (Origin.Provider, out _); var diagnosticContext = new DiagnosticContext (Origin, diagnosticsEnabled, context); - foreach (var sourceValue in Source) { - foreach (var targetValue in Target) { + foreach (var sourceValue in Source.AsEnumerable ()) { + foreach (var targetValue in Target.AsEnumerable ()) { if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers) throw new NotImplementedException (); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExponentialDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExponentialDataFlow.cs index dffe035d63648..d2d5b05da7238 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExponentialDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExponentialDataFlow.cs @@ -3,7 +3,9 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; namespace Mono.Linker.Tests.Cases.DataFlow { @@ -16,6 +18,8 @@ public static void Main () ExponentialArrayStates.Test (); ExponentialArrayStatesDataFlow.Test (); ArrayStatesDataFlow.Test (); + ExponentialArrayInStateMachine.Test (); + ExponentialStateFieldInStateMachine.Test (); } class ExponentialArrayStates @@ -91,8 +95,9 @@ class GenericTypeWithRequires< [ExpectedWarning ("IL3050", ProducedBy = Tool.Analyzer | Tool.NativeAot)] // The way we track arrays causes the analyzer to track exponentially many // ArrayValues in the ValueSet for the pattern in this method, hitting the limit. - // When this happens, we replace the ValueSet wit a TopValue, which doesn't - // produce a warning in this case. + // When this happens, we replace the ValueSet with an unknown value, producing + // this warning. + [ExpectedWarning ("IL2055", ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL2090", "'T'", ProducedBy = Tool.Trimmer | Tool.NativeAot)] [ExpectedWarning ("IL2090", "'T'", ProducedBy = Tool.Trimmer | Tool.NativeAot)] [ExpectedWarning ("IL2090", "'T'", ProducedBy = Tool.Trimmer | Tool.NativeAot)] @@ -163,5 +168,78 @@ public static void Test () static bool Condition => Random.Shared.Next (2) == 0; } + + class ExponentialArrayInStateMachine + { + // Force state machine + static async Task RecursiveReassignment () + { + typeof (TestType).RequiresAll (); // Force data flow analysis + + object[] args = null; + args = new[] { args }; + } + + public static void Test() + { + RecursiveReassignment ().Wait (); + } + } + + class ExponentialStateFieldInStateMachine + { + [ExpectedWarning ("IL2072", nameof (GetWithPublicFields), nameof (DataFlowTypeExtensions.RequiresAll), CompilerGeneratedCode = true)] + public static async void Test () + { + Type t = GetWithPublicFields (); + + // 100 + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + + // 200 + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + + // 300 + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); await MethodAsync (); + + t.RequiresAll (); + } + } + + class TestType { } + + static async Task MethodAsync () + { + return await Task.FromResult (0); + } + + static Type GetWithPublicFields () => null; } }