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

Replace old ReflectionMethodBodyScanner with the illink-based one #594

Merged
merged 4 commits into from
Jan 27, 2021
Merged
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
1 change: 1 addition & 0 deletions src/coreclr/tools/Common/Compiler/Logger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,5 +126,6 @@ public static class MessageSubCategory
{
public const string None = "";
public const string TrimAnalysis = "Trim analysis";
public const string AotAnalysis = "AOT analysis";
}
}
4 changes: 2 additions & 2 deletions src/coreclr/tools/Common/Compiler/Logging/MessageContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ internal static MessageContainer CreateErrorMessage(string text, int code, strin
/// <returns>New MessageContainer of 'Warning' category</returns>
internal static MessageContainer? CreateWarningMessage(Logger context, string text, int code, MessageOrigin origin, string subcategory = MessageSubCategory.None)
{
if (!(code > 2000 && code <= 6000))
throw new ArgumentOutOfRangeException(nameof(code), $"The provided code '{code}' does not fall into the warning category, which is in the range of 2001 to 6000 (inclusive).");
//if (!(code > 2000 && code <= 6000))
// throw new ArgumentOutOfRangeException(nameof(code), $"The provided code '{code}' does not fall into the warning category, which is in the range of 2001 to 6000 (inclusive).");

return CreateWarningMessageContainer(context, text, code, origin, subcategory);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ enum IntrinsicId
Expression_Field,
Expression_Property,
Expression_New,
Enum_GetValues,
Marshal_SizeOf,
Activator_CreateInstance_Type,
Activator_CreateInstance_AssemblyName_TypeName,
Activator_CreateInstanceFrom,
Expand Down Expand Up @@ -406,6 +408,18 @@ static IntrinsicId GetIntrinsicIdForMethod(MethodDesc calledMethod)
&& calledMethod.Signature.Length == 1
=> IntrinsicId.Expression_New,

// static Array System.Enum.GetValues (Type)
"GetValues" when calledMethod.IsDeclaredOnType("System", "Enum")
&& calledMethod.HasParameterOfType(0, "System", "Type")
&& calledMethod.Signature.Length == 1
=> IntrinsicId.Enum_GetValues,

// static int System.Runtime.InteropServices.Marshal.SizeOf (Type)
"SizeOf" when calledMethod.IsDeclaredOnType("System.Runtime.InteropServices", "Marshal")
&& calledMethod.HasParameterOfType(0, "System", "Type")
&& calledMethod.Signature.Length == 1
=> IntrinsicId.Marshal_SizeOf,

// static System.Type.GetType (string)
// static System.Type.GetType (string, Boolean)
// static System.Type.GetType (string, Boolean, Boolean)
Expand Down Expand Up @@ -890,6 +904,64 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet
}
break;

//
// System.Enum
//
// static GetValues (Type)
//
case IntrinsicId.Enum_GetValues:
{
// Enum.GetValues returns System.Array, but it's the array of the enum type under the hood
// and people depend on this undocumented detail (could have returned enum of the underlying
// type instead).
//
// At least until we have shared enum code, this needs extra handling to get it right.
foreach (var value in methodParams[0].UniqueValues())
{
if (value is SystemTypeValue systemTypeValue
&& !systemTypeValue.TypeRepresented.IsGenericDefinition
&& !systemTypeValue.TypeRepresented.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true))
{
if (systemTypeValue.TypeRepresented.IsEnum)
{
_dependencies.Add(_factory.ConstructedTypeSymbol(systemTypeValue.TypeRepresented.MakeArrayType()), "Enum.GetValues");
}
}
else
{
_logger.LogWarning(Resources.Strings.IL9701, 9701, callingMethodBody, offset, MessageSubCategory.AotAnalysis);
}
}
}
break;

//
// System.Runtime.InteropServices.Marshal
//
// static SizeOf (Type)
//
case IntrinsicId.Marshal_SizeOf:
{
// We need the data to do struct marshalling.
foreach (var value in methodParams[0].UniqueValues())
{
if (value is SystemTypeValue systemTypeValue
&& !systemTypeValue.TypeRepresented.IsGenericDefinition
&& !systemTypeValue.TypeRepresented.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true))
{
if (systemTypeValue.TypeRepresented.IsDefType)
{
_dependencies.Add(_factory.StructMarshallingData((DefType)systemTypeValue.TypeRepresented), "Marshal.SizeOf");
}
}
else
{
_logger.LogWarning(Resources.Strings.IL9702, 9702, callingMethodBody, offset, MessageSubCategory.AotAnalysis);
}
}
}
break;

//
// System.Object
//
Expand Down Expand Up @@ -972,14 +1044,18 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet
{
bool found = ILCompiler.DependencyAnalysis.ReflectionMethodBodyScanner.ResolveType(knownStringValue.Contents, ((MetadataType)callingMethodDefinition.OwningType).Module,
callingMethodDefinition.Context,
out TypeDesc foundType, out _);
out TypeDesc foundType, out ModuleDesc referenceModule);
if (!found)
{
// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
reflectionContext.RecordHandledPattern();
}
else
{
// Also add module metadata in case this reference was through a type forward
if (_factory.MetadataManager.CanGenerateMetadata(referenceModule.GetGlobalModuleType()))
_dependencies.Add(_factory.ModuleMetadata(referenceModule), reflectionContext.MemberWithRequirements.ToString());

reflectionContext.RecordRecognizedPattern(() => _dependencies.Add(_factory.MaximallyConstructableType(foundType), "Type.GetType reference"));
methodReturnValue = MergePointValue.MergeValues(methodReturnValue, new SystemTypeValue(foundType));
}
Expand Down Expand Up @@ -1924,13 +2000,17 @@ void RequireDynamicallyAccessedMembers(ref ReflectionPatternContext reflectionCo
{
ModuleDesc callingModule = ((reflectionContext.Source as MethodDesc)?.OwningType as MetadataType)?.Module;

if (!ILCompiler.DependencyAnalysis.ReflectionMethodBodyScanner.ResolveType(knownStringValue.Contents, callingModule, reflectionContext.Source.Context, out TypeDesc foundType, out _))
if (!ILCompiler.DependencyAnalysis.ReflectionMethodBodyScanner.ResolveType(knownStringValue.Contents, callingModule, reflectionContext.Source.Context, out TypeDesc foundType, out ModuleDesc referenceModule))
{
// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
reflectionContext.RecordHandledPattern();
}
else
{
// Also add module metadata in case this reference was through a type forward
if (_factory.MetadataManager.CanGenerateMetadata(referenceModule.GetGlobalModuleType()))
_dependencies.Add(_factory.ModuleMetadata(referenceModule), reflectionContext.MemberWithRequirements.ToString());

MarkType(ref reflectionContext, foundType);
MarkTypeForDynamicallyAccessedMembers(ref reflectionContext, foundType, requiredMemberTypes);
}
Expand Down Expand Up @@ -2017,21 +2097,55 @@ void MarkType(ref ReflectionPatternContext reflectionContext, TypeDesc type)

void MarkMethod(ref ReflectionPatternContext reflectionContext, MethodDesc method)
{
if (method.HasInstantiation || method.OwningType.IsGenericDefinition || method.OwningType.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true))
// Code below assumes we need to specialize generic methods
Debug.Assert(method.IsMethodDefinition);

// If there's any genericness involved, try to create a fitting instantiation that would be usable at runtime.
// This is not a complete solution to the problem.
// If we ever decide that MakeGenericType/MakeGenericMethod should simply be considered unsafe, this code can be deleted
// and instantiations that are not fully closed can be ignored.
if (method.OwningType.IsGenericDefinition || method.OwningType.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true))
{
if (_logger.IsVerbose)
_logger.Writer.WriteLine($"Would mark {method} but it's generic");
TypeDesc owningType = method.OwningType.GetTypeDefinition();
Instantiation inst = ILCompiler.TypeExtensions.GetInstantiationThatMeetsConstraints(owningType.Instantiation, allowCanon: false);
if (inst.IsNull)
{
if (_logger.IsVerbose)
_logger.Writer.WriteLine($"Would mark {method} but can't get a good owning type");
return;
}

method = method.Context.GetMethodForInstantiatedType(
method.GetTypicalMethodDefinition(),
((MetadataType)owningType).MakeInstantiatedType(inst));
}

if (method.HasInstantiation)
{
Instantiation inst = ILCompiler.TypeExtensions.GetInstantiationThatMeetsConstraints(method.Instantiation, allowCanon: false);
if (inst.IsNull)
{
if (_logger.IsVerbose)
_logger.Writer.WriteLine($"Would mark {method} but can't get a good instantiation");
return;
}

method = method.MakeInstantiatedMethod(inst);
}
else if (!MetadataManager.IsMethodSupportedInReflectionInvoke(method))

string reason = reflectionContext.MemberWithRequirements.ToString();

// For the method to be actually usable with reflection, we need to add the constructed type.
_dependencies.Add(_factory.MaximallyConstructableType(method.OwningType), reason);

if (!MetadataManager.IsMethodSupportedInReflectionInvoke(method))
{
if (_logger.IsVerbose)
_logger.Writer.WriteLine($"Would mark {method} but it's not usable for reflection invoke");
// TODO: do we need to drop a MethodMetadata node into the dependencies here?
}
else
{
string reason = reflectionContext.MemberWithRequirements.ToString();

if (method.IsVirtual)
{
if (method.HasInstantiation)
Expand Down Expand Up @@ -2185,6 +2299,12 @@ public static class Strings
public const string IL2089 = "value stored in field '{0}' does not satisfy {3} requirements. The generic parameter '{1}' of '{2}' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.";
public const string IL2090 = "'this' argument does not satisfy {3} in call to '{0}'. The generic parameter '{1}' of '{2}' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.";
public const string IL2091 = "'{0}' generic argument does not satisfy {4} in '{1}'. The generic parameter '{2}' of '{3}' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.";

// Error codes > 6000 are reserved for custom steps and illink doesn't claim ownership of them

// TODO: these are all unique to NativeAOT - mono/linker repo is not aware this error code is used.
public const string IL9701 = "Enum.GetValues was called with an unknown enum type. It might not be possible to construct an array of this type at runtime.";
public const string IL9702 = "Marshal.SizeOf was called with an unknown type. It might not be possible to compute the marshalling size at runtime.";
}
}
}
Expand Down
Loading