Fix check for types interesting to dataflow (#105642)

This makes the check `IsTypeInterestingForDataflow` more consistent
across ILLink/ILC/analyzer.

Fixes #104761, and fixes
analyzer behavior for byrefs of string.

src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs

@@ -148,18 +148,18 @@ public override void Initialize (AnalysisContext context)
  static void VerifyMemberOnlyApplyToTypesOrStrings (SymbolAnalysisContext context, ISymbol member)
  {
  var location = GetPrimaryLocation (member.Locations);
- if (member is IFieldSymbol field && field.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !field.Type.IsTypeInterestingForDataflow ())
+ if (member is IFieldSymbol field && field.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !field.Type.IsTypeInterestingForDataflow (isByRef: field.RefKind is not RefKind.None))
  context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersOnFieldCanOnlyApplyToTypesOrStrings), location, member.GetDisplayName ()));
  else if (member is IMethodSymbol method) {
- if (method.GetDynamicallyAccessedMemberTypesOnReturnType () != DynamicallyAccessedMemberTypes.None && !method.ReturnType.IsTypeInterestingForDataflow ())
+ if (method.GetDynamicallyAccessedMemberTypesOnReturnType () != DynamicallyAccessedMemberTypes.None && !method.ReturnType.IsTypeInterestingForDataflow (isByRef: method.ReturnsByRef))
  context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersOnMethodReturnValueCanOnlyApplyToTypesOrStrings), location, member.GetDisplayName ()));
- if (method.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !method.ContainingType.IsTypeInterestingForDataflow ())
+ if (method.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !method.ContainingType.IsTypeInterestingForDataflow (isByRef: false))
  context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersIsNotAllowedOnMethods), location));
  foreach (var parameter in method.Parameters) {
- if (parameter.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !parameter.Type.IsTypeInterestingForDataflow ())
+ if (parameter.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !parameter.Type.IsTypeInterestingForDataflow (isByRef: parameter.RefKind is not RefKind.None))
  context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersOnMethodParameterCanOnlyApplyToTypesOrStrings), location, parameter.GetDisplayName (), member.GetDisplayName ()));
  }
- } else if (member is IPropertySymbol property && property.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !property.Type.IsTypeInterestingForDataflow ()) {
+ } else if (member is IPropertySymbol property && property.GetDynamicallyAccessedMemberTypes () != DynamicallyAccessedMemberTypes.None && !property.Type.IsTypeInterestingForDataflow (isByRef: property.ReturnsByRef)) {
  context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersOnPropertyCanOnlyApplyToTypesOrStrings), location, member.GetDisplayName ()));
  }
  }

src/tools/illink/src/ILLink.RoslynAnalyzer/ITypeSymbolExtensions.cs

@@ -16,16 +16,19 @@ private enum HierarchyFlags
  IsSystemReflectionIReflect = 0x02,
  }
 
- public static bool IsTypeInterestingForDataflow (this ITypeSymbol type)
+ public static bool IsTypeInterestingForDataflow (this ITypeSymbol type, bool isByRef)
  {
- if (type.SpecialType == SpecialType.System_String)
+ if (type.SpecialType is SpecialType.System_String && !isByRef)
  return true;
 
- var flags = GetFlags (type);
+ if (type is not INamedTypeSymbol namedType)
+ return false;
+
+ var flags = GetFlags (namedType);
  return IsSystemType (flags) || IsSystemReflectionIReflect (flags);
  }
 
- private static HierarchyFlags GetFlags (ITypeSymbol type)
+ private static HierarchyFlags GetFlags (INamedTypeSymbol type)
  {
  HierarchyFlags flags = 0;
  if (type.IsTypeOf (WellKnownType.System_Reflection_IReflect)) {

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

@@ -121,8 +121,8 @@ public override MultiValue VisitConversion (IConversionOperation operation, Stat
  {
  var value = base.VisitConversion (operation, state);
 
- if (operation.OperatorMethod != null)
- return operation.OperatorMethod.ReturnType.IsTypeInterestingForDataflow () ? new MethodReturnValue (operation.OperatorMethod, isNewObj: false) : value;
+ if (operation.OperatorMethod is IMethodSymbol method)
+ return method.ReturnType.IsTypeInterestingForDataflow (isByRef: method.ReturnsByRef) ? new MethodReturnValue (method, isNewObj: false) : value;
 
  // TODO - is it possible to have annotation on the operator method parameters?
  // if so, will these be checked here?
@@ -346,7 +346,7 @@ public override void HandleReturnValue (MultiValue returnValue, IOperation opera
  if (OwningSymbol is not IMethodSymbol method)
  return;
 
- if (method.ReturnType.IsTypeInterestingForDataflow ()) {
+ if (method.ReturnType.IsTypeInterestingForDataflow (isByRef: method.ReturnsByRef)) {
  var returnParameter = new MethodReturnValue (method, isNewObj: false);
 
  TrimAnalysisPatterns.Add (

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

@@ -151,7 +151,14 @@ public bool IsTypeInterestingForDataflow (TypeReference typeReference)
  if (typeReference.MetadataType == MetadataType.String)
  return true;
 
- TypeDefinition? type = _context.TryResolve (typeReference);
+ // ByRef over an interesting type is itself interesting
+ if (typeReference.IsByReference)
+ typeReference = ((ByReferenceType) typeReference).ElementType;
+
+ if (!typeReference.IsNamedType ())
+ return false;
+
+ TypeDefinition? type = typeReference.ResolveToTypeDefinition (_context);
  return type != null && (
  _hierarchyInfo.IsSystemType (type) ||
  _hierarchyInfo.IsSystemReflectionIReflect (type));

src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs

@@ -408,6 +408,27 @@ public static TypeReference WithoutModifiers (this TypeReference type)
  return type;
  }
 
+ // Check whether this type represents a "named type" (i.e. a type that has a name and can be resolved to a TypeDefinition),
+ // not an array, pointer, byref, or generic parameter. Conceptually this is supposed to represent the same idea as Roslyn's
+ // INamedTypeSymbol, or ILC's DefType/MetadataType.
+ public static bool IsNamedType (this TypeReference typeReference) {
+ if (typeReference.IsDefinition || typeReference.IsGenericInstance)
+ return true;
+
+ if (typeReference.IsArray || typeReference.IsByReference || typeReference.IsPointer || typeReference.IsGenericParameter)
+ return false;
+
+ // Shouldn't get called for these cases
+ Debug.Assert (!typeReference.IsFunctionPointer);
+ Debug.Assert (!typeReference.IsRequiredModifier);
+ Debug.Assert (!typeReference.IsOptionalModifier);
+ Debug.Assert (!typeReference.IsPinned);
+ Debug.Assert (!typeReference.IsSentinel);
+
+ Debug.Assert (typeReference.GetType () == typeof (TypeReference));
+ return true;
+ }
+
  // Array types that are dynamically accessed should resolve to System.Array instead of its element type - which is what Cecil resolves to.
  // Any data flow annotations placed on a type parameter which receives an array type apply to the array itself. None of the members in its
  // element type should be marked.

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

@@ -31,6 +31,7 @@ public static void Main ()
  typeof (AttributeConstructorDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicConstructorAttribute));
  typeof (AttributeConstructorDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
  AllOnSelf.Test ();
+ AnnotationOnTypeArray.Test ();
  }
 
  [Kept]
@@ -160,6 +161,43 @@ public void Method () { }
  }
  }
 
+ [Kept]
+ class AnnotationOnTypeArray
+ {
+ [Kept]
+ [KeptBaseType (typeof (Attribute))]
+ class AttributeRequiresTypeArrayAttribute : Attribute
+ {
+ [Kept]
+ [ExpectedWarning ("IL2098")]
+ [UnexpectedWarning ("IL2067", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ public AttributeRequiresTypeArrayAttribute (
+ [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ Type[] types)
+ {
+ RequirePublicFields (types);
+ }
+
+ [Kept]
+ [ExpectedWarning ("IL2098")]
+ static void RequirePublicFields (
+ [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)]
+ Type[] types)
+ {
+ }
+ }
+
+ [Kept]
+ [KeptAttributeAttribute (typeof (AttributeRequiresTypeArrayAttribute))]
+ [UnexpectedWarning ("IL2062", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ [AttributeRequiresTypeArray (new Type[] { typeof (int) })]
+ public static void Test () {
+ typeof (AnnotationOnTypeArray).GetMethod ("Test").GetCustomAttribute (typeof (AttributeRequiresTypeArrayAttribute));
+ }
+ }
+
  [Kept]
  [KeptBaseType (typeof (Attribute))]
  class TypeArrayAttribute : Attribute

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

@@ -345,11 +345,52 @@ static void TestFlowOutOfField ()
  }
 
  [UnexpectedWarning ("IL2074", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
- public static void Test () {
+ static void TestUnsupportedType () {
  var t = GetUnsupportedTypeInstance ();
  unsupportedTypeInstance = t;
  TestFlowOutOfField ();
  }
+
+ ref struct StringRef
+ {
+ [ExpectedWarning ("IL2097")]
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ public ref string stringRef;
+
+ [UnexpectedWarning ("IL2069", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ public StringRef (ref string s)
+ {
+ stringRef = ref s;
+ }
+
+ static string GetString () => null;
+
+ [ExpectedWarning ("IL2098")]
+ static void RequirePublicFields (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)]
+ ref string s)
+ {
+ }
+
+ [UnexpectedWarning ("IL2077", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ void TestFlowOutOfField ()
+ {
+ RequirePublicFields (ref stringRef);
+ }
+
+ public static void Test ()
+ {
+ string s = GetString ();
+ var stringRef = new StringRef (ref s);
+ stringRef.TestFlowOutOfField ();
+ }
+ }
+
+ public static void Test ()
+ {
+ TestUnsupportedType ();
+ StringRef.Test ();
+ }
  }
 
  class WriteArrayField

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

@@ -13,6 +13,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
  // - so the main validation is done by the ExpectedWarning attributes.
  [SkipKeptItemsValidation]
  [ExpectedNoWarnings]
+ [SetupCompileArgument ("/unsafe")]
  [SetupLinkerArgument ("--keep-metadata", "parametername")]
  public class MethodParametersDataFlow
  {
@@ -44,6 +45,7 @@ public static void Main ()
  TestVarargsMethod (typeof (TestType), __arglist (0, 1, 2));
 #endif
  AnnotationOnUnsupportedParameter.Test ();
+ AnnotationOnByRefParameter.Test ();
  WriteCapturedParameter.Test ();
  }
 
@@ -237,7 +239,7 @@ static void TestVarargsMethod ([DynamicallyAccessedMembers (DynamicallyAccessedM
 
  class AnnotationOnUnsupportedParameter
  {
- class UnsupportedType ()
+ class UnsupportedType
  {
  }
 
@@ -260,10 +262,152 @@ static void RequirePublicFields (
  }
 
  [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
- public static void Test () {
+ static void TestUnsupportedType ()
+ {
  var t = GetUnsupportedTypeInstance ();
  RequirePublicMethods (t);
  }
+
+ static Type[] GetTypeArray () => null;
+
+ [ExpectedWarning ("IL2098")]
+ [UnexpectedWarning ("IL2067", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static void RequirePublicMethods (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ Type[] types)
+ {
+ RequirePublicFields (types);
+ }
+
+ [ExpectedWarning ("IL2098")]
+ static void RequirePublicFields (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)]
+ Type[] types)
+ {
+ }
+
+ [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static void TestTypeArray ()
+ {
+ var types = GetTypeArray ();
+ RequirePublicMethods (types);
+ }
+
+ static unsafe Type* GetTypePtr () => throw null;
+
+ [ExpectedWarning ("IL2098")]
+ [UnexpectedWarning ("IL2067", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static unsafe void RequirePublicMethods (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ Type* typePtr)
+ {
+ RequirePublicFields (typePtr);
+ }
+
+ [ExpectedWarning ("IL2098")]
+ static unsafe void RequirePublicFields (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)]
+ Type* typePtr)
+ {
+ }
+
+ [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static unsafe void TestTypePointer ()
+ {
+ var typePtr = GetTypePtr ();
+ RequirePublicMethods (typePtr);
+ }
+
+ static T GetTConstrainedToType<T> () where T : Type => throw null;
+
+ [ExpectedWarning ("IL2098")]
+ [UnexpectedWarning ("IL2067", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static void RequirePublicMethods<T> (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ T t) where T : Type
+ {
+ RequirePublicFields (t);
+ }
+
+ [ExpectedWarning ("IL2098")]
+ static void RequirePublicFields<T> (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)]
+ T t) where T : Type
+ {
+ }
+
+ [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static void TestTypeGenericParameter ()
+ {
+ var t = GetTConstrainedToType<Type> ();
+ RequirePublicMethods<Type> (t);
+ }
+
+ static ref string GetStringRef () => throw null;
+
+ [ExpectedWarning ("IL2098")]
+ [UnexpectedWarning ("IL2067", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static void RequirePublicMethods (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ ref string stringRef)
+ {
+ RequirePublicFields (ref stringRef);
+ }
+
+ [ExpectedWarning ("IL2098")]
+ static void RequirePublicFields (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)]
+ ref string stringRef)
+ {
+ }
+
+ [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")]
+ static void TestStringRef ()
+ {
+ var stringRef = GetStringRef ();
+ RequirePublicMethods (ref stringRef);
+ }
+
+ public static void Test () {
+ TestUnsupportedType ();
+ TestTypeArray ();
+ TestTypePointer ();
+ TestTypeGenericParameter ();
+ TestStringRef ();
+ }
+ }
+
+ class AnnotationOnByRefParameter
+ {
+ static ref Type GetTypeRef () => throw null;
+
+ [ExpectedWarning ("IL2067")]
+ [ExpectedWarning ("IL2067", Tool.NativeAot | Tool.Trimmer, "https://github.com/dotnet/runtime/issues/101734")]
+ static void RequirePublicMethods (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ ref Type typeRef)
+ {
+ RequirePublicFields (ref typeRef);
+ }
+
+ static void RequirePublicFields (
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)]
+ ref Type typeRef)
+ {
+ }
+
+ [ExpectedWarning ("IL2062", Tool.NativeAot | Tool.Trimmer, "https://github.com/dotnet/runtime/issues/101734")]
+ [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101734")]
+ static void TestTypeRef ()
+ {
+ var typeRef = GetTypeRef ();
+ RequirePublicMethods (ref typeRef);
+ }
+
+ public static void Test ()
+ {
+ TestTypeRef ();
+ }
  }
 
  class WriteCapturedParameter