Skip to content

Commit

Permalink
Track unknown value as result of unimplemented operations (#101752)
Browse files Browse the repository at this point in the history
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 #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`.
  • Loading branch information
sbomer authored May 1, 2024
1 parent 9e678f6 commit 5d05361
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,22 @@ public override void ApplyCondition (FeatureChecksValue featureChecksValue, ref
// - 'this' parameter (for annotated methods)
// - field reference

public override MultiValue Visit (IOperation? operation, StateValue argument)
public override MultiValue DefaultVisit (IOperation operation, StateValue argument)
{
var returnValue = base.Visit (operation, argument);
var returnValue = base.DefaultVisit (operation, argument);

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

if (TryGetConstantValue (operation, out var constValue))
return constValue;

if (operation.Type is not null)
return UnknownValue.Instance;

return returnValue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public Task InlineArrayDataflow ()
}

[Fact]
public Task InterpolatedStringHandlerDataFlow ()
public Task InterpolatedStringDataFlow ()
{
return RunTest ();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1309,7 +1309,9 @@ public static void Main()

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

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

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

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ namespace Mono.Linker.Tests.Cases.DataFlow
[ExpectedNoWarnings]
[SkipKeptItemsValidation]
[Define ("DEBUG")]
public class InterpolatedStringHandlerDataFlow
public class InterpolatedStringDataFlow
{
public static void Main ()
{
Test ();
TestInterpolatedStringHandler ();
TestUnknownInterpolatedString ();
}

static void Test(bool b = true) {
static void TestInterpolatedStringHandler (bool b = true) {
// Creates a control-flow graph for the analyzer that has an
// IFlowCaptureReferenceOperation that represents a capture
// because it is used as an out param (so has IsInitialization = true).
Expand All @@ -31,5 +32,10 @@ static void Test(bool b = true) {
// where IsInitialization = true.
Debug.Assert (b, $"Debug interpolated string handler {b}");
}

[ExpectedWarning ("IL2057")]
static void TestUnknownInterpolatedString (string input = "test") {
Type.GetType ($"{input}");
}
}
}

0 comments on commit 5d05361

Please # to comment.