diff options
author | Tlakaelel Axayakatl Ceja <tlakaelel.ceja@microsoft.com> | 2022-04-19 04:35:09 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-19 04:35:09 +0300 |
commit | 5c69665bbb929b8decacb266dc2edeb3e32b9858 (patch) | |
tree | 56b1b09404e33b6cbd54ac02951fa06966f0fba7 | |
parent | 1ba806627cb47cc106c6207df185cf475db94f4b (diff) |
Handle multiple values for an array node (#2744)
- The analyzer now has a logic that handles array blocks independently when an if statement is used, see the following example code:
Type[] arr = new Type[] { null };
if (i == 1)
arr[0] = GetMethods ();
else
arr[i] = GetFields ();
arr[0].RequiresAll ();
Gets translated to a structure similar to:
------[B1]------
-----/----\-----
--[B2]----[B3]--
-----\----/-----
------[B4]------
* The first block (B1) contains the array initialization
* Given that there could be different values depending on the result of the if statement, block 2 (B2) and block 3 (B3) handle the different results. Notice that they both start with a copy of the information on B1 which is their predecessor. B2 and B3 do not interact between them and generate their output based on merging the value of B1 and their own generated value.
* B2 executes GetMethods(); which has the return value PublicMethods, then we merge the values in index 0 which are: empty array from B1 and the return value in B2 (PublicMethods)
* B3 executes GetFields(); which has the return value PublicFields, then tries to access the array but the index is unknown so we return UnknownValue
* Block 4 (B4) executes RequiresAll(); in order to verify if arr[0] fulfills the requirements, we read the different values in the array index 0, meaning the result from B2 and B3. At that moment we attempt to merge the result from the different arrays, we verify that one of the results is unknown and therefore we cannot know if arr[0] fulfills the requirements. Generating a single warning stating that the value is unknown.
- Add tests
-rw-r--r-- | src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs | 15 | ||||
-rw-r--r-- | test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs | 33 |
2 files changed, 41 insertions, 7 deletions
diff --git a/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 8c18b0931..4e3cda618 100644 --- a/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -195,16 +195,17 @@ namespace ILLink.RoslynAnalyzer.TrimAnalysis public override MultiValue HandleArrayElementRead (MultiValue arrayValue, MultiValue indexValue, IOperation operation) { - if (arrayValue.AsSingleValue () is not ArrayValue arr) - return UnknownValue.Instance; - if (indexValue.AsConstInt () is not int index) return UnknownValue.Instance; - if (arr.TryGetValueByIndex (index, out var elementValue)) - return elementValue; - - return UnknownValue.Instance; + MultiValue result = TopValue; + foreach (var value in arrayValue) { + if (value is ArrayValue arr && arr.TryGetValueByIndex (index, out var elementValue)) + result = _multiValueLattice.Meet (result, elementValue); + else + return UnknownValue.Instance; + } + return result.Equals (TopValue) ? UnknownValue.Instance : result; } public override void HandleArrayElementWrite (MultiValue arrayValue, MultiValue indexValue, MultiValue valueToWrite, IOperation operation) diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs index 0bab171ef..161889350 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs @@ -22,11 +22,13 @@ namespace Mono.Linker.Tests.Cases.DataFlow TestArraySetElementOneElementStaticType (); TestArraySetElementOneElementParameter (typeof (TestType)); TestArraySetElementMultipleElementsStaticType (); + TestMergedArrayElement (1); TestArraySetElementMultipleElementsMix<TestType> (typeof (TestType)); TestArraySetElementAndInitializerMultipleElementsMix<TestType> (typeof (TestType)); TestGetElementAtUnknownIndex (); + TestMergedArrayElementWithUnknownIndex (0); // Array reset - certain operations on array are not tracked fully (or impossible due to unknown inputs) // and sometimes the only valid thing to do is to reset the array to all unknowns as it's impossible @@ -109,6 +111,24 @@ namespace Mono.Linker.Tests.Cases.DataFlow arr[3].RequiresPublicMethods (); // Should warn - unknown value at this index } + [ExpectedWarning ("IL2072", nameof (ArrayDataFlow.GetMethods))] + [ExpectedWarning ("IL2072", nameof (ArrayDataFlow.GetFields))] + static void TestMergedArrayElement (int i) + { + Type[] arr = new Type[] { null }; + if (i == 1) + arr[0] = GetMethods (); + else + arr[0] = GetFields (); + arr[0].RequiresAll (); // Should warn - Methods/Fields does not have match annotations with All. + } + + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + static Type GetMethods () => null; + + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + static Type GetFields () => null; + [ExpectedWarning ("IL2087", nameof (DataFlowTypeExtensions.RequiresPublicFields))] [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] static void TestArraySetElementMultipleElementsMix<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)] TProperties> ( @@ -148,6 +168,19 @@ namespace Mono.Linker.Tests.Cases.DataFlow arr[i].RequiresPublicFields (); } + // Trimmer code doesnt handle locals from different branches separetely, therefore merges incorrectly GetMethods with Unknown producing both warnings + [ExpectedWarning ("IL2072", nameof (ArrayDataFlow.GetMethods), ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresAll))] + static void TestMergedArrayElementWithUnknownIndex (int i) + { + Type[] arr = new Type[] { null }; + if (i == 1) + arr[0] = GetMethods (); + else + arr[i] = GetFields (); + arr[0].RequiresAll (); // Should warn - there is an unknown value on fields therefore the merged value should be unknown + } + [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicFields))] static void TestArrayResetStoreUnknownIndex (int i = 0) { |