Skip to content

Commit 5207060

Browse files
sbomermichaelgsharp
authored andcommitted
Track unknown value as result of unimplemented operations (dotnet#101752)
Up until now, the analyzer has been set up to return `TopValue` for operations which are not handled by their own `Visit` overrides, to avoid producing warnings that are not produced from ILLink or ILC. This changes the default handling to return `UnknownValue.Instance`, so that such operations produce warnings if the return value flows into a location with dataflow requirements. Fixes dotnet#101733, where the return value of a string interpolation operation (which doesn't have special handling in a `Visit` override) was not producing warnings when passed to `Type.GetType`.
1 parent ace22a7 commit 5207060

File tree

4 files changed

+25
-9
lines changed

4 files changed

+25
-9
lines changed

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,22 @@ public override void ApplyCondition (FeatureChecksValue featureChecksValue, ref
7575
// - 'this' parameter (for annotated methods)
7676
// - field reference
7777

78-
public override MultiValue Visit (IOperation? operation, StateValue argument)
78+
public override MultiValue DefaultVisit (IOperation operation, StateValue argument)
7979
{
80-
var returnValue = base.Visit (operation, argument);
80+
var returnValue = base.DefaultVisit (operation, argument);
8181

8282
// If the return value is empty (TopValue basically) and the Operation tree
8383
// reports it as having a constant value, use that as it will automatically cover
8484
// cases we don't need/want to handle.
85-
if (operation != null && returnValue.IsEmpty () && TryGetConstantValue (operation, out var constValue))
85+
if (!returnValue.IsEmpty ())
86+
return returnValue;
87+
88+
if (TryGetConstantValue (operation, out var constValue))
8689
return constValue;
8790

91+
if (operation.Type is not null)
92+
return UnknownValue.Instance;
93+
8894
return returnValue;
8995
}
9096

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ public Task InlineArrayDataflow ()
186186
}
187187

188188
[Fact]
189-
public Task InterpolatedStringHandlerDataFlow ()
189+
public Task InterpolatedStringDataFlow ()
190190
{
191191
return RunTest ();
192192
}

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,9 @@ public static void Main()
13091309

13101310
return VerifyDynamicallyAccessedMembersAnalyzer (Source, consoleApplication: false,
13111311
// (12,34): error CS0103: The name 'type' does not exist in the current context
1312-
DiagnosticResult.CompilerError ("CS0103").WithSpan (12, 34, 12, 38).WithArguments ("type"));
1312+
DiagnosticResult.CompilerError ("CS0103").WithSpan (12, 34, 12, 38).WithArguments ("type"),
1313+
// (12,34): warning IL2063: Value returned from method 'C.GetTypeWithAll()' can not be statically determined and may not meet 'DynamicallyAccessedMembersAttribute' requirements.
1314+
VerifyCS.Diagnostic (DiagnosticId.MethodReturnValueCannotBeStaticallyDetermined).WithSpan(12, 34, 12, 38).WithArguments("C.GetTypeWithAll()"));
13131315
}
13141316

13151317
[Fact]
@@ -1333,7 +1335,9 @@ public static void Main()
13331335

13341336
return VerifyDynamicallyAccessedMembersAnalyzer (Source, consoleApplication: false,
13351337
// (8,22): error CS0103: The name 'type' does not exist in the current context
1336-
DiagnosticResult.CompilerError ("CS0103").WithSpan (8, 22, 8, 26).WithArguments ("type"));
1338+
DiagnosticResult.CompilerError ("CS0103").WithSpan (8, 22, 8, 26).WithArguments ("type"),
1339+
// (8,3): warning IL2064: Value assigned to C.fieldRequiresAll can not be statically determined and may not meet 'DynamicallyAccessedMembersAttribute' requirements.
1340+
VerifyCS.Diagnostic(DiagnosticId.FieldValueCannotBeStaticallyDetermined).WithSpan(8, 3, 8, 26).WithArguments("C.fieldRequiresAll"));
13371341
}
13381342

13391343
[Fact]

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InterpolatedStringHandlerDataFlow.cs renamed to src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InterpolatedStringDataFlow.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@ namespace Mono.Linker.Tests.Cases.DataFlow
1515
[ExpectedNoWarnings]
1616
[SkipKeptItemsValidation]
1717
[Define ("DEBUG")]
18-
public class InterpolatedStringHandlerDataFlow
18+
public class InterpolatedStringDataFlow
1919
{
2020
public static void Main ()
2121
{
22-
Test ();
22+
TestInterpolatedStringHandler ();
23+
TestUnknownInterpolatedString ();
2324
}
2425

25-
static void Test(bool b = true) {
26+
static void TestInterpolatedStringHandler (bool b = true) {
2627
// Creates a control-flow graph for the analyzer that has an
2728
// IFlowCaptureReferenceOperation that represents a capture
2829
// because it is used as an out param (so has IsInitialization = true).
@@ -31,5 +32,10 @@ static void Test(bool b = true) {
3132
// where IsInitialization = true.
3233
Debug.Assert (b, $"Debug interpolated string handler {b}");
3334
}
35+
36+
[ExpectedWarning ("IL2057")]
37+
static void TestUnknownInterpolatedString (string input = "test") {
38+
Type.GetType ($"{input}");
39+
}
3440
}
3541
}

0 commit comments

Comments
 (0)