Fix ILLink behavior for modreq types (dotnet#108494)

Cecil represents modreq/modopt in signatures as part of the TypeReference (using subclasses RequiredModifierType/OptionalModifierType). We need to unwrap these to get the correct behavior in IsNamedType. This fixes the `IsTypeInterestingForDataflow` check for modreq/modopt types (`Type modreq(IsVolatile)` should be considered interesting for dataflow, and `Type[] modreq(IsVolatile)` should not). Includes a similar fix for function pointers.
sirntar · Oct 8, 2024 · 4438652 · 4438652
1 parent df57ef9
commit 4438652
Show file tree

Hide file tree

Showing 5 changed files with 271 additions and 4 deletions.
diff --git a/src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs b/src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs
@@ -415,18 +415,26 @@ public static TypeReference WithoutModifiers (this TypeReference type)
 		// 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.IsRequiredModifier)
+				return ((RequiredModifierType) typeReference).ElementType.IsNamedType ();
+			if (typeReference.IsOptionalModifier)
+				return ((OptionalModifierType) typeReference).ElementType.IsNamedType ();
+
 			if (typeReference.IsDefinition || typeReference.IsGenericInstance)
 				return true;
 
-			if (typeReference.IsArray || typeReference.IsByReference || typeReference.IsPointer || typeReference.IsGenericParameter)
+			if (typeReference.IsArray ||
+				typeReference.IsByReference ||
+				typeReference.IsPointer ||
+				typeReference.IsFunctionPointer ||
+				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);
+			if (typeReference.IsPinned || typeReference.IsSentinel)
+				return false;
 
 			Debug.Assert (typeReference.GetType () == typeof (TypeReference));
 			return true;

diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/ModifierDataFlow.il b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/ModifierDataFlow.il
@@ -0,0 +1,176 @@
+// Metadata version: v4.0.30319
+.assembly extern System.Runtime
+{
+  .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )
+}
+.assembly 'library'
+{
+  .hash algorithm 0x00008004
+  .ver 1:0:0:0
+}
+.module library.dll
+
+.class public auto ansi sealed beforefieldinit Library.ModifierDataFlow
+       extends [System.Runtime]System.Object
+{
+  .field public static class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType) modReqType
+  .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+  .field public static class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType) modreq(Library.ModifierDataFlow/ModifierType) multipleModReqType
+  .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+  .field public static class [System.Runtime]System.Type modopt(Library.ModifierDataFlow/ModifierType) modOptType
+  .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+   .field public static class [System.Runtime]System.Type modopt(Library.ModifierDataFlow/ModifierType) modreq(Library.ModifierDataFlow/ModifierType) modReqModOptType
+   .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+   .field public static class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType) modopt(Library.ModifierDataFlow/ModifierType) modOptModReqType
+   .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+   .field public static class [System.Runtime]System.Type[] modreq(Library.ModifierDataFlow/ModifierType) modReqArrayType
+   .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+   .field public static class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType)[] arrayModReqType
+   .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+   .field public static class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType)[] modreq(Library.ModifierDataFlow/ModifierType) modReqArrayModReqType
+   .custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes) = (
+       01 00 20 00 00 00 00 00
+   )
+
+  .class nested public auto ansi beforefieldinit ModifierType
+         extends [System.Runtime]System.Object
+  {
+  } // end of class ModifierType
+
+  .method private hidebysig static 
+        class [System.Runtime]System.Type GetUnknownType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: ldnull
+        IL_0001: ret
+    } // end of method Library.ModifierDataFlow::GetUnknownType
+
+    .method public hidebysig static 
+        void WriteModReqType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+        IL_0006: stsfld class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType) Library.ModifierDataFlow::modReqType
+        IL_000b: ret
+    } // end of method C::WriteModReqType
+
+    .method public hidebysig static 
+        void WriteMultipleModReqType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+        IL_0006: stsfld class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType) modreq(Library.ModifierDataFlow/ModifierType) Library.ModifierDataFlow::multipleModReqType
+        IL_000b: ret
+    } // end of method Library.ModifierDataFlow::WriteMultipleModReqType
+
+    .method public hidebysig static 
+        void WriteModOptType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+        IL_0006: stsfld class [System.Runtime]System.Type modopt(Library.ModifierDataFlow/ModifierType) Library.ModifierDataFlow::modOptType
+        IL_000b: ret
+    } // end of method Library.ModifierDataFlow::WriteModOptType
+
+    .method public hidebysig static 
+        void WriteModReqModOptType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+        IL_0006: stsfld class [System.Runtime]System.Type modopt(Library.ModifierDataFlow/ModifierType) modreq(Library.ModifierDataFlow/ModifierType) Library.ModifierDataFlow::modReqModOptType
+        IL_000b: ret
+    } // end of method Library.ModifierDataFlow::WriteModReqModOptType
+
+    .method public hidebysig static 
+        void WriteModOptModReqType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+        IL_0006: stsfld class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType) modopt(Library.ModifierDataFlow/ModifierType) Library.ModifierDataFlow::modOptModReqType
+        IL_000b: ret
+    } // end of method Library.ModifierDataFlow::WriteModOptModReqType
+
+    .method public hidebysig static 
+        void WriteModReqArrayType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: ldc.i4.1
+        IL_0002: newarr [System.Runtime]System.Type
+        IL_0007: dup
+        IL_0008: ldc.i4.0
+        IL_0009: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+
+        IL_000e: stelem.ref
+        IL_000f: stsfld class [System.Runtime]System.Type[] modreq(Library.ModifierDataFlow/ModifierType) Library.ModifierDataFlow::modReqArrayType
+        IL_0014: ret
+    } // end of method Library.ModifierDataFlow::WriteModReqArrayType
+
+    .method public hidebysig static 
+        void WriteArrayModReqType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: ldc.i4.1
+        IL_0002: newarr [System.Runtime]System.Type
+        IL_0007: dup
+        IL_0008: ldc.i4.0
+        IL_0009: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+
+        IL_000e: stelem.ref
+        IL_000f: stsfld class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType)[] Library.ModifierDataFlow::arrayModReqType
+        IL_0014: ret
+    } // end of method Library.ModifierDataFlow::WriteArrayModReqType
+
+    .method public hidebysig static 
+        void WriteModReqArrayModReqType () cil managed 
+    {
+        .maxstack 8
+
+        IL_0000: nop
+        IL_0001: ldc.i4.1
+        IL_0002: newarr [System.Runtime]System.Type
+        IL_0007: dup
+        IL_0008: ldc.i4.0
+        IL_0009: call class [System.Runtime]System.Type Library.ModifierDataFlow::GetUnknownType()
+
+        IL_000e: stelem.ref
+        IL_000f: stsfld class [System.Runtime]System.Type modreq(Library.ModifierDataFlow/ModifierType)[] modreq(Library.ModifierDataFlow/ModifierType) Library.ModifierDataFlow::modReqArrayModReqType
+        IL_0014: ret
+    } // end of method Library.ModifierDataFlow::WriteModReqArrayModReqType
+
+} // end of class Library.ModifierDataFlow
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FieldDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FieldDataFlow.cs
@@ -4,13 +4,15 @@
 using System;
 using System.Diagnostics.CodeAnalysis;
 using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
 using Mono.Linker.Tests.Cases.Expectations.Helpers;
 
 namespace Mono.Linker.Tests.Cases.DataFlow
 {
 	// Note: this test's goal is to validate that the product correctly reports unrecognized patterns
 	//   - so the main validation is done by the ExpectedWarning attributes.
 	[SkipKeptItemsValidation]
+	[SetupCompileArgument ("/unsafe")]
 	[ExpectedNoWarnings]
 	public class FieldDataFlow
 	{
@@ -394,11 +396,21 @@ static void TestTypeGenericParameter ()
 				GenericField<Type>.field = GetUnknownType ();
 			}
 
+			[ExpectedWarning ("IL2097")]
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			unsafe static delegate*<void> functionPointer;
+
+			unsafe static void TestFunctionPointer ()
+			{
+				functionPointer = null;
+			}
+
 			public static void Test ()
 			{
 				TestUnsupportedType ();
 				StringRef.Test ();
 				TestTypeGenericParameter ();
+				TestFunctionPointer ();
 			}
 		}
 

diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ModifierDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ModifierDataFlow.cs
@@ -0,0 +1,66 @@
+// 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.Diagnostics.CodeAnalysis;
+using System.Reflection;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.DataFlow
+{
+	// Note: this test's goal is to validate that the product correctly reports unrecognized patterns
+	//   - so the main validation is done by the ExpectedWarning attributes.
+	[SkipKeptItemsValidation]
+	[SetupCompileArgument ("/unsafe")]
+	[Define ("IL_ASSEMBLY_AVAILABLE")]
+	[SetupCompileBefore ("library.dll", new[] { "Dependencies/ModifierDataFlow.il" })]
+	[ExpectedNoWarnings]
+	[LogContains ("IL2074: Library.ModifierDataFlow.WriteModReqType().*'Library.ModifierDataFlow.modReqType'.*GetUnknownType()", regexMatch: true)]
+	[LogContains ("IL2074: Library.ModifierDataFlow.WriteMultipleModReqType().*'Library.ModifierDataFlow.multipleModReqType'.*GetUnknownType()", regexMatch: true)]
+	[LogContains ("IL2074: Library.ModifierDataFlow.WriteModOptType().*'Library.ModifierDataFlow.modOptType'.*GetUnknownType()", regexMatch: true)]
+	[LogContains ("IL2074: Library.ModifierDataFlow.WriteModReqModOptType().*'Library.ModifierDataFlow.modReqModOptType'.*GetUnknownType()", regexMatch: true)]
+	[LogContains ("IL2074: Library.ModifierDataFlow.WriteModOptModReqType().*'Library.ModifierDataFlow.modOptModReqType'.*GetUnknownType()", regexMatch: true)]
+	[LogDoesNotContain ("IL2074")]
+	[LogContains ("IL2097:.*Library.ModifierDataFlow.arrayModReqType", regexMatch: true)]
+	[LogContains ("IL2097:.*Library.ModifierDataFlow.modReqArrayType", regexMatch: true)]
+	[LogContains ("IL2097:.*Library.ModifierDataFlow.modReqArrayModReqType", regexMatch: true)]
+	[LogDoesNotContain ("IL2097")]
+	public class ModifierDataFlow
+	{
+		[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)]
+		static volatile Type volatileType;
+
+		[ExpectedWarning ("IL2074", nameof (GetUnknownType), nameof (volatileType))]
+		static void WriteVolatileType ()
+		{
+			volatileType = GetUnknownType ();
+		}
+
+		static Type GetUnknownType () => null;
+
+		[ExpectedWarning ("IL2097")]
+		[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+		static volatile Type[] volatileTypeArray;
+
+		static void WriteVolatileTypeArray ()
+		{
+			volatileTypeArray = new Type[] { GetUnknownType () };
+		}
+
+		public static void Main ()
+		{
+			WriteVolatileType ();
+			WriteVolatileTypeArray ();
+#if IL_ASSEMBLY_AVAILABLE
+			Library.ModifierDataFlow.WriteModReqType ();
+			Library.ModifierDataFlow.WriteMultipleModReqType ();
+			Library.ModifierDataFlow.WriteModOptType ();
+			Library.ModifierDataFlow.WriteModReqModOptType ();
+			Library.ModifierDataFlow.WriteModOptModReqType ();
+			Library.ModifierDataFlow.WriteModReqArrayType ();
+			Library.ModifierDataFlow.WriteArrayModReqType ();
+#endif
+		}
+	}
+}
diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
@@ -78,6 +78,11 @@ protected static void ValidateTypeRefsHaveValidAssemblyRefs (AssemblyDefinition
 						Assert.IsNotNull (assemblyRef, $"Type reference '{typeRef.FullName}' has a reference to assembly '{typeRef.Scope.Name}' which is not a reference of '{linked.FullName}'");
 						continue;
 					}
+				case ModuleDefinition: {
+						// There should be a Module row for this assembly
+						Assert.AreEqual (linked.MainModule.Name, typeRef.Scope.Name, $"Type reference '{typeRef.FullName}' has a reference to module '{typeRef.Scope.Name}' which is not the module of '{linked.FullName}'");
+						continue;
+				}
 				default:
 					throw new NotImplementedException ($"Unexpected scope type '{typeRef.Scope.GetType ()}' for type reference '{typeRef.FullName}'");
 				}