Non constant BindingFlags causes ILLinker to trim required methods

[eerhardt]

When a non-public method is called through Reflection, but the BindingFlags parameter is not a constant, the ILLinker won't preserve the called method, and it won't log a warning. Thus the user thinks their app will work successfully, but the app doesn't since the called method is trimmed.

This scenario was found in System.Private.Xml and is one of the underlying causes of issue dotnet/runtime#41389.

https://github.com/dotnet/runtime/blob/819a3cc69e2bf606787a44f9f107ebbbed0fcb11/src/libraries/System.Private.Xml/src/System/Xml/Serialization/CodeGenerator.cs#L24-L28

Example

Fully trim the following application:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

  <Target Name="EnsureAllAssembliesAreLinked"
        BeforeTargets="PrepareForILLink">
    <ItemGroup>
      <ManagedAssemblyToLink>
        <TrimMode>link</TrimMode>
      </ManagedAssemblyToLink>

      <!-- Pass the app assembly as a root -->
      <TrimmerRootAssembly Include="@(IntermediateAssembly)" />
    </ItemGroup>
  </Target>
</Project>

    internal class CodeGenerator
    {
        internal static BindingFlags InstanceBindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic;
    }

    public abstract class XmlSerializationWriter
    {
        protected void WriteStartDocument()
        {
        }
    }

    internal class Program
    {
        public static void Main()
        {
            MethodInfo XmlSerializationWriter_WriteStartDocument = typeof(XmlSerializationWriter).GetMethod(
                "WriteStartDocument",
                CodeGenerator.InstanceBindingFlags,
                null,
                Array.Empty<Type>(),
                null
                )!;
            Console.WriteLine(XmlSerializationWriter_WriteStartDocument.Name);
        }
    }

When trimmed, the application will throw:

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at ConsoleApp111.Program.Main() in C:\Users\eerhardt\source\repos\ConsoleApp111\ConsoleApp111\Program.cs:line 29

cc @vitek-karas

[vitek-karas]

#1619 has a sample test case for this.

The bug is that we only recognize binding flags if they are constants. If not, we actually behave as if an overload of GetMethod which doesn't take any binding flags was called, and so we "fallback" to "Public|Instance|Static" binding flags.

The correct behavior is to fallback to "annotations" (in spirit, not in implementation) and mark all methods on the affected type. Basically the same thing we do if IgnoreCase binding flag is specified.

This bug likely affects fields and properties and potentially other things as well.

[vitek-karas]

@tlakollo could you please take a look at this and possibly fix it?