Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable basic Kept validation in NativeAOT running linker tests #78828

Merged
merged 11 commits into from
Nov 30, 2022
12 changes: 4 additions & 8 deletions src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ public static string GetDisplayName(this MethodDesc method)
sb.Append(method.OwningType.GetDisplayName());
sb.Append('.');

if (method.IsConstructor)
if (method.IsConstructor && method.OwningType is DefType defType)
{
sb.Append(method.OwningType.GetDisplayNameWithoutNamespace());
sb.Append(defType.Name);
}
#if !READYTORUN
else if (method.GetPropertyForAccessor() is PropertyPseudoDesc property)
Expand Down Expand Up @@ -238,12 +238,8 @@ protected override Unit AppendNameForNamespaceType(StringBuilder sb, DefType typ

protected override Unit AppendNameForNestedType(StringBuilder sb, DefType nestedType, DefType containingType, FormatOptions options)
{
if ((options & FormatOptions.NamespaceQualify) != 0)
{
AppendName(sb, containingType, options);
sb.Append('.');
}

AppendName(sb, containingType, options);
sb.Append('.');
sb.Append(nestedType.Name);

return default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,7 @@ public GenericParameterOrigin(GenericParameterDesc genericParam)
{
GenericParameter = genericParam;
}

public string GetDisplayName() => GenericParameter.GetDisplayName();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto

protected override string GetName(NodeFactory factory)
{
return "Dataflow analysis for " + factory.NameMangler.GetMangledMethodName(_methodIL.OwningMethod).ToString();
return "Dataflow analysis for " + _methodIL.OwningMethod.ToString();
}

public override bool InterestingForDynamicDependencyAnalysis => false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ protected override void GetMetadataDependenciesDueToReflectability(ref Dependenc
bool fullyRoot;
string reason;

// https://github.com/dotnet/runtime/issues/78752
// Compat with https://github.com/dotnet/linker/issues/1541 IL Linker bug:
// Asking to root an assembly with entrypoint will not actually root things in the assembly.
// We need to emulate this because the SDK injects a root for the entrypoint assembly right now
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.CommandLine.Help;
using System.CommandLine.Parsing;
using System.IO;
using System.Runtime.InteropServices;

using Internal.TypeSystem;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,11 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions
[AttributeUsage (AttributeTargets.All, Inherited = false)]
public class KeptAttribute : BaseExpectedLinkedBehaviorAttribute
{
/// <summary>
/// By default the target should be kept by all platforms
/// This property can override that by setting only the platforms
/// which are expected to keep the target.
/// </summary>
public ProducedBy By { get; set; } = ProducedBy.TrimmerAnalyzerAndNativeAot;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ private static void RequireCombinationOnString (
{
}

[Kept]
// https://github.com/dotnet/runtime/issues/72833
// NativeAOT doesn't implement full type name parser yet
[Kept (By = ProducedBy.Trimmer)]
class FromStringConstantWithGenericInner
{
}
Expand All @@ -141,41 +143,53 @@ class FromStringConstantWithGeneric<T>
public T GetValue () { return default (T); }
}

[Kept]
// https://github.com/dotnet/runtime/issues/72833
// NativeAOT doesn't implement full type name parser yet
[Kept (By = ProducedBy.Trimmer)]
class FromStringConstantWithGenericInnerInner
{
[Kept]
[Kept (By = ProducedBy.Trimmer)]
public void Method ()
{
}

int unusedField;
}

[Kept]
[Kept (By = ProducedBy.Trimmer)]
class FromStringConstantWithGenericInnerOne<
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute), By = ProducedBy.Trimmer)]
T>
{
}

[Kept]
// https://github.com/dotnet/runtime/issues/72833
// NativeAOT doesn't implement full type name parser yet
[Kept (By = ProducedBy.Trimmer)]
class FromStringConstantWithGenericInnerTwo
{
void UnusedMethod ()
{
}
}

[Kept]
// https://github.com/dotnet/runtime/issues/72833
// NativeAOT doesn't implement full type name parser yet
[Kept (By = ProducedBy.Trimmer)]
class FromStringConstantWitGenericInnerMultiDimArray
{
}

// https://github.com/dotnet/runtime/issues/72833
// NativeAOT actually preserves this, but for a slightly wrong reason - it completely ignores the array notations
[Kept]
[KeptMember (".ctor()", By = ProducedBy.NativeAot)]
class FromStringConstantWithMultiDimArray
{
// https://github.com/dotnet/runtime/issues/72833
// NativeAOT actually preserves this, but for a slightly wrong reason - it completely ignores the array notations
[Kept (By = ProducedBy.NativeAot)]
public void UnusedMethod () { }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
Expand All @@ -24,9 +25,12 @@ public static void Main ()
TestArrayTypeGetType ();
TestArrayCreateInstanceByName ();
TestArrayInAttributeParameter ();
TestArrayInAttributeParameter_ViaReflection ();
}

[Kept]
// NativeAOT: No need to preserve the element type if it's never instantiated
// There will be a reflection record about it, but we don't validate that yet
[Kept (By = ProducedBy.Trimmer)]
class ArrayElementType
{
public ArrayElementType () { }
Expand Down Expand Up @@ -54,7 +58,9 @@ static void RequirePublicMethodsOnArrayOfGeneric<T> ()
RequirePublicMethods (typeof (T[]));
}

[Kept]
// NativeAOT: No need to preserve the element type if it's never instantiated
// There will be a reflection record about it, but we don't validate that yet
[Kept (By = ProducedBy.Trimmer)]
class ArrayElementInGenericType
{
public ArrayElementInGenericType () { }
Expand Down Expand Up @@ -91,7 +97,9 @@ static void RequirePublicMethodsOnArrayOfGenericParameter<T> ()
_ = new RequirePublicMethodsGeneric<T[]> ();
}

[Kept]
// NativeAOT: No need to preserve the element type if it's never instantiated
// There will be a reflection record about it, but we don't validate that yet
[Kept (By = ProducedBy.Trimmer)]
sealed class ArrayGetTypeFromMethodParamElement
{
// This method should not be marked, instead Array.* should be marked
Expand All @@ -110,7 +118,9 @@ static void TestArrayGetTypeFromMethodParam ()
TestArrayGetTypeFromMethodParamHelper (null);
}

[Kept]
// NativeAOT: No need to preserve the element type if it's never instantiated
// There will be a reflection record about it, but we don't validate that yet
[Kept (By = ProducedBy.Trimmer)]
sealed class ArrayGetTypeFromFieldElement
{
// This method should not be marked, instead Array.* should be marked
Expand All @@ -126,9 +136,15 @@ static void TestArrayGetTypeFromField ()
RequirePublicMethods (_arrayGetTypeFromField.GetType ());
}


// https://github.com/dotnet/runtime/issues/72833
// NativeAOT doesn't implement full type name parser yet - it ignores the [] and thus sees this as a direct type reference
[Kept]
sealed class ArrayTypeGetTypeElement
{
// https://github.com/dotnet/runtime/issues/72833
// NativeAOT doesn't implement full type name parser yet - it ignores the [] and thus sees this as a direct type reference
[Kept (By = ProducedBy.NativeAot)]
// This method should not be marked, instead Array.* should be marked
public void PublicMethod () { }
}
Expand All @@ -139,11 +155,13 @@ static void TestArrayTypeGetType ()
RequirePublicMethods (Type.GetType ("Mono.Linker.Tests.Cases.DataFlow.ComplexTypeHandling+ArrayTypeGetTypeElement[]"));
}

// Technically there's no reason to mark this type since it's only used as an array element type and CreateInstance
// ILLink: Technically there's no reason to mark this type since it's only used as an array element type and CreateInstance
// doesn't work on arrays, but the currently implementation will preserve it anyway due to how it processes
// string -> Type resolution. This will only impact code which would have failed at runtime, so very unlikely to
// actually occur in real apps (and even if it does happen, it just increases size, doesn't break behavior).
[Kept]
// NativeAOT: https://github.com/dotnet/runtime/issues/72833
// NativeAOT doesn't implement full type name parser yet - it ignores the [] and thus sees this as a direct type reference
[Kept (By = ProducedBy.Trimmer)]
class ArrayCreateInstanceByNameElement
{
public ArrayCreateInstanceByNameElement ()
Expand All @@ -157,20 +175,46 @@ static void TestArrayCreateInstanceByName ()
Activator.CreateInstance ("test", "Mono.Linker.Tests.Cases.DataFlow.ComplexTypeHandling+ArrayCreateInstanceByNameElement[]");
}

[Kept]
// NativeAOT doesn't keep attributes on non-reflectable methods
[Kept (By = ProducedBy.Trimmer)]
class ArrayInAttributeParamElement
{
// This method should not be marked, instead Array.* should be marked
public void PublicMethod () { }
}

[Kept]
[KeptAttributeAttribute (typeof (RequiresPublicMethodAttribute))]
// NativeAOT doesn't keep attributes on non-reflectable methods
[KeptAttributeAttribute (typeof (RequiresPublicMethodAttribute), By = ProducedBy.Trimmer)]
[RequiresPublicMethod (typeof (ArrayInAttributeParamElement[]))]
static void TestArrayInAttributeParameter ()
{
}

// The usage of a type in attribute parameter is not enough to create NativeAOT EEType
// which is what the test infra looks for right now, so the type is not kept.
// There should be a reflection record of the type though (we just don't validate that yet).
[Kept (By = ProducedBy.Trimmer)]
class ArrayInAttributeParamElement_ViaReflection
{
// This method should not be marked, instead Array.* should be marked
public void PublicMethod () { }
}

[Kept]
[KeptAttributeAttribute (typeof (RequiresPublicMethodAttribute))]
[RequiresPublicMethod (typeof (ArrayInAttributeParamElement_ViaReflection[]))]
static void TestArrayInAttributeParameter_ViaReflection_Inner ()
{
}

[Kept]
static void TestArrayInAttributeParameter_ViaReflection ()
{
typeof (ComplexTypeHandling)
.GetMethod (nameof (TestArrayInAttributeParameter_ViaReflection_Inner), BindingFlags.NonPublic)
.Invoke (null, new object[] { });
}

[Kept]
private static void RequirePublicMethods (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Globalization;
using System.Reflection;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Helpers;

namespace Mono.Linker.Tests.Cases.DataFlow
{
Expand Down Expand Up @@ -117,8 +118,10 @@ class MyReflect : IReflect
public object InvokeMember (string name, BindingFlags invokeAttr, Binder binder, object target, object[] args, ParameterModifier[] modifiers, CultureInfo culture, string[] namedParameters) => throw new NotImplementedException ();
}

[Kept]
[KeptBaseType (typeof (MyReflect))]
// NativeAOT: Doesn't preserve this type because there's no need. The type itself is never instantiated
// there's only a field of that type and accessed to that field can be made without knowing it's type (just memory address access)
[Kept (By = ProducedBy.Trimmer)]
[KeptBaseType (typeof (MyReflect), By = ProducedBy.Trimmer)]
class MyReflectDerived : MyReflect
{
}
Expand Down Expand Up @@ -167,7 +170,15 @@ class TestType
[Kept]
public static void Test ()
{
new MyReflectOverType (typeof (TestType)).GetFields (BindingFlags.Instance | BindingFlags.Public);
var i = new MyReflectOverType (typeof (TestType));
i.GetFields (BindingFlags.Instance | BindingFlags.Public);

#if NATIVEAOT
// In Native AOT the test infra doesn't setup the compiler in a way where it will force preserve
// all external types. Like here, it will actually track usage of methods on IReflect
// and remove any which are not used. We don't want that for this test.
typeof (IReflect).RequiresAll ();
#endif
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ namespace Mono.Linker.Tests.Cases.DataFlow
[KeptTypeInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType/PrivateNestedType")]
[KeptMemberInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType/PrivateNestedType", new string[] { "PrivateMethod()" })]

[KeptMember (".ctor()")]
// NativeAOT: https://github.com/dotnet/runtime/issues/78752
// Once the above issue is fixed the ctor should be preserved even in NativeAOT
[KeptMember (".ctor()", By = ProducedBy.Trimmer)]
public class MemberTypesAllOnCopyAssembly
{
public static void Main ()
Expand Down
Loading