Skip to content

Leaf level AOT annotation for CoreLib library #66976

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ private static AttributeUsageAttribute InternalGetAttributeUsage(Type type)
SR.Format(SR.Format_AttributeUsage, type));
}

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a suppression - we're creating an instance of a reference type array (System.Attribute is a reference type and Type is expected to be deriving from Attribute or the cast would fail).

Array.CreateInstance with a reference type element is safe.

private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount) =>
elementType.ContainsGenericParameters ? new Attribute[elementCount] : (Attribute[])Array.CreateInstance(elementType, elementCount);
#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using static System.RuntimeTypeHandle;

namespace System.Collections.Generic
Expand All @@ -25,6 +26,8 @@ internal static class ComparerHelpers
/// The logic in this method is replicated in vm/compile.cpp to ensure that NGen saves the right instantiations,
/// and in vm/jitinterface.cpp so the jit can model the behavior of this method.
/// </remarks>
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not used with NativeAOT - src/coreclr/System.Private.Corelib are files specific to the JIT-based CoreCLR.

It would not be safe - NativeAOT approaches comparers differently and does some work in the compiler to support them.

I don't know how best to approach this - suppressing this runs a risk that if we ever move this code under src/libraries/System.Private.CoreLib and for whatever reason it gets activated under NativeAOT, we'll have a bug.

Maybe the suppression is the best we can do, but it gives me a bad feeling.

As an option to consider, we could also disable the analyzer for CoreCLR's CoreLib.

Justification = "MakeGenericType is safe in this method")]
internal static object CreateDefaultComparer(Type type)
{
Debug.Assert(type != null && type is RuntimeType);
Expand Down Expand Up @@ -59,6 +62,8 @@ internal static object CreateDefaultComparer(Type type)
/// Creates the default <see cref="Comparer{T}"/> for a nullable type.
/// </summary>
/// <param name="nullableType">The nullable type to create the default comparer for.</param>
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is safe in this method")]
private static object? TryCreateNullableComparer(RuntimeType nullableType)
{
Debug.Assert(nullableType != null);
Expand Down Expand Up @@ -113,6 +118,8 @@ internal static object CreateDefaultComparer(Type type)
/// <remarks>
/// The logic in this method is replicated in vm/compile.cpp to ensure that NGen saves the right instantiations.
/// </remarks>
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is safe in this method")]
internal static object CreateDefaultEqualityComparer(Type type)
{
Debug.Assert(type != null && type is RuntimeType);
Expand Down Expand Up @@ -157,6 +164,8 @@ internal static object CreateDefaultEqualityComparer(Type type)
/// Creates the default <see cref="EqualityComparer{T}"/> for a nullable type.
/// </summary>
/// <param name="nullableType">The nullable type to create the default equality comparer for.</param>
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is safe in this method")]
private static object? TryCreateNullableEqualityComparer(RuntimeType nullableType)
{
Debug.Assert(nullableType != null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public override bool ContainsGenericParameters
}

[RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")]
[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")]
public override MethodInfo MakeGenericMethod(params Type[] arguments)
{
throw new InvalidOperationException(SR.Format(SR.Arg_NotGenericMethodDefinition, this));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,13 @@ public override Type MakeByRefType()
return SymbolType.FormCompoundType(m_format + "&", m_baseType, 0)!;
}

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
public override Type MakeArrayType()
{
return SymbolType.FormCompoundType(m_format + "[]", m_baseType, 0)!;
}

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
public override Type MakeArrayType(int rank)
{
string s = GetRankString(rank);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ public void Bake(ModuleBuilder module, int token)
#region Public Static Methods
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Similar comments apply below too - the mention of trimming doesn't describe the reason well enough)

Suggested change
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
Justification = "First parameter has to be a TypeBuilder, but it's not possible to obtain a TypeBuilder without getting an AOT warning.")]

public static MethodInfo GetMethod(Type type, MethodInfo method)
{
if (type is not TypeBuilder && type is not TypeBuilderInstantiation)
Expand Down Expand Up @@ -90,6 +92,8 @@ public static MethodInfo GetMethod(Type type, MethodInfo method)

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
public static ConstructorInfo GetConstructor(Type type, ConstructorInfo constructor)
{
if (type is not TypeBuilder && type is not TypeBuilderInstantiation)
Expand Down Expand Up @@ -1461,6 +1465,8 @@ public ConstructorBuilder DefineDefaultConstructor(MethodAttributes attributes)
Justification = "MakeGenericType is only called on a TypeBuilderInstantiation which is not subject to trimming")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "GetConstructor is only called on a TypeBuilderInstantiation which is not subject to trimming")]
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")]
private ConstructorBuilder DefineDefaultConstructorNoLock(MethodAttributes attributes)
{
ConstructorBuilder constBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,12 @@ public override Type MakeByRefType()
{
return SymbolType.FormCompoundType("&", this, 0)!;
}
[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
public override Type MakeArrayType()
{
return SymbolType.FormCompoundType("[]", this, 0)!;
}
[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
public override Type MakeArrayType(int rank)
{
if (rank <= 0)
Expand All @@ -109,6 +111,11 @@ public override Type MakeArrayType(int rank)
"Currently this is not supported by linker. Once it is supported the outercall (Type.MakeGenericType)" +
"will validate that the types fullfill the necessary requirements of annotations on type parameters." +
"As such the actual internals of the implementation are not interesting.")]
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Justification = "The entire TypeBuilderInstantiation is serving the MakeGenericType implementation. " +
"Currently this is not supported by linker. Once it is supported the outercall (Type.MakeGenericType)" +
"will validate that the types fullfill the necessary requirements of annotations on type parameters." +
"As such the actual internals of the implementation are not interesting.")]
private Type Substitute(Type[] substitutes)
{
Type[] inst = GetGenericArguments();
Expand Down Expand Up @@ -251,6 +258,7 @@ public override bool ContainsGenericParameters
public override Type GetGenericTypeDefinition() { return m_type; }

[RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")]
[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")]
public override Type MakeGenericType(params Type[] inst) { throw new InvalidOperationException(SR.Format(SR.Arg_NotGenericTypeDefinition, this)); }
public override bool IsAssignableFrom([NotNullWhen(true)] Type? c) { throw new NotSupportedException(); }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public override object Invoke(object? obj, BindingFlags invokeAttr, Binder? bind
public override bool IsGenericMethodDefinition => m_method.IsGenericMethodDefinition;
public override bool ContainsGenericParameters => m_method.ContainsGenericParameters;

[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")]
[RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")]
public override MethodInfo MakeGenericMethod(params Type[] typeArgs)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ private static RuntimeType ResolveType(RuntimeModule scope, string typeName)
}
#endregion

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as ComparerHelpers applies - this is not used for NativeAOT and the compiler otherwise ensures custom attribute blobs are constructable.

internal CustomAttributeTypedArgument(RuntimeModule scope, CustomAttributeEncodedArgument encodedArg)
{
CustomAttributeEncoding encodedType = encodedArg.CustomAttributeType.EncodedType;
Expand Down Expand Up @@ -1433,6 +1434,7 @@ internal static AttributeUsageAttribute GetAttributeUsage(RuntimeType decoratedA
return attributeUsageAttribute ?? AttributeUsageAttribute.Default;
}

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be suppressed because the type used in CreateInstance is a reference type (we handle IsValueType with an object array).

internal static object[] CreateAttributeArrayHelper(RuntimeType caType, int elementCount)
{
bool useAttributeArray = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ private Delegate CreateDelegateInternal(Type delegateType!!, object? firstArgume

#region Generics
[RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")]
[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")]
public override MethodInfo MakeGenericMethod(params Type[] methodInstantiation!!)
{
RuntimeType[] methodInstantionRuntimeType = new RuntimeType[methodInstantiation.Length];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ internal RuntimeType GetTypeChecked()
internal static extern bool IsInstanceOfType(RuntimeType type, [NotNullWhen(true)] object? o);

[RequiresUnreferencedCode("MakeGenericType cannot be statically analyzed. It's not possible to guarantee the availability of requirements of the generic type.")]
[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")]
internal static Type GetTypeHelper(Type typeStart, Type[]? genericArgs, IntPtr pModifiers, int cModifiers)
{
Type type = typeStart;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,10 @@ private void AddSpecialInterface(
Justification = "Calls to GetInterfaces technically require all interfaces on ReflectedType" +
"But this is not a public API to enumerate reflection items, all the public APIs which do that" +
"should be annotated accordingly.")]
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would not be safe (and the justification is off) - this code is not active for NativeAOT. Similar comments to the other place applies.

I really wonder if it would be better to disable the analyzer for CoreCLR's corelib.

Justification = "Calls to GetInterfaces technically require all interfaces on ReflectedType" +
"But this is not a public API to enumerate reflection items, all the public APIs which do that" +
"should be annotated accordingly.")]
private RuntimeType[] PopulateInterfaces(Filter filter)
{
ListBuilder<RuntimeType> list = default;
Expand Down Expand Up @@ -1601,6 +1605,7 @@ internal TypeCode TypeCode
return m_defaultMemberName;
}

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
internal object[] GetEmptyArray() => _emptyArray ??= (object[])Array.CreateInstance(m_runtimeType, 0);
#endregion

Expand Down Expand Up @@ -3336,6 +3341,7 @@ public override Type[] GetGenericArguments()
}

[RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")]
[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")]
public override Type MakeGenericType(Type[] instantiation!!)
{
if (!IsGenericTypeDefinition)
Expand Down Expand Up @@ -3438,8 +3444,10 @@ public override Type[] GetGenericParameterConstraints()

public override Type MakeByRefType() => new RuntimeTypeHandle(this).MakeByRef();

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
public override Type MakeArrayType() => new RuntimeTypeHandle(this).MakeSZArray();

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
public override Type MakeArrayType(int rank)
{
if (rank <= 0)
Expand Down Expand Up @@ -3860,6 +3868,7 @@ private extern object InvokeDispMethod(
return ret;
}

[RequiresDynamicCode("The code for an array of the specified type might not be available.")]
private static void WrapArgsForInvokeCall(object[] aArgs, int[] aArgsWrapperTypes)
{
int cArgs = aArgs.Length;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;

namespace System.StubHelpers
{
Expand Down Expand Up @@ -1045,6 +1046,7 @@ internal unsafe void ConvertToManaged(object pManagedHome, IntPtr pNativeHome)
}
}

[RequiresDynamicCode("Marshalling code for the object might not be available")]
internal void ClearNative(IntPtr pNativeHome)
{
if (pNativeHome != IntPtr.Zero)
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Private.CoreLib/src/System/Enum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ public static Type GetUnderlyingType(Type enumType!!) =>
enumType.GetEnumUnderlyingType();

#if !CORERT
[RequiresDynamicCode("It might not be possible to create an array of the enum type at runtime. Use the GetValues<TEnum> overload instead.")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeAOT of this is AOT friendly - we don't want to bubble this out like this because the only form factor that currently cares is safe:

public static TEnum[] GetValues<TEnum>() where TEnum : struct, Enum
{
Array values = GetEnumInfo(typeof(TEnum)).ValuesAsUnderlyingType;
TEnum[] result = new TEnum[values.Length];
Array.Copy(values, result, values.Length);
return result;
}

public static TEnum[] GetValues<TEnum>() where TEnum : struct, Enum =>
(TEnum[])GetValues(typeof(TEnum));
#endif
Expand Down