Skip to content

Commit f067016

Browse files
sbomersbomer
authored
[ILLink analyzer] Improve handling of invalid operations (#94888)
Adds a helper that's used in the default case whenever we switch on the IOperation type, that: - skips invalid operations - skips unexpected operations with nested invalid operations - skips INoneOperation - throws otherwise, in Debug mode (surfaced as AD0001 warning)
1 parent f73c719 commit f067016

File tree

6 files changed

+177
-41
lines changed

6 files changed

+177
-41
lines changed

‎src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,12 @@ public CapturedReferenceValue (IOperation operation)
2424
case OperationKind.InlineArrayAccess:
2525
case OperationKind.ImplicitIndexerReference:
2626
break;
27-
case OperationKind.None:
2827
case OperationKind.InstanceReference:
2928
case OperationKind.Invocation:
30-
case OperationKind.Invalid:
3129
// These will just be ignored when referenced later.
3230
break;
3331
default:
34-
// Assert on anything else as it means we need to implement support for it
35-
// but do not throw here as it means new Roslyn version could cause the analyzer to crash
36-
// which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do
37-
// so effectively ignoring constructs it doesn't understand is OK.
38-
Debug.Fail ($"{operation.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}");
32+
UnexpectedOperationHandler.Handle (operation);
3933
break;
4034
}
4135
Reference = operation;

‎src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -335,9 +335,6 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe
335335
// Seems like this can't happen with a flow capture operation.
336336
Debug.Assert (operation.Target is not IFlowCaptureReferenceOperation);
337337
break;
338-
case IInvalidOperation:
339-
// This can happen for a field assignment in an attribute instance.
340-
// TODO: validate against the field attributes.
341338
case IInstanceReferenceOperation:
342339
// Assignment to 'this' is not tracked currently.
343340
// Not relevant for trimming dataflow.
@@ -358,16 +355,7 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe
358355
// can show up in a flow capture reference (for example, where the right-hand side
359356
// is a null-coalescing operator).
360357
default:
361-
// NoneOperation represents operations which are unimplemented by Roslyn
362-
// (don't have specific I*Operation types), such as pointer dereferences.
363-
if (targetOperation.Kind is OperationKind.None)
364-
break;
365-
366-
// Assert on anything else as it means we need to implement support for it
367-
// but do not throw here as it means new Roslyn version could cause the analyzer to crash
368-
// which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do
369-
// so effectively ignoring constructs it doesn't understand is OK.
370-
Debug.Fail ($"{targetOperation.GetType ()}: {targetOperation.Syntax.GetLocation ().GetLineSpan ()}");
358+
UnexpectedOperationHandler.Handle (targetOperation);
371359
break;
372360
}
373361
return Visit (operation.Value, state);
@@ -584,9 +572,7 @@ public override TValue VisitDelegateCreation (IDelegateCreationOperation operati
584572
// No method symbol.
585573
break;
586574
default:
587-
// Unimplemented case that might need special handling.
588-
// Fail in debug mode only.
589-
Debug.Fail ($"{operation.Target.GetType ()}: {operation.Target.Syntax.GetLocation ().GetLineSpan ()}");
575+
UnexpectedOperationHandler.Handle (operation.Target);
590576
break;
591577
}
592578

@@ -781,7 +767,7 @@ TValue HandleMethodCallHelper (
781767
argumentOperation = callOperation.Arguments[argumentIndex];
782768
break;
783769
default:
784-
Debug.Fail ($"Unexpected operation {operation} for parameter {parameter.GetDisplayName ()}");
770+
UnexpectedOperationHandler.Handle (operation);
785771
continue;
786772
};
787773

‎src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/UnexpectedOperationHandler.cs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
using System.Diagnostics;
6+
using Microsoft.CodeAnalysis.Operations;
7+
8+
namespace Microsoft.CodeAnalysis
9+
{
10+
internal static class UnexpectedOperationHandler
11+
{
12+
// No-op in release builds, but fails in debug builds when
13+
// encountering an unexpected operation. InvalidOperation is skipped because
14+
// it is expected that any part of the control-flow graph may contain an
15+
// InvalidOperation for code that doesn't compile (for example, in an intermediate
16+
// state while editing).
17+
[Conditional ("DEBUG")]
18+
public static void Handle (IOperation operation)
19+
{
20+
// NoneOperation represents operations which are unimplemented by Roslyn
21+
// (don't have specific I*Operation types), such as pointer dereferences.
22+
if (operation.Kind is OperationKind.None)
23+
return;
24+
25+
if (operation.Kind is OperationKind.Invalid)
26+
return;
27+
28+
// It's also possible to hit a case where the operation is an unexpected operation kind,
29+
// but the code is in an invalid state where the unexpected operation is not IInvalidOperation, yet one
30+
// of its child operations is. For example:
31+
//
32+
// a + = 3;
33+
//
34+
// This is represented as an assignment where the target is an IBinaryOperation (a +) whose right-hand side
35+
// is an IInvalidOperation. The assignment logic doesn't support assigning to a binary operation,
36+
// but this should still not fail.
37+
foreach (var descendant in operation.Descendants()) {
38+
if (descendant.Kind is OperationKind.Invalid)
39+
return;
40+
}
41+
42+
// Throw on anything else as it means we need to implement support for it
43+
// but do not throw in Release builds as it means new Roslyn version could cause the analyzer to crash
44+
// which is not fixable by the user. The analyzer is not going to be 100% correct no
45+
// matter what we do so effectively ignoring constructs it doesn't understand is OK.
46+
// This is surfaced as warning AD0001 in Debug builds.
47+
throw new NotImplementedException ($"Unexpected operation type {operation.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}");
48+
}
49+
}
50+
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,6 +1384,25 @@ public static void Main()
13841384
DiagnosticResult.CompilerError ("CS0103").WithSpan (8, 3, 8, 7).WithArguments ("type"));
13851385
}
13861386

1387+
[Fact]
1388+
public Task AssignmentTargetHasNestedInvalidOperation ()
1389+
{
1390+
// The assignment target is an IBinaryOperation whose right-hand side is an IInvalidOperation.
1391+
var Source = $$"""
1392+
int a, b = 0;
1393+
a + = 3;
1394+
""";
1395+
1396+
return VerifyDynamicallyAccessedMembersAnalyzer (Source, consoleApplication: true,
1397+
// (2,6): error CS1525: Invalid expression term '='
1398+
DiagnosticResult.CompilerError("CS1525").WithSpan(2, 6, 2, 7).WithArguments("="),
1399+
// (2,2): error CS0165: Use of unassigned local variable 'a'
1400+
DiagnosticResult.CompilerError("CS0165").WithSpan(2, 2, 2, 3).WithArguments("a"),
1401+
// (1,9): warning CS0219: The variable 'b' is assigned but its value is never used
1402+
DiagnosticResult.CompilerWarning("CS0219").WithSpan(1, 9, 1, 10).WithArguments("b")
1403+
);
1404+
}
1405+
13871406
[Fact]
13881407
public Task CRefGenericParameterAnalysis ()
13891408
{

‎src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributeFieldDataflow.cs

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,56 @@ namespace Mono.Linker.Tests.Cases.DataFlow
1212
[ExpectedNoWarnings]
1313
class AttributeFieldDataflow
1414
{
15+
public static void Main ()
16+
{
17+
TestKeepsPublicConstructors ();
18+
TestKeepsPublicMethods ();
19+
TestKeepsPublicMethodsString ();
20+
TestKeepsPublicFields ();
21+
TestTypeArray ();
22+
}
23+
24+
[Kept]
1525
[KeptAttributeAttribute (typeof (KeepsPublicConstructorsAttribute))]
26+
[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
27+
public static void TestKeepsPublicConstructors ()
28+
{
29+
typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicConstructors)).GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
30+
}
31+
32+
[Kept]
1633
[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
34+
[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--")]
35+
[KeepsPublicMethods (Type = typeof (ClassWithKeptPublicMethods))]
36+
public static void TestKeepsPublicMethods ()
37+
{
38+
typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicMethods)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
39+
}
40+
41+
[Kept]
42+
[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
43+
// Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273
44+
[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
45+
[KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributeFieldDataflow+ClassWithKeptPublicMethods")]
46+
public static void TestKeepsPublicMethodsString ()
47+
{
48+
typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicMethodsString)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
49+
}
50+
51+
[Kept]
1752
[KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))]
18-
[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
19-
[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
20-
[KeepsPublicMethods (Type = "Mono.Linker.Tests.Cases.DataFlow.AttributeFieldDataflow+ClassWithKeptPublicMethods")]
2153
[KeepsPublicFields (Type = null, TypeName = null)]
54+
public static void TestKeepsPublicFields ()
55+
{
56+
typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicFields)).GetCustomAttribute (typeof (KeepsPublicFieldsAttribute));
57+
}
58+
59+
[Kept]
60+
[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
2261
[TypeArray (Types = new Type[] { typeof (AttributeFieldDataflow) })]
23-
// Trimmer only for now - https://github.com/dotnet/linker/issues/2273
24-
[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
25-
public static void Main ()
62+
public static void TestTypeArray ()
2663
{
27-
typeof (AttributeFieldDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
28-
typeof (AttributeFieldDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
64+
typeof (AttributeFieldDataflow).GetMethod (nameof (TestTypeArray)).GetCustomAttribute (typeof (TypeArrayAttribute));
2965
}
3066

3167
[Kept]
@@ -55,7 +91,12 @@ public KeepsPublicMethodsAttribute ()
5591
[Kept]
5692
[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
5793
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
58-
public string Type;
94+
public string TypeName;
95+
96+
[Kept]
97+
[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
98+
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
99+
public Type Type;
59100
}
60101

61102
// Use to test null values

‎src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,56 @@ public static void Main ()
2727

2828
class AttributesOnMethod
2929
{
30+
[Kept]
31+
public static void Test () {
32+
TestKeepsPublicConstructors ();
33+
TestKeepsPublicMethods ();
34+
TestKeepsPublicMethodsByName ();
35+
TestKeepsPublicFields ();
36+
TestTypeArray ();
37+
}
38+
3039
[Kept]
3140
[KeptAttributeAttribute (typeof (KeepsPublicConstructorsAttribute))]
41+
[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
42+
public static void TestKeepsPublicConstructors ()
43+
{
44+
typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicConstructors)).GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
45+
}
46+
47+
[Kept]
48+
[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
49+
[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--")]
50+
[KeepsPublicMethods (Type = typeof (ClassWithKeptPublicMethods))]
51+
public static void TestKeepsPublicMethods ()
52+
{
53+
typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicMethods)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
54+
}
55+
56+
[Kept]
3257
[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
58+
// Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273
59+
[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethodsKeptByName--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
60+
[KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+AttributesOnMethod+ClassWithKeptPublicMethodsKeptByName")]
61+
public static void TestKeepsPublicMethodsByName ()
62+
{
63+
typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicMethodsByName)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
64+
}
65+
66+
[Kept]
3367
[KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))]
34-
[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
35-
[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
36-
[KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+AttributesOnMethod+ClassWithKeptPublicMethods")]
3768
[KeepsPublicFields (Type = null, TypeName = null)]
69+
public static void TestKeepsPublicFields ()
70+
{
71+
typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicFields)).GetCustomAttribute (typeof (KeepsPublicFieldsAttribute));
72+
}
73+
74+
[Kept]
75+
[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
3876
[TypeArray (Types = new Type[] { typeof (AttributePropertyDataflow) })]
39-
// Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273
40-
[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
41-
public static void Test () {
42-
typeof (AttributesOnMethod).GetMethod ("Test").GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
43-
typeof (AttributePropertyDataflow).GetMethod ("Test").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
77+
public static void TestTypeArray ()
78+
{
79+
typeof (AttributesOnMethod).GetMethod (nameof (TestTypeArray)).GetCustomAttribute (typeof (TypeArrayAttribute));
4480
}
4581

4682
[Kept]
@@ -63,6 +99,16 @@ class ClassWithKeptPublicMethods
6399
public static void KeptMethod () { }
64100
static void Method () { }
65101
}
102+
103+
[Kept]
104+
class ClassWithKeptPublicMethodsKeptByName
105+
{
106+
[Kept]
107+
[KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
108+
[RequiresUnreferencedCode ("--ClassWithKeptPublicMethodsKeptByName--")]
109+
public static void KeptMethod () { }
110+
static void Method () { }
111+
}
66112
}
67113

68114
class AttributesOnField

0 commit comments

Comments
 (0)