Skip to content

Commit

Permalink
Implement support for InlineArray in the trimmer (dotnet#92060)
Browse files Browse the repository at this point in the history
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.

Co-authored-by: Sven Boemer <sbomer@gmail.com>
  • Loading branch information
vitek-karas and sbomer committed Sep 15, 2023
1 parent a6c64c3 commit d40f90c
Show file tree
Hide file tree
Showing 11 changed files with 230 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ public static IEnumerable<object[]> Generics ()
return TestNamesBySuiteName ();
}

public static IEnumerable<object[]> InlineArrays ()
{
return TestNamesBySuiteName();
}

public static IEnumerable<object[]> LinkXml()
{
return TestNamesBySuiteName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ public void Generics (string t)
Run (t);
}

[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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,11 @@ public virtual IEnumerable<string> GetCommonReferencedAssemblies (NPath workingD

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");
Expand Down
16 changes: 14 additions & 2 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3318,11 +3318,23 @@ void MarkImplicitlyUsedFields (TypeDefinition type)
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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ public Task GenericParameterDataFlow ()
return RunTest (nameof (GenericParameterDataFlow));
}

[Fact]
public Task InlineArrayDataflow ()
{
return RunTest ();
}

[Fact]
public Task MakeGenericDataFlow ()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}

}
}
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
Original file line number Diff line number Diff line change
@@ -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];
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ public static IEnumerable<TestCaseData> InheritanceVirtualMethodsTests ()
return NUnitCasesBySuiteName ("Inheritance.VirtualMethods");
}

public static IEnumerable<TestCaseData> InlineArrayTests ()
{
return NUnitCasesBySuiteName ("InlineArrays");
}

public static IEnumerable<TestCaseData> InteropTests ()
{
return NUnitCasesBySuiteName ("Interop");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ public void InheritanceVirtualMethodsTests (TestCase testCase)
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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,11 @@ public virtual IEnumerable<string> GetCommonReferencedAssemblies (NPath workingD

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");
Expand Down

0 comments on commit d40f90c

Please sign in to comment.