Skip to content

Commit af93dbb

Browse files
authored
Fix behavior of intrinsics with empty inputs (dotnet/linker#2652)
* Fix behavior of intrinsics with null or empty inputs Add tests for intrinsics receiving null or empty values, and tweak a few intrinsics to avoid warnings for these cases. This fixes some unnecessary warnings, and also fixes a crash in the linker. Some of the new shared intrinsics that don't produce type values need to have some tracked value. Presumably this should really be Unknown, but currently they fall back on the shared logic to track a value with annotations (which for these intrinsics will be None). A few unnecessary warnings are left as-is to avoid changing the linker behavior. Commit migrated from dotnet/linker@6aa9837
1 parent 0191b0a commit af93dbb

34 files changed

+856
-22
lines changed

src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl
7373
break;
7474

7575
case IntrinsicId.Type_get_TypeHandle:
76+
if (instanceValue.IsEmpty ()) {
77+
returnValue = MultiValueLattice.Top;
78+
break;
79+
}
80+
7681
foreach (var value in instanceValue) {
7782
if (value is SystemTypeValue typeValue)
7883
AddReturnValue (new RuntimeTypeHandleValue (typeValue.RepresentedType));
@@ -91,23 +96,37 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl
9196
// GetInterface (String, bool)
9297
//
9398
case IntrinsicId.Type_GetInterface: {
99+
if (instanceValue.IsEmpty () || argumentValues[0].IsEmpty ()) {
100+
returnValue = MultiValueLattice.Top;
101+
break;
102+
}
103+
94104
var targetValue = GetMethodThisParameterValue (calledMethod, DynamicallyAccessedMemberTypesOverlay.Interfaces);
95105
foreach (var value in instanceValue) {
96-
// For now no support for marking a single interface by name. We would have to correctly support
97-
// mangled names for generics to do that correctly. Simply mark all interfaces on the type for now.
98-
99-
// Require Interfaces annotation
100-
_requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
101-
102-
// Interfaces is transitive, so the return values will always have at least Interfaces annotation
103-
DynamicallyAccessedMemberTypes returnMemberTypes = DynamicallyAccessedMemberTypesOverlay.Interfaces;
104-
105-
// Propagate All annotation across the call - All is a superset of Interfaces
106-
if (value is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers
107-
&& valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.All)
108-
returnMemberTypes = DynamicallyAccessedMemberTypes.All;
109-
110-
AddReturnValue (GetMethodReturnValue (calledMethod, returnMemberTypes));
106+
foreach (var interfaceName in argumentValues[0]) {
107+
if (interfaceName == NullValue.Instance) {
108+
// Throws on null string, so no return value.
109+
returnValue ??= MultiValueLattice.Top;
110+
} else if (interfaceName is KnownStringValue stringValue && stringValue.Contents.Length == 0) {
111+
AddReturnValue (NullValue.Instance);
112+
} else {
113+
// For now no support for marking a single interface by name. We would have to correctly support
114+
// mangled names for generics to do that correctly. Simply mark all interfaces on the type for now.
115+
116+
// Require Interfaces annotation
117+
_requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
118+
119+
// Interfaces is transitive, so the return values will always have at least Interfaces annotation
120+
DynamicallyAccessedMemberTypes returnMemberTypes = DynamicallyAccessedMemberTypesOverlay.Interfaces;
121+
122+
// Propagate All annotation across the call - All is a superset of Interfaces
123+
if (value is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers
124+
&& valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.All)
125+
returnMemberTypes = DynamicallyAccessedMemberTypes.All;
126+
127+
AddReturnValue (GetMethodReturnValue (calledMethod, returnMemberTypes));
128+
}
129+
}
111130
}
112131
}
113132
break;
@@ -116,6 +135,11 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl
116135
// AssemblyQualifiedName
117136
//
118137
case IntrinsicId.Type_get_AssemblyQualifiedName: {
138+
if (instanceValue.IsEmpty ()) {
139+
returnValue = MultiValueLattice.Top;
140+
break;
141+
}
142+
119143
foreach (var value in instanceValue) {
120144
if (value is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers) {
121145
// Currently we don't need to track the difference between Type and String annotated values
@@ -467,9 +491,9 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl
467491
return true;
468492
}
469493

470-
// For some intrinsics we just use the return value from the annotations.
471-
if (returnValue == null)
472-
returnValue = calledMethod.ReturnsVoid () ? MultiValueLattice.Top : GetMethodReturnValue (calledMethod, returnValueDynamicallyAccessedMemberTypes);
494+
// For now, if the intrinsic doesn't set a return value, fall back on the annotations.
495+
// Note that this will be DynamicallyAccessedMembers.None for the intrinsics which don't return types.
496+
returnValue ??= calledMethod.ReturnsVoid () ? MultiValueLattice.Top : GetMethodReturnValue (calledMethod, returnValueDynamicallyAccessedMemberTypes);
473497

474498
// Validate that the return value has the correct annotations as per the method return value annotations
475499
if (returnValueDynamicallyAccessedMemberTypes != 0) {
@@ -480,6 +504,8 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl
480504
} else if (uniqueValue is SystemTypeValue) {
481505
// SystemTypeValue can fullfill any requirement, so it's always valid
482506
// The requirements will be applied at the point where it's consumed (passed as a method parameter, set as field value, returned from the method)
507+
} else if (uniqueValue == NullValue.Instance) {
508+
// NullValue can fulfill any requirements because reflection access to it will typically throw.
483509
} else {
484510
throw new InvalidOperationException ($"Internal linker error: in {GetContainingSymbolDisplayName ()} processing call to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds.");
485511
}

src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ bool AnalyzeGenericInstantiationTypeArray (in AnalysisContext analysisContext, i
10151015

10161016
bool allIndicesKnown = true;
10171017
for (int i = 0; i < size.Value; i++) {
1018-
if (!array.TryGetValueByIndex (i, out MultiValue value) || value.IsEmpty () || value.AsSingleValue () is UnknownValue) {
1018+
if (!array.TryGetValueByIndex (i, out MultiValue value) || value.AsSingleValue () is UnknownValue) {
10191019
allIndicesKnown = false;
10201020
break;
10211021
}
@@ -1058,7 +1058,15 @@ void ProcessCreateInstanceByName (in AnalysisContext analysisContext, MethodDefi
10581058

10591059
foreach (var assemblyNameValue in methodParams[methodParamsOffset]) {
10601060
if (assemblyNameValue is KnownStringValue assemblyNameStringValue) {
1061+
if (assemblyNameStringValue.Contents is string assemblyName && assemblyName.Length == 0) {
1062+
// Throws exception for zero-length assembly name.
1063+
continue;
1064+
}
10611065
foreach (var typeNameValue in methodParams[methodParamsOffset + 1]) {
1066+
if (typeNameValue is NullValue) {
1067+
// Throws exception for null type name.
1068+
continue;
1069+
}
10621070
if (typeNameValue is KnownStringValue typeNameStringValue) {
10631071
var resolvedAssembly = _context.TryResolve (assemblyNameStringValue.Contents);
10641072
if (resolvedAssembly == null) {

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ static void Main ()
2121
TestNull ();
2222
TestMultipleValues ();
2323
TestUnknownValue ();
24+
TestNoValue ();
25+
TestObjectGetTypeValue ();
2426
}
2527

2628
[ExpectedWarning ("IL2072", nameof (RequirePublicConstructors))]
@@ -92,6 +94,28 @@ static void TestUnknownValue (object[] o = null)
9294
RequireNothing (unknown); // shouldn't warn
9395
}
9496

97+
static void TestNoValue ()
98+
{
99+
Type t = null;
100+
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
101+
// t.TypeHandle throws at runtime so don't warn here.
102+
RequirePublicConstructors (noValue.AssemblyQualifiedName);
103+
}
104+
105+
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors)]
106+
class AnnotatedType
107+
{
108+
}
109+
110+
static void TestObjectGetTypeValue (AnnotatedType instance = null)
111+
{
112+
string type = instance.GetType ().AssemblyQualifiedName;
113+
// Currently Object.GetType is unimplemented in the analyzer, but
114+
// this still shouldn't warn.
115+
RequirePublicConstructors (type);
116+
RequireNothing (type);
117+
}
118+
95119
private static void RequirePublicParameterlessConstructor (
96120
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
97121
string type)

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,23 @@ public static void Main ()
2424

2525
class GetInterface_Name
2626
{
27+
static void TestNullName (Type type)
28+
{
29+
type.GetInterface (null);
30+
}
31+
32+
static void TestEmptyName (Type type)
33+
{
34+
type.GetInterface (string.Empty);
35+
}
36+
37+
static void TestNoValueName (Type type)
38+
{
39+
Type t = null;
40+
string noValue = t.AssemblyQualifiedName;
41+
type.GetInterface (noValue);
42+
}
43+
2744
[ExpectedWarning ("IL2070", nameof (Type.GetInterface), nameof (DynamicallyAccessedMemberTypes) + "." + nameof (DynamicallyAccessedMemberTypes.Interfaces))]
2845
static void TestNoAnnotation (Type type)
2946
{
@@ -76,6 +93,20 @@ static void TestMergedValues (int p, [DynamicallyAccessedMembers (DynamicallyAcc
7693
type.GetInterface ("ITestInterface").RequiresInterfaces ();
7794
}
7895

96+
static void TestNullValue ()
97+
{
98+
Type t = null;
99+
t.GetInterface ("ITestInterface").RequiresInterfaces ();
100+
}
101+
102+
static void TestNoValue ()
103+
{
104+
Type t = null;
105+
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
106+
// t.TypeHandle throws at runtime so don't warn here.
107+
noValue.GetInterface ("ITestInterface").RequiresInterfaces ();
108+
}
109+
79110
class GetInterfaceInCtor
80111
{
81112
public GetInterfaceInCtor ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type type)
@@ -86,13 +117,18 @@ public GetInterfaceInCtor ([DynamicallyAccessedMembers (DynamicallyAccessedMembe
86117

87118
public static void Test ()
88119
{
120+
TestNullName (typeof (TestType));
121+
TestEmptyName (typeof (TestType));
122+
TestNoValueName (typeof (TestType));
89123
TestNoAnnotation (typeof (TestType));
90124
TestWithAnnotation (typeof (TestType));
91125
TestWithAnnotation (typeof (ITestInterface));
92126
TestWithAll (typeof (TestType));
93127
TestKnownType ();
94128
TestMultipleValues (0, typeof (TestType));
95129
TestMergedValues (0, typeof (TestType));
130+
TestNullValue ();
131+
TestNoValue ();
96132
var _ = new GetInterfaceInCtor (typeof (TestType));
97133
}
98134
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ public static void Main ()
2121
TestPublicParameterlessConstructor ();
2222
TestPublicConstructors ();
2323
TestConstructors ();
24+
TestNull ();
25+
TestNoValue ();
2426
TestUnknownType ();
2527

2628
TestTypeNameFromParameter (null);
@@ -69,6 +71,22 @@ static void TestConstructors ()
6971
type.RequiresNone ();
7072
}
7173

74+
[ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresAll) + "(Type)", nameof (Type.GetType) + "(String)")]
75+
static void TestNull ()
76+
{
77+
// Warns about the return value of GetType, even though this throws at runtime.
78+
Type.GetType (null).RequiresAll ();
79+
}
80+
81+
[ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresAll) + "(Type)", nameof (Type.GetType) + "(String)")]
82+
static void TestNoValue ()
83+
{
84+
Type t = null;
85+
string noValue = t.AssemblyQualifiedName;
86+
// Warns about the return value of GetType, even though AssemblyQualifiedName throws at runtime.
87+
Type.GetType (noValue).RequiresAll ();
88+
}
89+
7290
[ExpectedWarning ("IL2057", nameof (GetType))]
7391
static void TestUnknownType ()
7492
{

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ public static void Main ()
1717
{
1818
TestNoAnnotations (typeof (TestType));
1919
TestWithAnnotations (typeof (TestType));
20+
TestWithNull ();
21+
TestWithNoValue ();
2022
}
2123

2224
[ExpectedWarning ("IL2067", nameof (DataFlowTypeExtensions.RequiresPublicMethods))]
@@ -34,6 +36,19 @@ static void TestWithAnnotations ([DynamicallyAccessedMembers (DynamicallyAccesse
3436
t.GetTypeInfo ().RequiresNone ();
3537
}
3638

39+
static void TestWithNull ()
40+
{
41+
Type t = null;
42+
t.GetTypeInfo ().RequiresPublicMethods ();
43+
}
44+
45+
static void TestWithNoValue ()
46+
{
47+
Type t = null;
48+
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
49+
noValue.GetTypeInfo ().RequiresPublicMethods ();
50+
}
51+
3752
class TestType { }
3853
}
3954
}

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ class MakeGenericType
2222
public static void Test ()
2323
{
2424
TestNullType ();
25+
TestNoValueInput ();
2526
TestUnknownInput (null);
27+
TestNullTypeArgument ();
28+
TestNoValueTypeArgument ();
2629
TestWithUnknownTypeArray (null);
2730
TestWithArrayUnknownIndexSet (0);
2831
TestWithArrayUnknownLengthSet (1);
@@ -53,6 +56,26 @@ static void TestNullType ()
5356
nullType.MakeGenericType (typeof (TestType));
5457
}
5558

59+
static void TestNoValueInput ()
60+
{
61+
Type t = null;
62+
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
63+
noValue.MakeGenericType (typeof (TestType));
64+
}
65+
66+
static void TestNullTypeArgument ()
67+
{
68+
Type t = null;
69+
typeof (GenericWithPublicFieldsArgument<>).MakeGenericType (t);
70+
}
71+
72+
static void TestNoValueTypeArgument ()
73+
{
74+
Type t = null;
75+
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
76+
typeof (GenericWithPublicFieldsArgument<>).MakeGenericType (noValue);
77+
}
78+
5679
[ExpectedWarning ("IL2055", nameof (Type.MakeGenericType))]
5780
static void TestUnknownInput (Type inputType)
5881
{
@@ -194,8 +217,11 @@ class MakeGenericMethod
194217
public static void Test ()
195218
{
196219
TestNullMethod ();
220+
TestNoValueMethod ();
197221
TestUnknownMethod (null);
198222
TestUnknownMethodButNoTypeArguments (null);
223+
TestNullTypeArgument ();
224+
TestNoValueTypeArgument ();
199225
TestWithUnknownTypeArray (null);
200226
TestWithArrayUnknownIndexSet (0);
201227
TestWithArrayUnknownIndexSetByRef (0);
@@ -235,6 +261,14 @@ static void TestNullMethod ()
235261
mi.MakeGenericMethod (typeof (TestType));
236262
}
237263

264+
[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod) + "(Type[])")]
265+
static void TestNoValueMethod ()
266+
{
267+
// GetMethod(null) throws at runtime.
268+
MethodInfo noValue = typeof (MakeGenericMethod).GetMethod (null);
269+
noValue.MakeGenericMethod (typeof (TestType));
270+
}
271+
238272
[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod))]
239273
static void TestUnknownMethod (MethodInfo mi)
240274
{
@@ -248,6 +282,21 @@ static void TestUnknownMethodButNoTypeArguments (MethodInfo mi)
248282
mi.MakeGenericMethod (Type.EmptyTypes);
249283
}
250284

285+
static void TestNullTypeArgument ()
286+
{
287+
Type t = null;
288+
typeof (MakeGenericMethod).GetMethod (nameof (GenericWithRequirements), BindingFlags.Static)
289+
.MakeGenericMethod (t);
290+
}
291+
292+
static void TestNoValueTypeArgument ()
293+
{
294+
Type t = null;
295+
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
296+
typeof (MakeGenericMethod).GetMethod (nameof (GenericWithRequirements), BindingFlags.Static)
297+
.MakeGenericMethod (noValue);
298+
}
299+
251300
[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod))]
252301
static void TestWithUnknownTypeArray (Type[] types)
253302
{

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public static void Main ()
4444
TestNoAnnotation (typeof (TestType));
4545
TestAnnotatedAndUnannotated (typeof (TestType), typeof (TestType), 0);
4646
TestNull ();
47+
TestNoValue ();
4748

4849
Mixed_Derived.Test (typeof (TestType), 0);
4950
}
@@ -211,6 +212,15 @@ static void TestNull ()
211212
type.BaseType.RequiresPublicMethods ();
212213
}
213214

215+
[ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions) + "." + nameof (DataFlowTypeExtensions.RequiresPublicMethods))]
216+
static void TestNoValue ()
217+
{
218+
Type t = null;
219+
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
220+
// Warns about the base type even though the above throws an exception at runtime.
221+
noValue.BaseType.RequiresPublicMethods ();
222+
}
223+
214224
class Mixed_Base
215225
{
216226
public static void PublicMethod () { }

0 commit comments

Comments
 (0)