ILLink: fix TypeProxy implementation of equality (dotnet#106990)

The equality check needs to determine whether two TypeProxy instances
represent the same type. The check was incorrect when two different
object instances were allocated to represent the same generic
instantiated type.

ILCompiler doesn't have this problem because it uses a cache to ensure
that the same object instance represents a given instantiated generic
type.

Discovered while investigating
dotnet#106215, see more context at
dotnet#106215 (comment).

The new testcase also uncovered an issue in the analyzer that was
fixed in dotnet#106909.

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

@@ -56,7 +56,7 @@ MultiValue GetValueForCustomAttributeArgument (CustomAttributeArgument argument)
  TypeDefinition? referencedType = ((TypeReference) argument.Value).ResolveToTypeDefinition (_context);
  return referencedType == null
  ? UnknownValue.Instance
- : new SystemTypeValue (referencedType);
+ : new SystemTypeValue (new (referencedType, _context));
  }
 
  if (argument.Type.MetadataType == MetadataType.String)
@@ -69,7 +69,7 @@ MultiValue GetValueForCustomAttributeArgument (CustomAttributeArgument argument)
  void RequireDynamicallyAccessedMembers (in DiagnosticContext diagnosticContext, in MultiValue value, ValueWithDynamicallyAccessedMembers targetValue)
  {
  var reflectionMarker = new ReflectionMarker (_context, _markStep, enabled: true);
- var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (reflectionMarker, diagnosticContext);
+ var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (_context, reflectionMarker, diagnosticContext);
  requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
  }
  }

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

@@ -16,9 +16,9 @@ namespace ILLink.Shared.TrimAnalysis
  /// </summary>
  internal sealed partial record FieldValue
  {
- public FieldValue (FieldReference fieldToLoad, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
+ public FieldValue (FieldReference fieldToLoad, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, ITryResolveMetadata resolver)
  {
- StaticType = fieldToLoad.FieldType.InflateFrom (fieldToLoad.DeclaringType as IGenericInstance);
+ StaticType = new (fieldToLoad.FieldType.InflateFrom (fieldToLoad.DeclaringType as IGenericInstance), resolver);
  Field = fieldToLoad;
  DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes;
  }

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

@@ -711,7 +711,7 @@ internal partial bool MethodRequiresDataFlowAnalysis (MethodProxy method)
 
 #pragma warning disable CA1822 // Mark members as static - Should be an instance method for consistency
  internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
- => MethodReturnValue.Create (method, isNewObj, dynamicallyAccessedMemberTypes);
+ => MethodReturnValue.Create (method, isNewObj, dynamicallyAccessedMemberTypes, _context);
 #pragma warning restore CA1822 // Mark members as static
 
  internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj)
@@ -727,7 +727,7 @@ internal partial GenericParameterValue GetGenericParameterValue (GenericParamete
 
 #pragma warning disable CA1822 // Mark members as static - Should be an instance method for consistency
  internal partial MethodParameterValue GetMethodParameterValue (ParameterProxy param, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
- => new (param.ParameterType, param, dynamicallyAccessedMemberTypes);
+ => new (param.ParameterType, param, dynamicallyAccessedMemberTypes, _context);
 #pragma warning restore CA1822 // Mark members as static
 
  internal partial MethodParameterValue GetMethodParameterValue (ParameterProxy param)
@@ -738,7 +738,7 @@ internal partial MethodParameterValue GetMethodThisParameterValue (MethodProxy m
  {
  if (!method.HasImplicitThis ())
  throw new InvalidOperationException ($"Cannot get 'this' parameter of method {method.GetDisplayName ()} with no 'this' parameter.");
- return new MethodParameterValue (method.Method.DeclaringType, new ParameterProxy (method, (ParameterIndex) 0), dynamicallyAccessedMemberTypes);
+ return new MethodParameterValue (method.Method.DeclaringType, new ParameterProxy (method, (ParameterIndex) 0), dynamicallyAccessedMemberTypes, _context);
  }
 #pragma warning restore CA1822 // Mark members as static
 
@@ -756,7 +756,7 @@ internal SingleValue GetFieldValue (FieldReference field)
  => field.Name switch {
  "EmptyTypes" when field.DeclaringType.IsTypeOf (WellKnownType.System_Type) => ArrayValue.Create (0, field.DeclaringType),
  "Empty" when field.DeclaringType.IsTypeOf (WellKnownType.System_String) => new KnownStringValue (string.Empty),
- _ => new FieldValue (field, GetFieldAnnotation (field))
+ _ => new FieldValue (field, GetFieldAnnotation (field), _context)
  };
 
  internal SingleValue GetTypeValueFromGenericArgument (TypeReference genericArgument)
@@ -770,18 +770,18 @@ internal SingleValue GetTypeValueFromGenericArgument (TypeReference genericArgum
  var innerGenericArgument = (genericArgument as IGenericInstance)?.GenericArguments.FirstOrDefault ();
  switch (innerGenericArgument) {
  case GenericParameter gp:
- return new NullableValueWithDynamicallyAccessedMembers (genericArgumentType,
+ return new NullableValueWithDynamicallyAccessedMembers (new (genericArgumentType, _context),
  new GenericParameterValue (gp, _context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (gp)));
 
  case TypeReference underlyingType:
  if (underlyingType.ResolveToTypeDefinition (_context) is TypeDefinition underlyingTypeDefinition)
- return new NullableSystemTypeValue (genericArgumentType, new SystemTypeValue (underlyingTypeDefinition));
+ return new NullableSystemTypeValue (new (genericArgumentType, _context), new SystemTypeValue (new (underlyingTypeDefinition, _context)));
  else
  return UnknownValue.Instance;
  }
  }
  // All values except for Nullable<T>, including Nullable<> (with no type arguments)
- return new SystemTypeValue (genericArgumentType);
+ return new SystemTypeValue (new (genericArgumentType, _context));
  } else {
  return UnknownValue.Instance;
  }

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

@@ -37,7 +37,7 @@ public void ProcessGenericArgumentDataFlow (GenericParameter genericParameter, T
  void RequireDynamicallyAccessedMembers (in DiagnosticContext diagnosticContext, in MultiValue value, ValueWithDynamicallyAccessedMembers targetValue)
  {
  var reflectionMarker = new ReflectionMarker (_context, _markStep, enabled: true);
- var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (reflectionMarker, diagnosticContext);
+ var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (_context, reflectionMarker, diagnosticContext);
  requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
  }
  }

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

@@ -41,7 +41,7 @@ public HandleCallAction (
  _diagnosticContext = diagnosticContext;
  _callingMethodDefinition = callingMethodDefinition;
  _annotations = context.Annotations.FlowAnnotations;
- _requireDynamicallyAccessedMembersAction = new (reflectionMarker, diagnosticContext);
+ _requireDynamicallyAccessedMembersAction = new (context, reflectionMarker, diagnosticContext);
  }
 
  private partial bool TryHandleIntrinsic (
@@ -155,7 +155,7 @@ private partial bool TryHandleIntrinsic (
  // This can be seen a little bit as a violation of the annotation, but we already have similar cases
  // where a parameter is annotated and if something in the method sets a specific known type to it
  // we will also make it just work, even if the annotation doesn't match the usage.
- AddReturnValue (new SystemTypeValue (staticType));
+ AddReturnValue (new SystemTypeValue (new (staticType, _context)));
  } else if (staticTypeDef.IsTypeOf ("System", "Enum")) {
  AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.PublicFields));
  } else {
@@ -220,13 +220,13 @@ private partial IEnumerable<SystemReflectionMethodBaseValue> GetMethodsOnTypeHie
  private partial IEnumerable<SystemTypeValue> GetNestedTypesOnType (TypeProxy type, string name, BindingFlags? bindingFlags)
  {
  foreach (var nestedType in type.Type.GetNestedTypesOnType (_context, t => t.Name == name, bindingFlags))
- yield return new SystemTypeValue (new TypeProxy (nestedType));
+ yield return new SystemTypeValue (new TypeProxy (nestedType, _context));
  }
 
  private partial bool TryGetBaseType (TypeProxy type, out TypeProxy? baseType)
  {
  if (type.Type.ResolveToTypeDefinition (_context)?.BaseType is TypeReference baseTypeRef && _context.TryResolve (baseTypeRef) is TypeDefinition baseTypeDefinition) {
- baseType = new TypeProxy (baseTypeDefinition);
+ baseType = new TypeProxy (baseTypeDefinition, _context);
  return true;
  }
 
@@ -255,7 +255,7 @@ private partial bool TryResolveTypeNameForCreateInstanceAndMark (in MethodProxy
  return false;
  }
 
- resolvedType = new TypeProxy (resolvedTypeDefinition);
+ resolvedType = new TypeProxy (resolvedTypeDefinition, _context);
  return true;
  }
 

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

@@ -791,20 +791,20 @@ void ScanLdtoken (Instruction operation, Stack<StackSlot> currentStack)
  if (typeReference is IGenericInstance instance && resolvedDefinition.IsTypeOf (WellKnownType.System_Nullable_T)) {
  switch (instance.GenericArguments[0]) {
  case GenericParameter genericParam:
- var nullableDam = new RuntimeTypeHandleForNullableValueWithDynamicallyAccessedMembers (new TypeProxy (resolvedDefinition),
+ var nullableDam = new RuntimeTypeHandleForNullableValueWithDynamicallyAccessedMembers (new TypeProxy (resolvedDefinition, _context),
  new RuntimeTypeHandleForGenericParameterValue (genericParam));
  currentStack.Push (new StackSlot (nullableDam));
  return;
  case TypeReference underlyingTypeReference when ResolveToTypeDefinition (underlyingTypeReference) is TypeDefinition underlyingType:
- var nullableType = new RuntimeTypeHandleForNullableSystemTypeValue (new TypeProxy (resolvedDefinition), new SystemTypeValue (underlyingType));
+ var nullableType = new RuntimeTypeHandleForNullableSystemTypeValue (new TypeProxy (resolvedDefinition, _context), new SystemTypeValue (new (underlyingType, _context)));
  currentStack.Push (new StackSlot (nullableType));
  return;
  default:
  PushUnknown (currentStack);
  return;
  }
  } else {
- var typeHandle = new RuntimeTypeHandleValue (new TypeProxy (resolvedDefinition));
+ var typeHandle = new RuntimeTypeHandleValue (new TypeProxy (resolvedDefinition, _context));
  currentStack.Push (new StackSlot (typeHandle));
  return;
  }

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

@@ -3,9 +3,9 @@
 
 using System.Diagnostics.CodeAnalysis;
 using ILLink.Shared.TypeSystemProxy;
+using Mono.Linker;
 using TypeReference = Mono.Cecil.TypeReference;
 
-
 namespace ILLink.Shared.TrimAnalysis
 {
 
@@ -14,9 +14,9 @@ namespace ILLink.Shared.TrimAnalysis
  /// </summary>
  internal partial record MethodParameterValue
  {
- public MethodParameterValue (TypeReference? staticType, ParameterProxy param, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
+ public MethodParameterValue (TypeReference? staticType, ParameterProxy param, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, ITryResolveMetadata resolver)
  {
- StaticType = staticType == null ? null : new (staticType);
+ StaticType = staticType == null ? null : new (staticType, resolver);
  DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes;
  Parameter = param;
  }

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

@@ -19,17 +19,17 @@ namespace ILLink.Shared.TrimAnalysis
  /// </summary>
  internal partial record MethodReturnValue
  {
- public static MethodReturnValue Create (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
+ public static MethodReturnValue Create (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, ITryResolveMetadata resolver)
  {
  Debug.Assert (!isNewObj || method.Definition.IsConstructor, "isNewObj can only be true for constructors");
  var methodRef = method.Method;
  var staticType = isNewObj ? methodRef.DeclaringType : methodRef.ReturnType.InflateFrom (methodRef as IGenericInstance ?? methodRef.DeclaringType as IGenericInstance);
- return new MethodReturnValue (staticType, method.Definition, dynamicallyAccessedMemberTypes);
+ return new MethodReturnValue (staticType, method.Definition, dynamicallyAccessedMemberTypes, resolver);
  }
 
- private MethodReturnValue (TypeReference? staticType, MethodDefinition method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
+ private MethodReturnValue (TypeReference? staticType, MethodDefinition method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, ITryResolveMetadata resolver)
  {
- StaticType = staticType == null ? null : new (staticType);
+ StaticType = staticType == null ? null : new (staticType, resolver);
  Method = method;
  DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes;
  }

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

@@ -11,20 +11,23 @@ namespace ILLink.Shared.TrimAnalysis
 {
  internal partial struct RequireDynamicallyAccessedMembersAction
  {
+ readonly ITryResolveMetadata _resolver;
  readonly ReflectionMarker _reflectionMarker;
 
  public RequireDynamicallyAccessedMembersAction (
+ ITryResolveMetadata resolver,
  ReflectionMarker reflectionMarker,
  in DiagnosticContext diagnosticContext)
  {
+ _resolver = resolver;
  _reflectionMarker = reflectionMarker;
  _diagnosticContext = diagnosticContext;
  }
 
  public partial bool TryResolveTypeNameAndMark (string typeName, bool needsAssemblyName, out TypeProxy type)
  {
  if (_reflectionMarker.TryResolveTypeNameAndMark (typeName, _diagnosticContext, needsAssemblyName, out TypeDefinition? foundType)) {
- type = new (foundType);
+ type = new (foundType, _resolver);
  return true;
  } else {
  type = default;

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

@@ -49,7 +49,7 @@ public void MarkAndProduceDiagnostics (ReflectionMarker reflectionMarker, LinkCo
  if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers)
  throw new NotImplementedException ();
 
- var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (reflectionMarker, diagnosticContext);
+ var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (context, reflectionMarker, diagnosticContext);
  requireDynamicallyAccessedMembersAction.Invoke (sourceValue, targetWithDynamicallyAccessedMembers);
  }
  }

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

@@ -1,17 +1,20 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.Immutable;
 using Mono.Cecil;
 using Mono.Linker;
 
 namespace ILLink.Shared.TypeSystemProxy
 {
- internal readonly partial struct TypeProxy
+ internal readonly partial struct TypeProxy : IEquatable<TypeProxy>
  {
- public TypeProxy (TypeReference type) => Type = type;
-
- public static implicit operator TypeProxy (TypeReference type) => new (type);
+ public TypeProxy (TypeReference type, ITryResolveMetadata resolver)
+ {
+ Type = type;
+ this.resolver = resolver;
+ }
 
  internal partial ImmutableArray<GenericParameterProxy> GetGenericParameters ()
  {
@@ -26,6 +29,8 @@ internal partial ImmutableArray<GenericParameterProxy> GetGenericParameters ()
  return builder.ToImmutableArray ();
  }
 
+ private readonly ITryResolveMetadata resolver;
+
  public TypeReference Type { get; }
 
  public string Name { get => Type.Name; }
@@ -39,5 +44,11 @@ internal partial ImmutableArray<GenericParameterProxy> GetGenericParameters ()
  public string GetDisplayName () => Type.GetDisplayName ();
 
  public override string ToString () => Type.ToString ();
+
+ public bool Equals (TypeProxy other) => TypeReferenceEqualityComparer.AreEqual (Type, other.Type, resolver);
+
+ public override bool Equals (object? o) => o is TypeProxy other && Equals (other);
+
+ public override int GetHashCode () => TypeReferenceEqualityComparer.GetHashCodeFor (Type);
  }
 }

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

@@ -0,0 +1,17 @@
+using System;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace ILLink.RoslynAnalyzer.Tests
+{
+ public sealed partial class GenericsTests : LinkerTestBase
+ {
+ protected override string TestSuiteName => "Generics";
+
+ [Fact]
+ public Task InstantiatedGenericEquality ()
+ {
+ return RunTest ();
+ }
+ }
+}

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/GenericsTests.g.cs

@@ -7,8 +7,6 @@ namespace ILLink.RoslynAnalyzer.Tests
  public sealed partial class GenericsTests : LinkerTestBase
  {
 
- protected override string TestSuiteName => "Generics";
-
  [Fact]
  public Task ArrayVariantCasting ()
  {

src/tools/illink/test/Mono.Linker.Tests.Cases/Generics/InstantiatedGenericEquality.cs

@@ -0,0 +1,51 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Diagnostics.CodeAnalysis;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Helpers;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.Generics
+{
+ [ExpectedNoWarnings]
+ class InstantiatedGenericEquality
+ {
+ public static void Main ()
+ {
+ GenericReturnType.Test ();
+ }
+
+ class GenericReturnType
+ {
+ [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+ [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+ class ReturnType<T>
+ {
+ [Kept]
+ public void Method () { }
+ }
+
+ [Kept]
+ static ReturnType<T> GetGenericReturnType<T> () => default;
+
+ // Regression test for an issue where ILLink's representation of a generic instantiated type
+ // was using reference equality. The test uses a lambda to ensure that it goes through the
+ // interprocedural analysis code path that merges patterns and relies on a correct implementation
+ // of equality.
+ [Kept]
+ public static void Test ()
+ {
+ var instance = GetGenericReturnType<int> ();
+
+ var lambda =
+ () => {
+ var type = instance.GetType ();
+ type.GetMethod ("Method");
+ };
+
+ lambda ();
+ }
+ }
+ }
+}