[release/8.0] Implement support for `InlineArray` in the trimmer (#92107)
authorgithub-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fri, 15 Sep 2023 17:23:10 +0000 (10:23 -0700)
committerGitHub <noreply@github.com>
Fri, 15 Sep 2023 17:23:10 +0000 (10:23 -0700)
* Implement support for InlineArray in the trimmer

Types annotated with `InlineArray` need to preserve all of their fields, even if otherwise this would not be the case. It's possible to have a struct with `LayoutKind.Auto` and with `InlineArray` - trimmer would normally trim fields on such struct. This leads to corruption since the field is never accessed directly.

This changes the marking to preserve all fields on a type with `InlineArray` attribute just like we would for explicit layout struct and similar other types.

Adds tests to cover both the explicit usage of `InlineArray` attribute as well as the compiler generate usage via collection literals.

* Update src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InlineArrayDataflow.cs

Co-authored-by: Sven Boemer <sbomer@gmail.com>
---------

Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com>
Co-authored-by: Sven Boemer <sbomer@gmail.com>
src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestDatabase.cs
src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestSuites.cs
src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/TestCaseCompilationMetadataProvider.cs
src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs
src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/InlineArraysTests.cs [new file with mode: 0644]
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InlineArrayDataflow.cs [new file with mode: 0644]
src/tools/illink/test/Mono.Linker.Tests.Cases/InlineArrays/InlineArray.cs [new file with mode: 0644]
src/tools/illink/test/Mono.Linker.Tests/TestCases/TestDatabase.cs
src/tools/illink/test/Mono.Linker.Tests/TestCases/TestSuites.cs
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompilationMetadataProvider.cs

index 07147b1..31cd445 100644 (file)
@@ -29,6 +29,11 @@ namespace Mono.Linker.Tests.TestCases
                        return TestNamesBySuiteName ();
                }
 
+               public static IEnumerable<object[]> InlineArrays ()
+               {
+                       return TestNamesBySuiteName();
+               }
+
                public static IEnumerable<object[]> LinkXml()
                {
                        return TestNamesBySuiteName();
index 883582c..744fb23 100644 (file)
@@ -31,6 +31,13 @@ namespace Mono.Linker.Tests.TestCases
                }
 
                [Theory]
+               [MemberData(nameof(TestDatabase.InlineArrays), MemberType = typeof(TestDatabase))]
+               public void InlineArrays(string t)
+               {
+                       Run(t);
+               }
+
+               [Theory]
                [MemberData (nameof (TestDatabase.LinkXml), MemberType = typeof (TestDatabase))]
                public void LinkXml (string t)
                {
index e97a51a..6441776 100644 (file)
@@ -129,9 +129,11 @@ namespace Mono.Linker.Tests.TestCasesRunner
 
                                yield return Path.Combine (referenceDir, "mscorlib.dll");
                                yield return Path.Combine (referenceDir, "System.Collections.dll");
+                               yield return Path.Combine (referenceDir, "System.Collections.Immutable.dll");
                                yield return Path.Combine (referenceDir, "System.ComponentModel.TypeConverter.dll");
                                yield return Path.Combine (referenceDir, "System.Console.dll");
                                yield return Path.Combine (referenceDir, "System.Linq.Expressions.dll");
+                               yield return Path.Combine (referenceDir, "System.Memory.dll");
                                yield return Path.Combine (referenceDir, "System.ObjectModel.dll");
                                yield return Path.Combine (referenceDir, "System.Runtime.dll");
                                yield return Path.Combine (referenceDir, "System.Runtime.Extensions.dll");
index ded92af..52fe64b 100644 (file)
@@ -3317,11 +3317,23 @@ namespace Mono.Linker.Steps
                        if (type?.HasFields != true)
                                return;
 
-                       // keep fields for types with explicit layout and for enums
-                       if (!type.IsAutoLayout || type.IsEnum)
+                       // keep fields for types with explicit layout, for enums and for InlineArray types
+                       if (!type.IsAutoLayout || type.IsEnum || TypeIsInlineArrayType(type))
                                MarkFields (type, includeStatic: type.IsEnum, reason: new DependencyInfo (DependencyKind.MemberOfType, type));
                }
 
+               static bool TypeIsInlineArrayType(TypeDefinition type)
+               {
+                       if (!type.IsValueType)
+                               return false;
+
+                       foreach (var customAttribute in type.CustomAttributes)
+                               if (customAttribute.AttributeType.IsTypeOf ("System.Runtime.CompilerServices", "InlineArrayAttribute"))
+                                       return true;
+
+                       return false;
+               }
+
                protected virtual void MarkRequirementsForInstantiatedTypes (TypeDefinition type)
                {
                        if (Annotations.IsInstantiated (type))
index 3c5bb3c..29622dc 100644 (file)
@@ -150,6 +150,12 @@ namespace ILLink.RoslynAnalyzer.Tests
                }
 
                [Fact]
+               public Task InlineArrayDataflow ()
+               {
+                       return RunTest ();
+               }
+
+               [Fact]
                public Task MakeGenericDataFlow ()
                {
                        return RunTest ();
diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/InlineArraysTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/InlineArraysTests.cs
new file mode 100644 (file)
index 0000000..e8e1717
--- /dev/null
@@ -0,0 +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.Threading.Tasks;
+using Xunit;
+
+namespace ILLink.RoslynAnalyzer.Tests
+{
+       public sealed partial class InlineArraysTests : LinkerTestBase
+       {
+               protected override string TestSuiteName => "InlineArrays";
+
+               [Fact]
+               public Task InlineArray ()
+               {
+                       return RunTest (nameof (InlineArray));
+               }
+
+       }
+}
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InlineArrayDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InlineArrayDataflow.cs
new file mode 100644 (file)
index 0000000..7269107
--- /dev/null
@@ -0,0 +1,77 @@
+// 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.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.DataFlow
+{
+       [SkipKeptItemsValidation]
+       [ExpectedNoWarnings]
+       public class InlineArrayDataflow
+       {
+               public static void Main()
+               {
+                       AccessPrimitiveTypeArray ();
+                       AccessUnannotatedTypeArray ();
+                       AccessAnnotatedTypeArray ();
+               }
+
+               public int TestProperty { get; set; }
+
+               [InlineArray (5)]
+               struct PrimitiveTypeArray
+               {
+                       public BindingFlags value;
+               }
+
+               // This case will fallback to not understanding the binding flags and will end up marking all properties
+               static void AccessPrimitiveTypeArray ()
+               {
+                       PrimitiveTypeArray a = new PrimitiveTypeArray ();
+                       ref var item = ref a[1];
+                       item = BindingFlags.Public;
+
+                       typeof (InlineArrayDataflow).GetProperty (nameof (TestProperty), a[1]);
+               }
+
+               [InlineArray (5)]
+               struct UnannotatedTypeArray
+               {
+                       public Type value;
+               }
+
+               // Analyzer doesn't understand inline arrays currently - so it doesn't produce a warning here
+               [ExpectedWarning ("IL2065", "GetProperty", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
+               static void AccessUnannotatedTypeArray ()
+               {
+                       UnannotatedTypeArray a = new UnannotatedTypeArray ();
+                       ref var item = ref a[2];
+                       item = typeof (InlineArrayDataflow);
+
+                       a[2].GetProperty (nameof (TestProperty));
+               }
+
+               [InlineArray (5)]
+               struct AnnotatedTypeArray
+               {
+                       [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)]
+                       public Type value;
+               }
+
+               // Currently tracking of annotations on inline array values is not implemented
+               [ExpectedWarning("IL2065", "GetProperty", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
+               static void AccessAnnotatedTypeArray ()
+               {
+                       AnnotatedTypeArray a = new AnnotatedTypeArray ();
+                       ref var item = ref a[3];
+                       item = typeof (InlineArrayDataflow);
+
+                       a[3].GetProperty (nameof (TestProperty));
+               }
+       }
+}
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/InlineArrays/InlineArray.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/InlineArrays/InlineArray.cs
new file mode 100644 (file)
index 0000000..ce0fd7b
--- /dev/null
@@ -0,0 +1,86 @@
+// 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.Collections.Immutable;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.InlineArrays
+{
+       [ExpectedNoWarnings]
+       public class InlineArray
+       {
+               public static void Main()
+               {
+                       InlineArrayUsage.Test ();
+                       CollectionLiteralsOfArrays.Test ();
+               }
+
+               [Kept]
+               class InlineArrayUsage
+               {
+                       // NativeAOT will remove most of the struct type information as it's not needed
+                       // in the generated native code. Eventually we might come up with a better test infra to validate this.
+                       [Kept (By = Tool.Trimmer)]
+                       public struct StructWithFixedBuffer
+                       {
+                               [Kept (By = Tool.Trimmer)]
+                               public FixedBuffer Buffer;
+
+                               [Kept (By = Tool.Trimmer)]
+                               [KeptAttributeAttribute (typeof(InlineArrayAttribute), By = Tool.Trimmer)]
+                               [InlineArray (8)]
+                               public partial struct FixedBuffer
+                               {
+                                       [Kept (By = Tool.Trimmer)]
+                                       public int e0;
+                               }
+                       }
+
+                       [Kept (By = Tool.Trimmer)]
+                       public struct StructWithAutoLayoutBuffer
+                       {
+                               [Kept (By = Tool.Trimmer)]
+                               public AutoLayoutBuffer Buffer;
+
+                               [Kept (By = Tool.Trimmer)]
+                               [KeptAttributeAttribute (typeof (InlineArrayAttribute), By = Tool.Trimmer)]
+                               [InlineArray (8)]
+                               [StructLayout (LayoutKind.Auto)]
+                               public struct AutoLayoutBuffer
+                               {
+                                       [Kept (By = Tool.Trimmer)]
+                                       public int e0;
+                               }
+                       }
+
+                       [Kept]
+                       public static void Test ()
+                       {
+                               var s = new StructWithFixedBuffer ();
+                               s.Buffer[0] = 5;
+
+                               var sa = new StructWithAutoLayoutBuffer ();
+                               _ = sa.Buffer[1];
+                       }
+               }
+
+               [Kept]
+               [KeptMember (".cctor()")]
+               class CollectionLiteralsOfArrays
+               {
+                       [Kept]
+                       public static readonly ImmutableArray<string> ImmutableValues = ["one", "two"];
+                       [Kept]
+                       public static readonly string[] ArrayValues = ["one", "two"];
+
+                       [Kept]
+                       public static void Test()
+                       {
+                               _ = CollectionLiteralsOfArrays.ImmutableValues[0];
+                               _ = CollectionLiteralsOfArrays.ArrayValues[1];
+                       }
+               }
+       }
+}
index a4c0008..40740bc 100644 (file)
@@ -119,6 +119,11 @@ namespace Mono.Linker.Tests.TestCases
                        return NUnitCasesBySuiteName ("Inheritance.VirtualMethods");
                }
 
+               public static IEnumerable<TestCaseData> InlineArrayTests ()
+               {
+                       return NUnitCasesBySuiteName ("InlineArrays");
+               }
+
                public static IEnumerable<TestCaseData> InteropTests ()
                {
                        return NUnitCasesBySuiteName ("Interop");
index b26accc..fd25f5c 100644 (file)
@@ -142,6 +142,12 @@ namespace Mono.Linker.Tests.TestCases
                        Run (testCase);
                }
 
+               [TestCaseSource (typeof (TestDatabase), nameof (TestDatabase.InlineArrayTests))]
+               public void InlineArrayTests (TestCase testCase)
+               {
+                       Run (testCase);
+               }
+
                [TestCaseSource (typeof (TestDatabase), nameof (TestDatabase.InteropTests))]
                public void InteropTests (TestCase testCase)
                {
index 5109942..c26e833 100644 (file)
@@ -148,9 +148,11 @@ namespace Mono.Linker.Tests.TestCasesRunner
 
                                yield return Path.Combine (referenceDir, "mscorlib.dll");
                                yield return Path.Combine (referenceDir, "System.Collections.dll");
+                               yield return Path.Combine (referenceDir, "System.Collections.Immutable.dll");
                                yield return Path.Combine (referenceDir, "System.ComponentModel.TypeConverter.dll");
                                yield return Path.Combine (referenceDir, "System.Console.dll");
                                yield return Path.Combine (referenceDir, "System.Linq.Expressions.dll");
+                               yield return Path.Combine (referenceDir, "System.Memory.dll");
                                yield return Path.Combine (referenceDir, "System.ObjectModel.dll");
                                yield return Path.Combine (referenceDir, "System.Runtime.dll");
                                yield return Path.Combine (referenceDir, "System.Runtime.Extensions.dll");