Skip to content

Commit 30b3721

Browse files
sbomersbomer
authored
Preserve static type info for return value of ctor (#101212)
Instead of tracking the return value as "TopValue" or "unknown", this models the constructor as returning a value with a static type when called with newobj, letting us undo the workaround from #101031.
1 parent 017593d commit 30b3721

File tree

23 files changed

+129
-55
lines changed

23 files changed

+129
-55
lines changed

‎src/coreclr/tools/Common/Compiler/Dataflow/MethodProxy.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ internal partial ImmutableArray<GenericParameterProxy> GetGenericParameters()
6666
return builder.ToImmutableArray();
6767
}
6868

69+
internal partial bool IsConstructor() => Method.IsConstructor;
70+
6971
internal partial bool IsStatic() => Method.Signature.IsStatic;
7072

7173
internal partial bool HasImplicitThis() => !Method.Signature.IsStatic;

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -951,12 +951,12 @@ internal partial bool MethodRequiresDataFlowAnalysis(MethodProxy method)
951951
=> RequiresDataflowAnalysisDueToSignature(method.Method);
952952

953953
#pragma warning disable CA1822 // Other partial implementations are not in the ilc project
954-
internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
954+
internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
955955
#pragma warning restore CA1822 // Mark members as static
956-
=> new MethodReturnValue(method.Method, dynamicallyAccessedMemberTypes);
956+
=> new MethodReturnValue(method.Method, isNewObj, dynamicallyAccessedMemberTypes);
957957

958-
internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method)
959-
=> GetMethodReturnValue(method, GetReturnParameterAnnotation(method.Method));
958+
internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method, bool isNewObj)
959+
=> GetMethodReturnValue(method, isNewObj, GetReturnParameterAnnotation(method.Method));
960960

961961
internal partial GenericParameterValue GetGenericParameterValue(GenericParameterProxy genericParameter)
962962
=> new GenericParameterValue(genericParameter.GenericParameter, GetGenericParameterAnnotation(genericParameter.GenericParameter));

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public HandleCallAction(
3939
{
4040
_reflectionMarker = reflectionMarker;
4141
_operation = operation;
42+
_isNewObj = operation == ILOpcode.newobj;
4243
_diagnosticContext = diagnosticContext;
4344
_callingMethod = callingMethod;
4445
_annotations = annotations;
@@ -337,12 +338,6 @@ private partial bool TryHandleIntrinsic (
337338
if (Intrinsics.GetIntrinsicIdForMethod(_callingMethod) == IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo)
338339
break;
339340

340-
if (param.IsEmpty())
341-
{
342-
// The static value is unknown and the below `foreach` won't execute
343-
_reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ReflectedDelegate(null), "Delegate.Method access on unknown delegate type");
344-
}
345-
346341
foreach (var valueNode in param.AsEnumerable())
347342
{
348343
TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type;
@@ -390,7 +385,7 @@ private partial bool TryHandleIntrinsic (
390385
if (staticType is null || (!staticType.IsDefType && !staticType.IsArray))
391386
{
392387
// We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations"
393-
AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod));
388+
AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj));
394389
}
395390
else if (staticType.IsSealed() || staticType.IsTypeOf("System", "Delegate"))
396391
{
@@ -425,7 +420,7 @@ private partial bool TryHandleIntrinsic (
425420
// Return a value which is "unknown type" with annotation. For now we'll use the return value node
426421
// for the method, which means we're loosing the information about which staticType this
427422
// started with. For now we don't need it, but we can add it later on.
428-
AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, annotation));
423+
AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj, annotation));
429424
}
430425
}
431426
}

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Generic;
5+
using System.Diagnostics;
56
using System.Diagnostics.CodeAnalysis;
67
using ILCompiler.Dataflow;
78
using ILLink.Shared.DataFlow;
@@ -16,9 +17,10 @@ namespace ILLink.Shared.TrimAnalysis
1617
/// </summary>
1718
internal partial record MethodReturnValue
1819
{
19-
public MethodReturnValue(MethodDesc method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
20+
public MethodReturnValue(MethodDesc method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
2021
{
21-
StaticType = method.Signature.ReturnType;
22+
Debug.Assert(!isNewObj || method.IsConstructor, "isNewObj can only be true for constructors");
23+
StaticType = isNewObj ? method.OwningType : method.Signature.ReturnType;
2224
Method = method;
2325
DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes;
2426
}

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ protected override void Scan(MethodIL methodBody, ref InterproceduralState inter
118118
if (!methodBody.OwningMethod.Signature.ReturnType.IsVoid)
119119
{
120120
var method = methodBody.OwningMethod;
121-
var methodReturnValue = _annotations.GetMethodReturnValue(method);
121+
var methodReturnValue = _annotations.GetMethodReturnValue(method, isNewObj: false);
122122
if (methodReturnValue.DynamicallyAccessedMemberTypes != 0)
123123
HandleAssignmentPattern(_origin, ReturnValue, methodReturnValue, method.GetDisplayName());
124124
}
@@ -315,7 +315,8 @@ public static MultiValue HandleCall(
315315
var callingMethodDefinition = callingMethodBody.OwningMethod;
316316
Debug.Assert(callingMethodDefinition == diagnosticContext.Origin.MemberDefinition);
317317

318-
var annotatedMethodReturnValue = reflectionMarker.Annotations.GetMethodReturnValue(calledMethod);
318+
bool isNewObj = operation == ILOpcode.newobj;
319+
var annotatedMethodReturnValue = reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, isNewObj);
319320
Debug.Assert(
320321
RequiresReflectionMethodBodyScannerForCallSite(reflectionMarker.Annotations, calledMethod) ||
321322
annotatedMethodReturnValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None);

‎src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,11 @@ public static DynamicallyAccessedMemberTypes GetTypeAnnotation(ITypeSymbol type)
103103
internal partial bool MethodRequiresDataFlowAnalysis (MethodProxy method)
104104
=> RequiresDataFlowAnalysis (method.Method);
105105

106-
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
107-
=> new MethodReturnValue (method.Method, dynamicallyAccessedMemberTypes);
106+
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
107+
=> new MethodReturnValue (method.Method, isNewObj, dynamicallyAccessedMemberTypes);
108108

109-
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method)
110-
=> GetMethodReturnValue (method, GetMethodReturnValueAnnotation (method.Method));
109+
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj)
110+
=> GetMethodReturnValue (method, isNewObj, GetMethodReturnValueAnnotation (method.Method));
111111

112112
internal partial GenericParameterValue GetGenericParameterValue (GenericParameterProxy genericParameter)
113113
=> new GenericParameterValue (genericParameter.TypeParameterSymbol);

‎src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public HandleCallAction (
3333
{
3434
_owningSymbol = owningSymbol;
3535
_operation = operation;
36+
_isNewObj = operation.Kind == OperationKind.ObjectCreation;
3637
_diagnosticContext = diagnosticContext;
3738
_annotations = FlowAnnotations.Instance;
3839
_reflectionAccessAnalyzer = default;
@@ -86,7 +87,7 @@ private partial bool TryHandleIntrinsic (
8687

8788
if (staticType is null) {
8889
// We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations"
89-
AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod));
90+
AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj));
9091
} else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.TypeKind == TypeKind.Array) {
9192
// We can treat this one the same as if it was a typeof() expression
9293

@@ -106,7 +107,7 @@ private partial bool TryHandleIntrinsic (
106107
AddReturnValue (new SystemTypeValue (new (staticType)));
107108
} else {
108109
var annotation = FlowAnnotations.GetTypeAnnotation (staticType);
109-
AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, annotation));
110+
AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj, annotation));
110111
}
111112
}
112113
break;

‎src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ internal partial ImmutableArray<GenericParameterProxy> GetGenericParameters ()
4646
return builder.ToImmutableArray ();
4747
}
4848

49+
internal partial bool IsConstructor () => Method.IsConstructor ();
50+
4951
internal partial bool IsStatic () => Method.IsStatic;
5052

5153
internal partial bool HasImplicitThis () => Method.HasImplicitThis ();

‎src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5+
using System.Diagnostics;
56
using System.Diagnostics.CodeAnalysis;
67
using ILLink.RoslynAnalyzer;
78
using ILLink.Shared.DataFlow;
@@ -11,16 +12,17 @@ namespace ILLink.Shared.TrimAnalysis
1112
{
1213
internal partial record MethodReturnValue
1314
{
14-
public MethodReturnValue (IMethodSymbol methodSymbol)
15-
: this (methodSymbol, FlowAnnotations.GetMethodReturnValueAnnotation (methodSymbol))
15+
public MethodReturnValue (IMethodSymbol methodSymbol, bool isNewObj)
16+
: this (methodSymbol, isNewObj, FlowAnnotations.GetMethodReturnValueAnnotation (methodSymbol))
1617
{
1718
}
1819

19-
public MethodReturnValue (IMethodSymbol methodSymbol, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
20+
public MethodReturnValue (IMethodSymbol methodSymbol, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
2021
{
22+
Debug.Assert (!isNewObj || methodSymbol.MethodKind == MethodKind.Constructor, "isNewObj can only be true for constructors");
2123
MethodSymbol = methodSymbol;
2224
DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes;
23-
StaticType = new (methodSymbol.ReturnType);
25+
StaticType = new (isNewObj ? methodSymbol.ContainingType : methodSymbol.ReturnType);
2426
}
2527

2628
public readonly IMethodSymbol MethodSymbol;

‎src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public override MultiValue VisitConversion (IConversionOperation operation, Stat
116116
var value = base.VisitConversion (operation, state);
117117

118118
if (operation.OperatorMethod != null)
119-
return operation.OperatorMethod.ReturnType.IsTypeInterestingForDataflow () ? new MethodReturnValue (operation.OperatorMethod) : value;
119+
return operation.OperatorMethod.ReturnType.IsTypeInterestingForDataflow () ? new MethodReturnValue (operation.OperatorMethod, isNewObj: false) : value;
120120

121121
// TODO - is it possible to have annotation on the operator method parameters?
122122
// if so, will these be checked here?
@@ -341,7 +341,7 @@ public override void HandleReturnValue (MultiValue returnValue, IOperation opera
341341
return;
342342

343343
if (method.ReturnType.IsTypeInterestingForDataflow ()) {
344-
var returnParameter = new MethodReturnValue (method);
344+
var returnParameter = new MethodReturnValue (method, isNewObj: false);
345345

346346
TrimAnalysisPatterns.Add (
347347
new TrimAnalysisAssignmentPattern (returnValue, returnParameter, operation, OwningSymbol, featureContext),

‎src/tools/illink/src/ILLink.Shared/TrimAnalysis/FlowAnnotations.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ partial class FlowAnnotations
1919
{
2020
internal partial bool MethodRequiresDataFlowAnalysis (MethodProxy method);
2121

22-
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes);
22+
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes);
2323

24-
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method);
24+
internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj);
2525

2626
internal partial GenericParameterValue GetGenericParameterValue (GenericParameterProxy genericParameter);
2727

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ internal partial struct HandleCallAction
2626
private readonly DiagnosticContext _diagnosticContext;
2727
private readonly FlowAnnotations _annotations;
2828
private readonly RequireDynamicallyAccessedMembersAction _requireDynamicallyAccessedMembersAction;
29+
private readonly bool _isNewObj;
2930

3031
public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnlyList<MultiValue> argumentValues, IntrinsicId intrinsicId, out MultiValue methodReturnValue)
3132
{
@@ -37,11 +38,12 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl
3738

3839
// As a convenience, if the code above didn't set the return value (and the method has a return value),
3940
// we will set it to be an unknown value with the return type of the method.
40-
var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod);
41+
var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod, _isNewObj);
4142
bool returnsVoid = calledMethod.ReturnsVoid ();
42-
methodReturnValue = maybeMethodReturnValue ?? (returnsVoid ?
43-
MultiValueLattice.Top :
44-
annotatedMethodReturnValue);
43+
methodReturnValue = maybeMethodReturnValue ??
44+
((returnsVoid && !_isNewObj)
45+
? MultiValueLattice.Top
46+
: annotatedMethodReturnValue);
4547

4648
// Validate that the return value has the correct annotations as per the method return value annotations
4749
if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0) {
@@ -80,7 +82,7 @@ bool TryHandleSharedIntrinsic (
8082
MultiValue? returnValue = methodReturnValue = null;
8183

8284
bool requiresDataFlowAnalysis = _annotations.MethodRequiresDataFlowAnalysis (calledMethod);
83-
var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod);
85+
var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod, _isNewObj);
8486
Debug.Assert (requiresDataFlowAnalysis || annotatedMethodReturnValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None);
8587

8688
switch (intrinsicId) {
@@ -237,7 +239,7 @@ GenericParameterValue genericParam
237239
&& valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.All)
238240
returnMemberTypes = DynamicallyAccessedMemberTypes.All;
239241

240-
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, returnMemberTypes));
242+
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, returnMemberTypes));
241243
}
242244
}
243245
}
@@ -544,7 +546,7 @@ GenericParameterValue genericParam
544546

545547
// We only applied the annotation based on binding flags, so we will keep the necessary types
546548
// but we will not keep anything on them. So the return value has no known annotations on it
547-
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, DynamicallyAccessedMemberTypes.None));
549+
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.None));
548550
}
549551
}
550552
} else if (value is NullValue) {
@@ -558,9 +560,9 @@ GenericParameterValue genericParam
558560
// since All applies recursively to all nested type (see MarkStep.MarkEntireType).
559561
// Otherwise we only mark the nested type itself, nothing on it, so the return value has no annotation on it.
560562
if (value is ValueWithDynamicallyAccessedMembers { DynamicallyAccessedMemberTypes: DynamicallyAccessedMemberTypes.All })
561-
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, DynamicallyAccessedMemberTypes.All));
563+
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.All));
562564
else
563-
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, DynamicallyAccessedMemberTypes.None));
565+
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.None));
564566
}
565567
}
566568
}
@@ -832,7 +834,7 @@ GenericParameterValue genericParam
832834
} else if (typeNameValue is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers && valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes != 0) {
833835
// Propagate the annotation from the type name to the return value. Annotation on a string value will be fulfilled whenever a value is assigned to the string with annotation.
834836
// So while we don't know which type it is, we can guarantee that it will fulfill the annotation.
835-
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes));
837+
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes));
836838
} else {
837839
_diagnosticContext.AddDiagnostic (DiagnosticId.UnrecognizedTypeNameInTypeGetType, calledMethod.GetDisplayName ());
838840
AddReturnValue (MultiValueLattice.Top);
@@ -956,7 +958,7 @@ GenericParameterValue genericParam
956958
propagatedMemberTypes |= DynamicallyAccessedMemberTypes.Interfaces;
957959
}
958960

959-
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, propagatedMemberTypes));
961+
AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, propagatedMemberTypes));
960962
} else if (value is SystemTypeValue systemTypeValue) {
961963
if (TryGetBaseType (systemTypeValue.RepresentedType, out var baseType))
962964
AddReturnValue (new SystemTypeValue (baseType.Value));

‎src/tools/illink/src/ILLink.Shared/TypeSystemProxy/MethodProxy.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ internal bool HasParameterOfType (ParameterIndex parameterIndex, string fullType
5656
internal partial bool HasGenericParameters ();
5757
internal partial bool HasGenericParametersCount (int genericParameterCount);
5858
internal partial ImmutableArray<GenericParameterProxy> GetGenericParameters ();
59+
internal partial bool IsConstructor ();
5960
internal partial bool IsStatic ();
6061
internal partial bool HasImplicitThis ();
6162
internal partial bool ReturnsVoid ();

0 commit comments

Comments
 (0)