-
Notifications
You must be signed in to change notification settings - Fork 128
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.
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
Support RunClassConstructor in the linker #1121
Changes from all commits
b580ce8
02a6c8a
ea08bc6
fe764f7
0abc1b5
b11f4f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -156,6 +156,7 @@ enum IntrinsicId | |
None = 0, | ||
IntrospectionExtensions_GetTypeInfo, | ||
Type_GetTypeFromHandle, | ||
Type_get_TypeHandle, | ||
|
||
// Anything above this marker will require the method to be run through | ||
// the reflection body scanner. | ||
|
@@ -183,7 +184,8 @@ enum IntrinsicId | |
RuntimeReflectionExtensions_GetRuntimeEvent, | ||
RuntimeReflectionExtensions_GetRuntimeField, | ||
RuntimeReflectionExtensions_GetRuntimeMethod, | ||
RuntimeReflectionExtensions_GetRuntimeProperty | ||
RuntimeReflectionExtensions_GetRuntimeProperty, | ||
RuntimeHelpers_RunClassConstructor | ||
} | ||
|
||
static IntrinsicId GetIntrinsicIdForMethod (MethodDefinition calledMethod) | ||
|
@@ -196,6 +198,9 @@ static IntrinsicId GetIntrinsicIdForMethod (MethodDefinition calledMethod) | |
// System.Type.GetTypeInfo (Type type) | ||
"GetTypeFromHandle" when calledMethod.IsDeclaredOnType ("System", "Type") => IntrinsicId.Type_GetTypeFromHandle, | ||
|
||
// System.Type.GetTypeHandle (Type type) | ||
"get_TypeHandle" when calledMethod.IsDeclaredOnType ("System", "Type") => IntrinsicId.Type_get_TypeHandle, | ||
|
||
// static System.Type.MakeGenericType (Type [] typeArguments) | ||
"MakeGenericType" when calledMethod.IsDeclaredOnType ("System", "Type") => IntrinsicId.Type_MakeGenericType, | ||
|
||
|
@@ -379,6 +384,11 @@ static IntrinsicId GetIntrinsicIdForMethod (MethodDefinition calledMethod) | |
&& calledMethod.HasParameterOfType (0, "System", "String") | ||
=> IntrinsicId.Assembly_CreateInstance, | ||
|
||
// System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor (RuntimeTypeHandle type) | ||
"RunClassConstructor" when calledMethod.IsDeclaredOnType ("System.Runtime.CompilerServices", "RuntimeHelpers") | ||
&& calledMethod.HasParameterOfType (0, "System", "RuntimeTypeHandle") | ||
=> IntrinsicId.RuntimeHelpers_RunClassConstructor, | ||
|
||
_ => IntrinsicId.None, | ||
}; | ||
} | ||
|
@@ -418,6 +428,18 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c | |
} | ||
break; | ||
|
||
case IntrinsicId.Type_get_TypeHandle: { | ||
foreach (var value in methodParams [0].UniqueValues()) { | ||
if (value is SystemTypeValue typeValue) | ||
methodReturnValue = MergePointValue.MergeValues (methodReturnValue, new RuntimeTypeHandleValue (typeValue.TypeRepresented)); | ||
else if (value == NullValue.Instance) | ||
methodReturnValue = MergePointValue.MergeValues (methodReturnValue, value); | ||
else | ||
methodReturnValue = MergePointValue.MergeValues (methodReturnValue, UnknownValue.Instance); | ||
} | ||
} | ||
break; | ||
|
||
case IntrinsicId.Type_MakeGenericType: { | ||
// Don't care about the actual arguments, but we don't want to lose track of the type | ||
// in case this is e.g. Activator.CreateInstance(typeof(Foo<>).MakeGenericType(...)); | ||
|
@@ -926,6 +948,28 @@ methodParams [argsParam] is ArrayValue arrayValue && | |
reflectionContext.RecordUnrecognizedPattern ($"Activator call '{reflectionContext.MethodCalled.FullName}' inside '{reflectionContext.MethodCalling.FullName}' is not yet supported"); | ||
break; | ||
|
||
// | ||
// System.Runtime.CompilerServices.RuntimeHelpers | ||
// | ||
// RunClassConstructor (RuntimeTypeHandle type) | ||
// | ||
case IntrinsicId.RuntimeHelpers_RunClassConstructor: { | ||
reflectionContext.AnalyzingPattern (); | ||
foreach (var typeHandleValue in methodParams [0].UniqueValues ()) { | ||
if (typeHandleValue is RuntimeTypeHandleValue runtimeTypeHandleValue) { | ||
_markStep.MarkStaticConstructor (runtimeTypeHandleValue.TypeRepresented, new DependencyInfo (DependencyKind.AccessedViaReflection, reflectionContext.MethodCalling)); | ||
reflectionContext.RecordHandledPattern (); | ||
} else if (typeHandleValue == NullValue.Instance) | ||
reflectionContext.RecordHandledPattern (); | ||
else { | ||
reflectionContext.RecordUnrecognizedPattern ($"A {GetValueDescriptionForErrorMessage (typeHandleValue)} " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit/not really necessary: If we really wanted - the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. For now I would leave it as is. Supporting the annotation would mean to flow it through the get_TypeHandle call as well. Until we hit a use case I'm perfectly fine supporting just the simple constant cases. |
||
$"is passed into the {GetMetadataTokenDescriptionForErrorMessage (reflectionContext.MethodCalled.Parameters[0])}. " + | ||
$"It's not possible to guarantee availability of the target static constructor."); | ||
} | ||
} | ||
} | ||
break; | ||
|
||
default: | ||
if (requiresDataFlowAnalysis) { | ||
reflectionContext.AnalyzingPattern (); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
using System; | ||
using System.Reflection; | ||
using Mono.Linker.Tests.Cases.Expectations.Assertions; | ||
using Mono.Linker.Tests.Cases.Expectations.Metadata; | ||
using System.Runtime.CompilerServices; | ||
|
||
namespace Mono.Linker.Tests.Cases.Reflection | ||
{ | ||
[SetupCSharpCompilerToUse ("csc")] | ||
public class RunClassConstructorUsedViaReflection | ||
{ | ||
public static void Main () | ||
{ | ||
TestRunClassConstructor (); | ||
TestNonKeptStaticConstructor (); | ||
TestNull (); | ||
TestDataFlowType (); | ||
TestIfElseUsingRuntimeTypeHandle (1); | ||
TestIfElseUsingType (1); | ||
} | ||
|
||
[Kept] | ||
[RecognizedReflectionAccessPattern] | ||
static void TestRunClassConstructor () | ||
{ | ||
RuntimeHelpers.RunClassConstructor (typeof (OnlyUsedViaReflection).TypeHandle); | ||
} | ||
|
||
[Kept] | ||
static void TestNonKeptStaticConstructor () | ||
{ | ||
var a = new NonKeptStaticConstructorClass(); | ||
} | ||
|
||
[Kept] | ||
[RecognizedReflectionAccessPattern] | ||
static void TestNull () | ||
{ | ||
Type type = null; | ||
RuntimeHelpers.RunClassConstructor (type.TypeHandle); | ||
tlakollo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
[Kept] | ||
static Type FindType () | ||
{ | ||
return null; | ||
} | ||
|
||
[Kept] | ||
[UnrecognizedReflectionAccessPattern (typeof (RuntimeHelpers), nameof (RuntimeHelpers.RunClassConstructor), new Type [] { typeof (RuntimeTypeHandle) }, | ||
"A value from unknown source is passed into the parameter 'type' of method "+ | ||
"'System.Void System.Runtime.CompilerServices.RuntimeHelpers::RunClassConstructor(System.RuntimeTypeHandle)'. "+ | ||
"It's not possible to guarantee availability of the target static constructor.")] | ||
|
||
static void TestDataFlowType () | ||
{ | ||
Type type = FindType (); | ||
RuntimeHelpers.RunClassConstructor (type.TypeHandle); | ||
tlakollo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
[Kept] | ||
[RecognizedReflectionAccessPattern] | ||
[UnrecognizedReflectionAccessPattern (typeof (RuntimeHelpers), nameof (RuntimeHelpers.RunClassConstructor), new Type [] { typeof (RuntimeTypeHandle) }, | ||
"A value from unknown source is passed into the parameter 'type' of method "+ | ||
"'System.Void System.Runtime.CompilerServices.RuntimeHelpers::RunClassConstructor(System.RuntimeTypeHandle)'. "+ | ||
"It's not possible to guarantee availability of the target static constructor.")] | ||
|
||
static void TestIfElseUsingRuntimeTypeHandle (int i) | ||
{ | ||
RuntimeTypeHandle myType; | ||
if (i == 1) { | ||
myType = typeof (IfClass).TypeHandle; | ||
} else if (i == 2) { | ||
myType = FindType ().TypeHandle; | ||
} else { | ||
myType = typeof (ElseClass).TypeHandle; | ||
} | ||
RuntimeHelpers.RunClassConstructor (myType); | ||
} | ||
|
||
[Kept] | ||
[RecognizedReflectionAccessPattern] | ||
static void TestIfElseUsingType (int i) | ||
{ | ||
Type myType; | ||
if (i == 1) { | ||
myType = typeof (IfClass2); | ||
}else if (i==2) { | ||
myType = null; | ||
} | ||
else { | ||
myType = typeof (ElseClass2); | ||
} | ||
RuntimeHelpers.RunClassConstructor (myType.TypeHandle); | ||
} | ||
|
||
[Kept] | ||
[KeptMember (".cctor()")] | ||
class OnlyUsedViaReflection | ||
{ | ||
[Kept] | ||
static int i = 5; | ||
} | ||
|
||
[Kept] | ||
[KeptMember (".ctor()")] | ||
class NonKeptStaticConstructorClass | ||
{ | ||
static int i = 5; | ||
} | ||
|
||
[Kept] | ||
[KeptMember (".cctor()")] | ||
class IfClass | ||
{ | ||
public IfClass () | ||
{ } | ||
private IfClass (int foo) | ||
{ } | ||
} | ||
|
||
[Kept] | ||
[KeptMember (".cctor()")] | ||
class ElseClass | ||
{ | ||
[Kept] | ||
static ElseClass () | ||
{ } | ||
public ElseClass (int foo) | ||
{ } | ||
} | ||
[Kept] | ||
[KeptMember (".cctor()")] | ||
class IfClass2 | ||
{ | ||
public IfClass2 () | ||
{ } | ||
private IfClass2 (int foo) | ||
{ } | ||
} | ||
|
||
[Kept] | ||
[KeptMember (".cctor()")] | ||
class ElseClass2 | ||
{ | ||
[Kept] | ||
static ElseClass2 () | ||
{ } | ||
public ElseClass2 (int foo) | ||
{ } | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this really be in
DependencyKind.AccessedViaReflection
category?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could create a new one I guess. But it's not that different from something like
type.GetConstructor().Invoke()
. I do think it's still reflection, even if the method/type is not in the reflection namespace.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we made a new category for this, it would set a precedent and we'll need yet another category for e.g.
FormatterServices.GetUninitializedObject
- these are all reflection-like APIs...