Support Type.GetMember usage analysis properly with IL3050 warning (NAOT)

[hamarb123]

Description

Using the Type.GetMember function gives warnings that are not necessary when AOT analysis is enabled.

Reproduction Steps

Enable AOT compatible analysis: <IsAotCompatible>true</IsAotCompatible>.
Write code like so:

void X(Type t)
{
    t.GetMember("SomeMethod", MemberTypes.Method, BindingFlags.Public | BindingFlags.Static | BindingFlags.NonPublic);
}

Expected behavior

All that should be necessary to appease the analyser is:

[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)]

Actual behavior

Instead I get the following:

Regression?

No response

Known Workarounds

Ignore the warning.

Configuration

.NET SDK 8.0.0
VS 17.8.0
Windows 10.0.19045 x64
I don't see why it would be specific to this configuration

Other information

This was likely just overlooked when implementing the analyser. I've only discovered it now since I'm only now adding AOT support.

[ghost]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Using the Type.GetMember function gives warnings that are not necessary when AOT analysis is enabled.

Reproduction Steps

Enable AOT compatible analysis: <IsAotCompatible>true</IsAotCompatible>.
Write code like so:

type.GetMember("SomeMethod", MemberTypes.Method, BindingFlags.Public | BindingFlags.Static | BindingFlags.NonPublic);

Expected behavior

All that should be necessary to appease the analyser is:

[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)]

Actual behavior

Instead I get the following:

Regression?

No response

Known Workarounds

Ignore the warning.

Configuration

.NET SDK 8.0.0
VS 17.8.0
Windows 10.0.19045 x64
I don't see why it would be specific to this configuration

Other information

This was likely just looked over when implementing the analyser. I've only discovered it now since I'm only now adding AOT support.

Author:	hamarb123
Assignees:	-
Labels:	untriaged, area-NativeAOT-coreclr
Milestone:	-

[hamarb123]

I'd be happy to make a PR to fix this btw - if someone could point me to the code for this analyser, and people are happy for me to do it.

[agocke]

I think you're confusing GetMethod with GetMember. You're correct that GetMethod only requires PublicMethods and PrivateMethods, but GetMember requires DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods | DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields | DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes | DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties | DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents

[hamarb123]

@agocke no I'm not, I specified MemberTypes.Method in the code, therefore I don't see why I'd need anything other than methods.

[MichalStrehovsky]

We'd need to extend intrinsic handling of this method in the Roslyn analyzer and in ILLinker/NativeAOT compiler. Searching the codebase for Type_GetMember should get you close enough. Looks like we already look at BindingFlags but that doesn't help much.

Known Workarounds

Ignore the warning.

The warning should be suppressible and this might be safe to suppress (unless there's some obscure corner case behavior difference between GetMember(MemberTypes.Method) and GetMethod, which we'd need to investigate when the intrinsic is introduced).

[ghost]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Using the Type.GetMember function gives warnings that are not necessary when AOT analysis is enabled.

Reproduction Steps

Enable AOT compatible analysis: <IsAotCompatible>true</IsAotCompatible>.
Write code like so:

void X(Type t)
{
    t.GetMember("SomeMethod", MemberTypes.Method, BindingFlags.Public | BindingFlags.Static | BindingFlags.NonPublic);
}

Expected behavior

All that should be necessary to appease the analyser is:

[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)]

Actual behavior

Instead I get the following:

Regression?

No response

Known Workarounds

Ignore the warning.

Configuration

.NET SDK 8.0.0
VS 17.8.0
Windows 10.0.19045 x64
I don't see why it would be specific to this configuration

Other information

This was likely just overlooked when implementing the analyser. I've only discovered it now since I'm only now adding AOT support.

Author:	hamarb123
Assignees:	-
Labels:	untriaged, area-Tools-ILLink
Milestone:	-

[vitek-karas]

The fix would be here:

runtime/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs

Line 382 in 18cac43

bindingFlags = GetBindingFlagsFromValue (argumentValues[2]);

The value of the new enum should already be tracked as an integer by dataflow, so I don't think it would need changes to the dataflow itself.

Note that this code is shared by all 3 tools (ILLink, ILCompiler, the analyzer). Would need some more logic to figure out the correct member type masks but probably not too difficult.

Technically we can do even better and basically map this onto the handling of GetMethod - which has the ability to preserve only a single method, if the name of the method is a constant. But that's maybe a next level optimization on top.

I agree with Michal that we should investigate if there are behavioral differences between GetMember(.. Methods ...) and GetMethod().

[agocke]

Yup, my mistake, missed the binding flags. It would be cool to support this properly

[hamarb123]

I agree with Michal that we should investigate if there are behavioral differences between GetMember(.. Methods ...) and GetMethod().

Both GetMember and GetMethod begin to share the same implementation by this point.

runtime/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.BindingFlags.cs

Line 67 in 2e912a3

return Query<MethodInfo>(MethodPolicies.Instance, name, bindingAttr).Disambiguate();

runtime/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.GetMember.cs

Line 106 in 2e912a3

queryResult = Query<M>(policies, optionalName, bindingAttr, optionalPredicate);

The only differences I see up to this point is that GetMember allows a pattern with * at the end to specify anything starting with the prior characters (e.g., abc* means anything starting with abc). That seems to be the only difference to me.