Skip to content

Implements more advanced ILLink/ILCompiler analysis for Type.GetMember #94879

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.

Sign up for GitHub

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 20 commits into from
Closed

Implements more advanced ILLink/ILCompiler analysis for Type.GetMember #94879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ced1292
Implement masking based on MemberTypes
hamarb123 Nov 16, 2023
d0df383
Update HandleCallAction.cs
hamarb123 Nov 16, 2023
65a5a1c
Implement matching by name
hamarb123 Nov 16, 2023
74e87f2
Merge branch 'main' into main7
hamarb123 Dec 7, 2023
b15124a
Implement methods in other files
hamarb123 Dec 7, 2023
f9c437f
Merge branch 'main7' of https://github.com/hamarb123/runtime into main7
hamarb123 Dec 7, 2023
b3c8d5f
Fix compile errors
hamarb123 Dec 7, 2023
8439ea2
Fix some compile errors and spacing
hamarb123 Dec 7, 2023
21e41b0
Update HandleCallAction.cs
hamarb123 Dec 7, 2023
3093fe5
Update HandleCallAction.cs
hamarb123 Dec 7, 2023
1053805
Add some unit tests & fix some code
hamarb123 Dec 9, 2023
02b6c45
Fix typo & move ctors to top
hamarb123 Dec 9, 2023
40044cd
Update MemberUsedViaReflection.cs
hamarb123 Dec 9, 2023
0646bb1
Update MemberUsedViaReflection.cs
hamarb123 Dec 9, 2023
b665d99
Update some `[Kept]` attributes
hamarb123 Dec 10, 2023
dd6379b
Apply some feedback (incomplete)
hamarb123 Jan 17, 2024
ec45c87
Update HandleCallAction.cs
hamarb123 Jan 17, 2024
ac1066a
Merge branch 'main' into main7
hamarb123 Jan 17, 2024
4d8ccf3
Update HandleCallAction.cs
hamarb123 Jan 30, 2024
f8cfd86
Update HandleCallAction.cs
hamarb123 Jan 30, 2024
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
121 changes: 114 additions & 7 deletions src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,18 +367,24 @@ GenericParameterValue genericParam
// GetMember (String, MemberTypes, BindingFlags)
//
case IntrinsicId.Type_GetMember: {
if (instanceValue.IsEmpty ()) {
if (instanceValue.IsEmpty () || argumentValues[0].IsEmpty ()) {
returnValue = MultiValueLattice.Top;
break;
}

BindingFlags? bindingFlags;
MemberTypes? memberTypes;
if (calledMethod.HasMetadataParametersCount (1)) {
// Assume a default value for BindingFlags for methods that don't use BindingFlags as a parameter
bindingFlags = BindingFlags.Public | BindingFlags.Instance;
} else if (calledMethod.HasMetadataParametersCount (2) && calledMethod.HasParameterOfType ((ParameterIndex) 2, "System.Reflection.BindingFlags"))
// Assume a default value for MemberTypes for methods that don't use MemberTypes as a parameter
memberTypes = MemberTypes.All;
// Assume a default value for BindingFlags for methods that don't use BindingFlags as a parameter (Type.DefaultLookup)
bindingFlags = BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance;
Copy link
Member

@sbomer sbomer Dec 21, 2023

Choose a reason for hiding this comment

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

This changes the default behavior (looks correct to me). Could we make sure there are tests for this part of the change? So add a test that checks that static methods are kept when not passing binding flags.

Copy link
Contributor Author

@hamarb123 hamarb123 Dec 27, 2023

Choose a reason for hiding this comment

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

I will take a look through this next week some time (away currently).

} else if (calledMethod.HasMetadataParametersCount (2) && calledMethod.HasParameterOfType ((ParameterIndex) 2, "System.Reflection.BindingFlags")) {
// Assume a default value for MemberTypes for methods that don't use MemberTypes as a parameter
memberTypes = MemberTypes.All;
bindingFlags = GetBindingFlagsFromValue (argumentValues[1]);
else if (calledMethod.HasMetadataParametersCount (3) && calledMethod.HasParameterOfType ((ParameterIndex) 3, "System.Reflection.BindingFlags")) {
} else if (calledMethod.HasMetadataParametersCount (3) && calledMethod.HasParameterOfType ((ParameterIndex) 2, "System.Reflection.MemberTypes") && calledMethod.HasParameterOfType ((ParameterIndex) 3, "System.Reflection.BindingFlags")) {
memberTypes = GetMemberTypesFromValue (argumentValues[1]);
bindingFlags = GetBindingFlagsFromValue (argumentValues[2]);
} else // Non recognized intrinsic
throw new ArgumentException ($"Reflection call '{calledMethod.GetDisplayName ()}' inside '{GetContainingSymbolDisplayName ()}' is an unexpected intrinsic.");
Expand All @@ -394,13 +400,59 @@ GenericParameterValue genericParam
} else {
requiredMemberTypes = GetDynamicallyAccessedMemberTypesFromBindingFlagsForMembers (bindingFlags);
}
if (!MemberTypesAreUnsupported (memberTypes)) {
DynamicallyAccessedMemberTypes requiredMemberTypesMaskFromMemberTypes;
requiredMemberTypesMaskFromMemberTypes = GetDynamicallyAccessedMemberTypesFromMemberTypesForMembers (memberTypes);
requiredMemberTypes &= requiredMemberTypesMaskFromMemberTypes;
}

var targetValue = _annotations.GetMethodThisParameterValue (calledMethod, requiredMemberTypes);

// Go over all types we've seen
foreach (var value in instanceValue.AsEnumerable ()) {
// Mark based on bitfield requirements
_requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
if (value is SystemTypeValue systemTypeValue) {
foreach (var stringParam in argumentValues[0].AsEnumerable ()) {
if (stringParam is KnownStringValue stringValue && !BindingFlagsAreUnsupported (bindingFlags) && !MemberTypesAreUnsupported (memberTypes)) {
// determine if we've got a prefix (for example, abc* searches for anything starting with abc)
var isPrefix = stringValue.Contents.EndsWith("*");
if (isPrefix)
{
// Mark based on bitfield requirements
_requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
continue;
}
var name = stringValue.Contents;

// search for all the things we want by name
if (name == ".ctor" && (requiredMemberTypes & (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)) != 0) {
MarkConstructorsOnType (systemTypeValue.RepresentedType, bindingFlags, parameterCount: null);
}
if ((requiredMemberTypes & (DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents)) != 0) {
MarkEventsOnTypeHierarchy (systemTypeValue.RepresentedType, name, bindingFlags);
}
if ((requiredMemberTypes & (DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields)) != 0) {
MarkFieldsOnTypeHierarchy (systemTypeValue.RepresentedType, name, bindingFlags);
}
if ((requiredMemberTypes & (DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)) != 0) {
foreach (var methodValue in ProcessGetMethodByName (systemTypeValue.RepresentedType, name, bindingFlags)) /*mark method*/;
}
if ((requiredMemberTypes & (DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties)) != 0) {
MarkPropertiesOnTypeHierarchy (systemTypeValue.RepresentedType, name, bindingFlags);
}
if ((requiredMemberTypes & (DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes)) != 0) {
foreach (var nestedTypeValue in GetNestedTypesOnType (systemTypeValue.RepresentedType, name, bindingFlags)) {
MarkType (nestedTypeValue.RepresentedType);
}
}
} else if (stringParam is not NullValue) {
// Mark based on bitfield requirements
_requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
}
}
} else if (value is not NullValue) {
// Mark based on bitfield requirements
_requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
}
}
}
break;
Expand Down Expand Up @@ -1352,6 +1404,7 @@ private void ProcessCreateInstanceByName (MethodProxy calledMethod, IReadOnlyLis
}

internal static BindingFlags? GetBindingFlagsFromValue (in MultiValue parameter) => (BindingFlags?) parameter.AsConstInt ();
internal static MemberTypes? GetMemberTypesFromValue (in MultiValue parameter) => (MemberTypes?) parameter.AsConstInt ();

internal static bool BindingFlagsAreUnsupported (BindingFlags? bindingFlags)
{
Expand Down Expand Up @@ -1381,6 +1434,25 @@ internal static bool BindingFlagsAreUnsupported (BindingFlags? bindingFlags)
return (flags & ~(UnderstoodBindingFlags | IgnorableBindingFlags)) != 0;
}

internal static bool MemberTypesAreUnsupported (MemberTypes? memberTypes)
{
if (memberTypes == null)
return true;

// Member types we understand
const MemberTypes UnderstoodMemberTypes =
MemberTypes.Constructor |
MemberTypes.Event |
MemberTypes.Field |
MemberTypes.Method |
MemberTypes.Property |
MemberTypes.TypeInfo |
MemberTypes.NestedType;
Copy link
Member

@sbomer sbomer Dec 21, 2023

Choose a reason for hiding this comment

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

Could you also please add test coverage for the various member types (ensure that when MemberTypes are passed, we keep exactly those member types on a reflected type)?

Copy link
Contributor Author

@hamarb123 hamarb123 Jan 30, 2024

Choose a reason for hiding this comment

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

I haven't implemented the changes to the tests yet, will close these comments when I do. I want to get the current tests to not fail first, then I'll change them to be what the new code should expect.


MemberTypes flags = memberTypes.Value;
return (flags & ~UnderstoodMemberTypes) != 0;
}

internal static bool HasBindingFlag (BindingFlags? bindingFlags, BindingFlags? search) => bindingFlags != null && (bindingFlags & search) == search;

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForNestedTypes (BindingFlags? bindingFlags) =>
Expand Down Expand Up @@ -1421,6 +1493,41 @@ internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes
GetDynamicallyAccessedMemberTypesFromBindingFlagsForProperties (bindingFlags) |
GetDynamicallyAccessedMemberTypesFromBindingFlagsForNestedTypes (bindingFlags);

internal static bool HasMemberType (MemberTypes? memberTypes, MemberTypes? search) => memberTypes != null && (memberTypes & search) == search;

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromMemberTypesForNestedTypes (MemberTypes? memberTypes) =>
(HasMemberType (memberTypes, MemberTypes.TypeInfo) ? DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None) |
(HasMemberType (memberTypes, MemberTypes.NestedType) ? DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None) |
(MemberTypesAreUnsupported (memberTypes) ? DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None);

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromMemberTypesForConstructors (MemberTypes? memberTypes) =>
(HasMemberType (memberTypes, MemberTypes.Constructor) ? DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None) |
(MemberTypesAreUnsupported (memberTypes) ? DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None);

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromMemberTypesForMethods (MemberTypes? memberTypes) =>
(HasMemberType (memberTypes, MemberTypes.Method) ? DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None) |
(MemberTypesAreUnsupported (memberTypes) ? DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None);

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromMemberTypesForFields (MemberTypes? memberTypes) =>
(HasMemberType (memberTypes, MemberTypes.Field) ? DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None) |
(MemberTypesAreUnsupported (memberTypes) ? DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None);

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromMemberTypesForProperties (MemberTypes? memberTypes) =>
(HasMemberType (memberTypes, MemberTypes.Property) ? DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None) |
(MemberTypesAreUnsupported (memberTypes) ? DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None);

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromMemberTypesForEvents (MemberTypes? memberTypes) =>
(HasMemberType (memberTypes, MemberTypes.Event) ? DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None) |
(MemberTypesAreUnsupported (memberTypes) ? DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None);

internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromMemberTypesForMembers (MemberTypes? memberTypes) =>
GetDynamicallyAccessedMemberTypesFromMemberTypesForConstructors (memberTypes) |
GetDynamicallyAccessedMemberTypesFromMemberTypesForEvents (memberTypes) |
GetDynamicallyAccessedMemberTypesFromMemberTypesForFields (memberTypes) |
GetDynamicallyAccessedMemberTypesFromMemberTypesForMethods (memberTypes) |
GetDynamicallyAccessedMemberTypesFromMemberTypesForProperties (memberTypes) |
GetDynamicallyAccessedMemberTypesFromMemberTypesForNestedTypes (memberTypes);

/// <Summary>
/// Returns true if the method is a .ctor for System.Type or a type that derives from System.Type (i.e. fields and params of this type can have DynamicallyAccessedMembers annotations)
/// </Summary>
Expand Down
Loading