Skip to content

Commit 108fe5b

Browse files
MichalStrehovskyMichalStrehovsky
authored
Fix handling of unknown BindingFlags (#2288)
` 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.
1 parent c230fdf commit 108fe5b

File tree

6 files changed

+129
-8
lines changed

6 files changed

+129
-8
lines changed

‎src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,8 +1117,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
11171117
reflectionContext.RecordHandledPattern ();
11181118
} else {
11191119
// Otherwise fall back to the bitfield requirements
1120-
var requiredMemberTypes = HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicConstructors : DynamicallyAccessedMemberTypes.None;
1121-
requiredMemberTypes |= HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None;
1120+
var requiredMemberTypes = GetDynamicallyAccessedMemberTypesFromBindingFlagsForConstructors (bindingFlags);
11221121
// We can scope down the public constructors requirement if we know the number of parameters is 0
11231122
if (requiredMemberTypes == DynamicallyAccessedMemberTypes.PublicConstructors && ctorParameterCount == 0)
11241123
requiredMemberTypes = DynamicallyAccessedMemberTypes.PublicParameterlessConstructor;
@@ -2407,27 +2406,33 @@ void ValidateGenericMethodInstantiation (
24072406

24082407
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForNestedTypes (BindingFlags? bindingFlags) =>
24092408
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicNestedTypes : DynamicallyAccessedMemberTypes.None) |
2410-
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None);
2409+
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None) |
2410+
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None);
24112411

24122412
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForConstructors (BindingFlags? bindingFlags) =>
24132413
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicConstructors : DynamicallyAccessedMemberTypes.None) |
2414-
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None);
2414+
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None) |
2415+
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors : DynamicallyAccessedMemberTypes.None);
24152416

24162417
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForMethods (BindingFlags? bindingFlags) =>
24172418
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicMethods : DynamicallyAccessedMemberTypes.None) |
2418-
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None);
2419+
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None) |
2420+
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods : DynamicallyAccessedMemberTypes.None);
24192421

24202422
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForFields (BindingFlags? bindingFlags) =>
24212423
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicFields : DynamicallyAccessedMemberTypes.None) |
2422-
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None);
2424+
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None) |
2425+
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields : DynamicallyAccessedMemberTypes.None);
24232426

24242427
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForProperties (BindingFlags? bindingFlags) =>
24252428
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicProperties : DynamicallyAccessedMemberTypes.None) |
2426-
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None);
2429+
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None) |
2430+
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties : DynamicallyAccessedMemberTypes.None);
24272431

24282432
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForEvents (BindingFlags? bindingFlags) =>
24292433
(HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicEvents : DynamicallyAccessedMemberTypes.None) |
2430-
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None);
2434+
(HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None) |
2435+
(BindingFlagsAreUnsupported (bindingFlags) ? DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents : DynamicallyAccessedMemberTypes.None);
24312436
static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForMembers (BindingFlags? bindingFlags) =>
24322437
GetDynamicallyAccessedMemberTypesFromBindingFlagsForConstructors (bindingFlags) |
24332438
GetDynamicallyAccessedMemberTypesFromBindingFlagsForEvents (bindingFlags) |

‎test/Mono.Linker.Tests.Cases/Reflection/EventUsedViaReflection.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public static void Main ()
1717
TestNameBindingFlags ();
1818
TestNameWrongBindingFlags ();
1919
TestNameUnknownBindingFlags (BindingFlags.Public);
20+
TestNameUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
2021
TestNullName ();
2122
TestEmptyName ();
2223
TestNonExistingName ();
@@ -71,6 +72,13 @@ static void TestNameUnknownBindingFlags (BindingFlags bindingFlags)
7172
var eventInfo = typeof (UnknownBindingFlags).GetEvent ("PrivateEvent", bindingFlags);
7273
}
7374

75+
[Kept]
76+
static void TestNameUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
77+
{
78+
// Since the binding flags are not known linker should mark all events on the type
79+
var eventInfo = typeof (UnknownBindingFlagsAndName).GetEvent (name, bindingFlags);
80+
}
81+
7482
[Kept]
7583
static void TestNullName ()
7684
{
@@ -221,6 +229,30 @@ class UnknownBindingFlags
221229
public event EventHandler<EventArgs> PublicEvent;
222230
}
223231

232+
class UnknownBindingFlagsAndName
233+
{
234+
[Kept]
235+
[KeptEventAddMethod]
236+
[KeptEventRemoveMethod]
237+
[method: ExpectBodyModified]
238+
internal event EventHandler<EventArgs> InternalEvent;
239+
[Kept]
240+
[KeptBackingField]
241+
[KeptEventAddMethod]
242+
[KeptEventRemoveMethod]
243+
static event EventHandler<EventArgs> Static;
244+
[Kept]
245+
[KeptEventAddMethod]
246+
[KeptEventRemoveMethod]
247+
[method: ExpectBodyModified]
248+
private event EventHandler<EventArgs> PrivateEvent;
249+
[Kept]
250+
[KeptEventAddMethod]
251+
[KeptEventRemoveMethod]
252+
[method: ExpectBodyModified]
253+
public event EventHandler<EventArgs> PublicEvent;
254+
}
255+
224256
class IfClass
225257
{
226258
[Kept]

‎test/Mono.Linker.Tests.Cases/Reflection/FieldUsedViaReflection.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ public static void Main ()
1515
TestNameBindingFlags ();
1616
TestNameWrongBindingFlags ();
1717
TestNameUnknownBindingFlags (BindingFlags.Public);
18+
TestNameUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
1819
TestNullName ();
1920
TestEmptyName ();
2021
TestNonExistingName ();
@@ -66,6 +67,13 @@ static void TestNameUnknownBindingFlags (BindingFlags bindingFlags)
6667
var field = typeof (UnknownBindingFlags).GetField ("field", bindingFlags);
6768
}
6869

70+
[Kept]
71+
static void TestNameUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
72+
{
73+
// Since the binding flags and name are not known linker should mark all fields on the type
74+
var field = typeof (UnknownBindingFlagsAndName).GetField (name, bindingFlags);
75+
}
76+
6977
[Kept]
7078
static void TestNullName ()
7179
{
@@ -186,6 +194,17 @@ private class UnknownBindingFlags
186194
private static int privatefield;
187195
}
188196

197+
[Kept]
198+
private class UnknownBindingFlagsAndName
199+
{
200+
[Kept]
201+
public static int field;
202+
[Kept]
203+
public int nonStatic;
204+
[Kept]
205+
private static int privatefield;
206+
}
207+
189208
[Kept]
190209
private class IfClass
191210
{

‎test/Mono.Linker.Tests.Cases/Reflection/MethodUsedViaReflection.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public static void Main ()
1717
GetMethod_Name_Types.TestNameAndType ();
1818
GetMethod_Name_BindingAttr.TestExplicitBindingFlags ();
1919
GetMethod_Name_BindingAttr.TestUnknownBindingFlags (BindingFlags.Public);
20+
GetMethod_Name_BindingAttr.TestUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
2021
GetMethod_Name_BindingAttr.TestUnknownNullBindingFlags (BindingFlags.Public);
2122
GetMethod_Name_BindingAttr_Binder_Types_Modifiers.TestNameBindingFlagsAndParameterModifier ();
2223
GetMethod_Name_BindingAttr_Binder_CallConvention_Types_Modifiers.TestNameBindingFlagsCallingConventionParameterModifier ();
@@ -199,6 +200,25 @@ public static void TestUnknownBindingFlags (BindingFlags bindingFlags)
199200
method.Invoke (null, new object[] { });
200201
}
201202

203+
[Kept]
204+
class UnknownBindingFlagsAndName
205+
{
206+
[Kept]
207+
private static int OnlyCalledViaReflection ()
208+
{
209+
return 42;
210+
}
211+
}
212+
213+
[Kept]
214+
[RecognizedReflectionAccessPattern]
215+
public static void TestUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
216+
{
217+
// Since the binding flags and name are not known linker should mark all methods on the type
218+
var method = typeof (UnknownBindingFlagsAndName).GetMethod (name, bindingFlags);
219+
method.Invoke (null, new object[] { });
220+
}
221+
202222
[Kept]
203223
private class NullBindingFlags
204224
{

‎test/Mono.Linker.Tests.Cases/Reflection/NestedTypeUsedViaReflection.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public static void Main ()
1717
TestPrivateByName ();
1818
TestByBindingFlags ();
1919
TestByUnknownBindingFlags (BindingFlags.Public);
20+
TestByUnknownBindingFlagsAndName (BindingFlags.Public, "DoesntMatter");
2021
TestNonExistingName ();
2122
TestNullType ();
2223
TestIgnoreCaseBindingFlags ();
@@ -81,6 +82,16 @@ static void TestByUnknownBindingFlags (BindingFlags bindingFlags)
8182
_ = typeof (UnknownBindingFlags).GetNestedType (nameof (PublicNestedType), bindingFlags);
8283
}
8384

85+
[Kept]
86+
[RecognizedReflectionAccessPattern (
87+
typeof (Type), nameof (Type.GetNestedType), new Type[] { typeof (string), typeof (BindingFlags) },
88+
typeof (UnknownBindingFlagsAndName.PublicNestedType), null, (Type[]) null)]
89+
static void TestByUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
90+
{
91+
// Since the binding flags and name are not known linker should mark all nested types on the type
92+
_ = typeof (UnknownBindingFlagsAndName).GetNestedType (name, bindingFlags);
93+
}
94+
8495
[Kept]
8596
static void TestNonExistingName ()
8697
{
@@ -125,6 +136,16 @@ public static class PublicNestedType { }
125136
private static class PrivateNestedType { }
126137
}
127138

139+
[Kept]
140+
private class UnknownBindingFlagsAndName
141+
{
142+
[Kept]
143+
public static class PublicNestedType { }
144+
145+
[Kept]
146+
private static class PrivateNestedType { }
147+
}
148+
128149
[Kept]
129150
private class IgnoreCaseClass
130151
{

‎test/Mono.Linker.Tests.Cases/Reflection/PropertyUsedViaReflection.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public static void Main ()
1717
TestGetterOnly ();
1818
TestBindingFlags ();
1919
TestUnknownBindingFlags (BindingFlags.Public);
20+
TestUnknownBindingFlagsAndName (BindingFlags.Public, "IrrelevantName");
2021
TestNullName ();
2122
TestEmptyName ();
2223
TestNonExistingName ();
@@ -89,6 +90,17 @@ static void TestUnknownBindingFlags (BindingFlags bindingFlags)
8990
property.GetValue (null, new object[] { });
9091
}
9192

93+
[Kept]
94+
[RecognizedReflectionAccessPattern (
95+
typeof (Type), nameof (Type.GetProperty), new Type[] { typeof (string), typeof (BindingFlags) },
96+
typeof (UnknownBindingFlagsAndName), nameof (UnknownBindingFlagsAndName.SomeProperty), (Type[]) null)]
97+
static void TestUnknownBindingFlagsAndName (BindingFlags bindingFlags, string name)
98+
{
99+
// Since the binding flags and name are not known linker should mark all properties on the type
100+
var property = typeof (UnknownBindingFlagsAndName).GetProperty (name, bindingFlags);
101+
property.GetValue (null, new object[] { });
102+
}
103+
92104
[Kept]
93105
static void TestNullName ()
94106
{
@@ -313,6 +325,18 @@ internal static int SomeProperty {
313325
}
314326
}
315327

328+
[Kept]
329+
class UnknownBindingFlagsAndName
330+
{
331+
[Kept]
332+
internal static int SomeProperty {
333+
[Kept]
334+
private get { return _field; }
335+
[Kept]
336+
set { _field = value; }
337+
}
338+
}
339+
316340
[Kept]
317341
class IgnoreCaseBindingFlagsClass
318342
{

0 commit comments

Comments
 (0)