Skip to content

Commit fe3f59b

Browse files
MichalStrehovskyMichalStrehovsky
authored
Fix missed places in dataflow analysis that introduce genericness (#118020)
1 parent 913c282 commit fe3f59b

File tree

6 files changed

+97
-6
lines changed

6 files changed

+97
-6
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -592,17 +592,22 @@ protected virtual void Scan(MethodIL methodBody, ref InterproceduralState interp
592592
case ILOpcode.conv_r8:
593593
case ILOpcode.ldind_ref:
594594
case ILOpcode.ldobj:
595-
case ILOpcode.mkrefany:
596595
case ILOpcode.unbox:
597596
case ILOpcode.unbox_any:
598-
case ILOpcode.box:
599597
case ILOpcode.neg:
600598
case ILOpcode.not:
601599
PopUnknown(currentStack, 1, methodBody, offset);
602600
PushUnknown(currentStack);
603601
reader.Skip(opcode);
604602
break;
605603

604+
case ILOpcode.box:
605+
case ILOpcode.mkrefany:
606+
HandleTypeTokenAccess(methodBody, offset, (TypeDesc)methodBody.GetObject(reader.ReadILToken()));
607+
PopUnknown(currentStack, 1, methodBody, offset);
608+
PushUnknown(currentStack);
609+
break;
610+
606611
case ILOpcode.isinst:
607612
case ILOpcode.castclass:
608613
// We can consider a NOP because the value doesn't change.
@@ -622,6 +627,7 @@ protected virtual void Scan(MethodIL methodBody, ref InterproceduralState interp
622627
{
623628
StackSlot count = PopUnknown(currentStack, 1, methodBody, offset);
624629
var arrayElement = (TypeDesc)methodBody.GetObject(reader.ReadILToken());
630+
HandleTypeTokenAccess(methodBody, offset, arrayElement);
625631
currentStack.Push(new StackSlot(ArrayValue.Create(count.Value, arrayElement)));
626632
}
627633
break;

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ public static bool RequiresReflectionMethodBodyScannerForAccess(FlowAnnotations
6060
field.DoesFieldRequire(DiagnosticUtilities.RequiresDynamicCodeAttribute, out _);
6161
}
6262

63+
public static bool RequiresReflectionMethodBodyScannerForAccess(FlowAnnotations flowAnnotations, TypeDesc type)
64+
{
65+
return GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(flowAnnotations, type) ||
66+
type.DoesTypeRequire(DiagnosticUtilities.RequiresUnreferencedCodeAttribute, out _) ||
67+
type.DoesTypeRequire(DiagnosticUtilities.RequiresAssemblyFilesAttribute, out _) ||
68+
type.DoesTypeRequire(DiagnosticUtilities.RequiresDynamicCodeAttribute, out _);
69+
}
70+
6371
internal static void CheckAndReportAllRequires(in DiagnosticContext diagnosticContext, TypeSystemEntity calledMember)
6472
{
6573
CheckAndReportRequires(diagnosticContext, calledMember, DiagnosticUtilities.RequiresUnreferencedCodeAttribute);
@@ -429,10 +437,10 @@ private static bool IsComInterop(MarshalAsDescriptor? marshalInfoProvider, TypeD
429437

430438
private void ProcessGenericArgumentDataFlow(MethodDesc method)
431439
{
432-
// We only need to validate static methods and then all generic methods
433-
// Instance non-generic methods don't need validation because the creation of the instance
434-
// is the place where the validation will happen.
435-
if (!method.Signature.IsStatic && !method.HasInstantiation && !method.IsConstructor)
440+
// We mostly need to validate static methods and generic methods
441+
// Instance non-generic methods on reference types don't need validation
442+
// because the creation of the instance is the place where the validation will happen.
443+
if (!method.Signature.IsStatic && !method.HasInstantiation && !method.IsConstructor && !method.OwningType.IsValueType)
436444
return;
437445

438446
if (GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(_annotations, method))

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,6 +1240,10 @@ public bool CanGenerateMetadata(FieldDesc field)
12401240
protected abstract MetadataCategory GetMetadataCategory(TypeDesc type);
12411241
protected abstract MetadataCategory GetMetadataCategory(FieldDesc field);
12421242

1243+
public virtual void GetDependenciesDueToAccess(ref DependencyList dependencies, NodeFactory factory, MethodIL methodIL, TypeDesc accessedType)
1244+
{
1245+
}
1246+
12431247
public virtual void GetDependenciesDueToAccess(ref DependencyList dependencies, NodeFactory factory, MethodIL methodIL, MethodDesc calledMethod)
12441248
{
12451249
}

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,15 @@ public override void GetDependenciesDueToAccess(ref DependencyList dependencies,
773773
}
774774
}
775775

776+
public override void GetDependenciesDueToAccess(ref DependencyList dependencies, NodeFactory factory, MethodIL methodIL, TypeDesc accessedType)
777+
{
778+
bool scanReflection = (_generationOptions & UsageBasedMetadataGenerationOptions.ReflectionILScanning) != 0;
779+
if (scanReflection && Dataflow.ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForAccess(FlowAnnotations, accessedType))
780+
{
781+
AddDataflowDependency(ref dependencies, factory, methodIL, "Access to interesting type");
782+
}
783+
}
784+
776785
public override void GetDependenciesDueToAccess(ref DependencyList dependencies, NodeFactory factory, MethodIL methodIL, MethodDesc calledMethod)
777786
{
778787
bool scanReflection = (_generationOptions & UsageBasedMetadataGenerationOptions.ReflectionILScanning) != 0;

‎src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,7 @@ private void ImportMkRefAny(int token)
921921
{
922922
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.TypeHandleToRuntimeType), "mkrefany");
923923
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.TypeHandleToRuntimeTypeHandle), "mkrefany");
924+
_factory.MetadataManager.GetDependenciesDueToAccess(ref _dependencies, _factory, _methodIL, (TypeDesc)_canonMethodIL.GetObject(token));
924925
ImportTypedRefOperationDependencies(token, "mkrefany");
925926
}
926927

@@ -960,6 +961,8 @@ private void ImportLdToken(int token)
960961
}
961962
}
962963

964+
_factory.MetadataManager.GetDependenciesDueToAccess(ref _dependencies, _factory, _methodIL, (TypeDesc)_canonMethodIL.GetObject(token));
965+
963966
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.GetRuntimeTypeHandle), "ldtoken");
964967
_dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.GetRuntimeType), "ldtoken");
965968

@@ -1190,6 +1193,9 @@ private void AddBoxingDependencies(TypeDesc type, string reason)
11901193
if (!type.IsValueType)
11911194
return;
11921195

1196+
TypeDesc typeForAccessCheck = type.IsRuntimeDeterminedSubtype ? type.ConvertToCanonForm(CanonicalFormKind.Specific) : type;
1197+
_factory.MetadataManager.GetDependenciesDueToAccess(ref _dependencies, _factory, _methodIL, typeForAccessCheck);
1198+
11931199
if (type.IsRuntimeDeterminedSubtype)
11941200
{
11951201
_dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.TypeHandle, type), reason);
@@ -1217,6 +1223,7 @@ private void ImportLeave(BasicBlock target)
12171223
private void ImportNewArray(int token)
12181224
{
12191225
var elementType = (TypeDesc)_methodIL.GetObject(token);
1226+
_factory.MetadataManager.GetDependenciesDueToAccess(ref _dependencies, _factory, _methodIL, (TypeDesc)_canonMethodIL.GetObject(token));
12201227
if (elementType.IsRuntimeDeterminedSubtype)
12211228
{
12221229
_dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.TypeHandle, elementType.MakeArrayType()), "newarr");

‎src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ public static void Main()
4949
TestNoWarningsInRUCMethod<TestType>();
5050
TestNoWarningsInRUCType<TestType, TestType>();
5151
TestGenericParameterFlowsToNestedType.Test();
52+
53+
TestInstanceMethodOnValueType<object>();
54+
TestValueTypeBox<object>();
55+
TestMkrefAny<object>();
56+
TestInArray<object>();
5257
}
5358

5459
static void TestSingleGenericParameterOnType()
@@ -851,6 +856,58 @@ static void TestNoWarningsInRUCType<T, U>()
851856
rucType.VirtualMethodRequiresPublicMethods<T>();
852857
}
853858

859+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")]
860+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")]
861+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")]
862+
static void TestInstanceMethodOnValueType<T>()
863+
{
864+
default(RequiresParameterlessCtor<T>).Do();
865+
}
866+
867+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")]
868+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")]
869+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")]
870+
[ExpectedWarning("IL2091", "IRequireParameterlessCtor", Tool.Trimmer, "")]
871+
[ExpectedWarning("IL2091", "IRequireParameterlessCtor", Tool.Trimmer, "")]
872+
static void TestValueTypeBox<T>()
873+
{
874+
if (default(RequiresParameterlessCtor<T>) is IRequireParameterlessCtor<T> i)
875+
{
876+
i.Do();
877+
}
878+
}
879+
880+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")]
881+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")]
882+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")]
883+
static void TestMkrefAny<T>()
884+
{
885+
RequiresParameterlessCtor<T> val = default;
886+
TypedReference tr = __makeref(val);
887+
// This is a potential box operation, e.g. TypedReference.ToObject(tr);
888+
}
889+
890+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")]
891+
[ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")]
892+
static void TestInArray<T>()
893+
{
894+
var arr = new RequiresParameterlessCtor<T>[1];
895+
// This is a potential box operation, e.g. arr.GetValue(0)
896+
}
897+
898+
interface IRequireParameterlessCtor<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T>
899+
{
900+
T Do();
901+
}
902+
903+
struct RequiresParameterlessCtor<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T> : IRequireParameterlessCtor<T>
904+
{
905+
public T Do()
906+
{
907+
return Activator.CreateInstance<T>();
908+
}
909+
}
910+
854911
class TestGenericParameterFlowsToNestedType
855912
{
856913
class Generic<T>

0 commit comments

Comments
 (0)