Skip to content

Commit

Permalink
Fix handling of unknown BindingFlags (#2288)
Browse files Browse the repository at this point in the history 
` GetDynamicallyAccessedMemberTypesFromBindingFlagsForXXX` would return `DynamicallyAccessedMembers.None` when the binding flags are null/unknown.

This fix is a bit more targeted so that we can potentially take it to 6.0.

In general, there's some duality in how BindingFlags are handled - there are places that call `BindingFlagsAreUnsupported` and avoid `GetDynamicallyAccessedMemberTypesFromBindingFlagsForXXX` for null that way. Others call into this API without checking whether the flags are supported first.

I'm unclear whether it would be more appropriate to check for `BindingFlagsAreUnsupported` in the code I'm adding. It's a valid option as well.
  • Loading branch information
@MichalStrehovsky
MichalStrehovsky authored Sep 21, 2021
1 parent c230fdf commit 108fe5b
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 8 deletions.
21 changes: 13 additions & 8 deletions src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1117,8 +1117,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
reflectionContext.RecordHandledPattern ();
} else {
// Otherwise fall back to the bitfield requirements
var requiredMemberTypes = HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicConstructors : DynamicallyAccessedMemberTypes.None;
requiredMemberTypes |= HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None;
var requiredMemberTypes = GetDynamicallyAccessedMemberTypesFromBindingFlagsForConstructors (bindingFlags);
// We can scope down the public constructors requirement if we know the number of parameters is 0
if (requiredMemberTypes == DynamicallyAccessedMemberTypes.PublicConstructors && ctorParameterCount == 0)
requiredMemberTypes = DynamicallyAccessedMemberTypes.PublicParameterlessConstructor;
Expand Down Expand Up @@ -2407,27 +2406,33 @@ void ValidateGenericMethodInstantiation (

static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForNestedTypes (BindingFlags? bindingFlags) =>
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicNestedTypes : DynamicallyAccessedMemberTypes.None) |
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None);
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None) |
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None);

static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForConstructors (BindingFlags? bindingFlags) =>
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicConstructors : DynamicallyAccessedMemberTypes.None) |
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None);
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None) |
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None);

static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForMethods (BindingFlags? bindingFlags) =>
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicMethods : DynamicallyAccessedMemberTypes.None) |
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None);
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None) |
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None);

static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForFields (BindingFlags? bindingFlags) =>
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicFields : DynamicallyAccessedMemberTypes.None) |
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None);
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None) |
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None);

static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForProperties (BindingFlags? bindingFlags) =>
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicProperties : DynamicallyAccessedMemberTypes.None) |
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None);
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None) |
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None);

static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForEvents (BindingFlags? bindingFlags) =>
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicEvents : DynamicallyAccessedMemberTypes.None) |
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None);
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None) |
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None);
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForMembers (BindingFlags? bindingFlags) =>
GetDynamicallyAccessedMemberTypesFromBindingFlagsForConstructors (bindingFlags) |
GetDynamicallyAccessedMemberTypesFromBindingFlagsForEvents (bindingFlags) |
Expand Down
32 changes: 32 additions & 0 deletions test/Mono.Linker.Tests.Cases/Reflection/EventUsedViaReflection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public static void Main ()
TestNameBindingFlags ();
TestNameWrongBindingFlags ();
TestNameUnknownBindingFlags (BindingFlags.Public);
TestNameUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
TestNullName ();
TestEmptyName ();
TestNonExistingName ();
Expand Down Expand Up @@ -71,6 +72,13 @@ static void TestNameUnknownBindingFlags (BindingFlags bindingFlags)
var eventInfo = typeof (UnknownBindingFlags).GetEvent ("PrivateEvent", bindingFlags);
}

[Kept]
static void TestNameUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
{
// Since the binding flags are not known linker should mark all events on the type
var eventInfo = typeof (UnknownBindingFlagsAndName).GetEvent (name, bindingFlags);
}

[Kept]
static void TestNullName ()
{
Expand Down Expand Up @@ -221,6 +229,30 @@ class UnknownBindingFlags
public event EventHandler<EventArgs> PublicEvent;
}

class UnknownBindingFlagsAndName
{
[Kept]
[KeptEventAddMethod]
[KeptEventRemoveMethod]
[method: ExpectBodyModified]
internal event EventHandler<EventArgs> InternalEvent;
[Kept]
[KeptBackingField]
[KeptEventAddMethod]
[KeptEventRemoveMethod]
static event EventHandler<EventArgs> Static;
[Kept]
[KeptEventAddMethod]
[KeptEventRemoveMethod]
[method: ExpectBodyModified]
private event EventHandler<EventArgs> PrivateEvent;
[Kept]
[KeptEventAddMethod]
[KeptEventRemoveMethod]
[method: ExpectBodyModified]
public event EventHandler<EventArgs> PublicEvent;
}

class IfClass
{
[Kept]
Expand Down
19 changes: 19 additions & 0 deletions test/Mono.Linker.Tests.Cases/Reflection/FieldUsedViaReflection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public static void Main ()
TestNameBindingFlags ();
TestNameWrongBindingFlags ();
TestNameUnknownBindingFlags (BindingFlags.Public);
TestNameUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
TestNullName ();
TestEmptyName ();
TestNonExistingName ();
Expand Down Expand Up @@ -66,6 +67,13 @@ static void TestNameUnknownBindingFlags (BindingFlags bindingFlags)
var field = typeof (UnknownBindingFlags).GetField ("field", bindingFlags);
}

[Kept]
static void TestNameUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
{
// Since the binding flags and name are not known linker should mark all fields on the type
var field = typeof (UnknownBindingFlagsAndName).GetField (name, bindingFlags);
}

[Kept]
static void TestNullName ()
{
Expand Down Expand Up @@ -186,6 +194,17 @@ private class UnknownBindingFlags
private static int privatefield;
}

[Kept]
private class UnknownBindingFlagsAndName
{
[Kept]
public static int field;
[Kept]
public int nonStatic;
[Kept]
private static int privatefield;
}

[Kept]
private class IfClass
{
Expand Down
20 changes: 20 additions & 0 deletions test/Mono.Linker.Tests.Cases/Reflection/MethodUsedViaReflection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public static void Main ()
GetMethod_Name_Types.TestNameAndType ();
GetMethod_Name_BindingAttr.TestExplicitBindingFlags ();
GetMethod_Name_BindingAttr.TestUnknownBindingFlags (BindingFlags.Public);
GetMethod_Name_BindingAttr.TestUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
GetMethod_Name_BindingAttr.TestUnknownNullBindingFlags (BindingFlags.Public);
GetMethod_Name_BindingAttr_Binder_Types_Modifiers.TestNameBindingFlagsAndParameterModifier ();
GetMethod_Name_BindingAttr_Binder_CallConvention_Types_Modifiers.TestNameBindingFlagsCallingConventionParameterModifier ();
Expand Down Expand Up @@ -199,6 +200,25 @@ public static void TestUnknownBindingFlags (BindingFlags bindingFlags)
method.Invoke (null, new object[] { });
}

[Kept]
class UnknownBindingFlagsAndName
{
[Kept]
private static int OnlyCalledViaReflection ()
{
return 42;
}
}

[Kept]
[RecognizedReflectionAccessPattern]
public static void TestUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
{
// Since the binding flags and name are not known linker should mark all methods on the type
var method = typeof (UnknownBindingFlagsAndName).GetMethod (name, bindingFlags);
method.Invoke (null, new object[] { });
}

[Kept]
private class NullBindingFlags
{
Expand Down
21 changes: 21 additions & 0 deletions test/Mono.Linker.Tests.Cases/Reflection/NestedTypeUsedViaReflection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public static void Main ()
TestPrivateByName ();
TestByBindingFlags ();
TestByUnknownBindingFlags (BindingFlags.Public);
TestByUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
TestNonExistingName ();
TestNullType ();
TestIgnoreCaseBindingFlags ();
Expand Down Expand Up @@ -81,6 +82,16 @@ static void TestByUnknownBindingFlags (BindingFlags bindingFlags)
_ = typeof (UnknownBindingFlags).GetNestedType (nameof (PublicNestedType), bindingFlags);
}

[Kept]
[RecognizedReflectionAccessPattern (
typeof (Type), nameof (Type.GetNestedType), new Type[] { typeof (string), typeof (BindingFlags) },
typeof (UnknownBindingFlagsAndName.PublicNestedType), null, (Type[]) null)]
static void TestByUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
{
// Since the binding flags and name are not known linker should mark all nested types on the type
_ = typeof (UnknownBindingFlagsAndName).GetNestedType (name, bindingFlags);
}

[Kept]
static void TestNonExistingName ()
{
Expand Down Expand Up @@ -125,6 +136,16 @@ public static class PublicNestedType { }
private static class PrivateNestedType { }
}

[Kept]
private class UnknownBindingFlagsAndName
{
[Kept]
public static class PublicNestedType { }

[Kept]
private static class PrivateNestedType { }
}

[Kept]
private class IgnoreCaseClass
{
Expand Down
24 changes: 24 additions & 0 deletions test/Mono.Linker.Tests.Cases/Reflection/PropertyUsedViaReflection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public static void Main ()
TestGetterOnly ();
TestBindingFlags ();
TestUnknownBindingFlags (BindingFlags.Public);
TestUnknownBindingFlagsAndName (BindingFlags.Public, "IrrelevantName");
TestNullName ();
TestEmptyName ();
TestNonExistingName ();
Expand Down Expand Up @@ -89,6 +90,17 @@ static void TestUnknownBindingFlags (BindingFlags bindingFlags)
property.GetValue (null, new object[] { });
}

[Kept]
[RecognizedReflectionAccessPattern (
typeof (Type), nameof (Type.GetProperty), new Type[] { typeof (string), typeof (BindingFlags) },
typeof (UnknownBindingFlagsAndName), nameof (UnknownBindingFlagsAndName.SomeProperty), (Type[]) null)]
static void TestUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
{
// Since the binding flags and name are not known linker should mark all properties on the type
var property = typeof (UnknownBindingFlagsAndName).GetProperty (name, bindingFlags);
property.GetValue (null, new object[] { });
}

[Kept]
static void TestNullName ()
{
Expand Down Expand Up @@ -313,6 +325,18 @@ internal static int SomeProperty {
}
}

[Kept]
class UnknownBindingFlagsAndName
{
[Kept]
internal static int SomeProperty {
[Kept]
private get { return _field; }
[Kept]
set { _field = value; }
}
}

[Kept]
class IgnoreCaseBindingFlagsClass
{
Expand Down

0 comments on commit 108fe5b

Please sign in to comment.