Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[release/8.0-staging] Fix ILLink/ILC hang when tracking too many hoisted local values #95302

Merged
merged 1 commit into from
Jan 2, 2024
Merged

[release/8.0-staging] Fix ILLink/ILC hang when tracking too many hoisted local values #95302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ArrayValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal partial record ArrayValue
public static MultiValue Create(MultiValue size, TypeDesc elementType)
{
MultiValue result = MultiValueLattice.Top;
foreach (var sizeValue in size)
foreach (var sizeValue in size.AsEnumerable ())
{
result = MultiValueLattice.Meet(result, new MultiValue(new ArrayValue(sizeValue, elementType)));
}
Expand Down Expand Up @@ -92,7 +92,7 @@ public override SingleValue DeepCopy()
// Since it's possible to store a reference to array as one of its own elements
// simple deep copy could lead to endless recursion.
// So instead we simply disallow arrays as element values completely - and treat that case as "too complex to analyze".
foreach (SingleValue v in kvp.Value.Value)
foreach (SingleValue v in kvp.Value.Value.AsEnumerable ())
{
System.Diagnostics.Debug.Assert(v is not ArrayValue);
}
Expand Down Expand Up @@ -123,7 +123,7 @@ public override string ToString()
result.Append(element.Key);
result.Append(",(");
bool firstValue = true;
foreach (var v in element.Value.Value)
foreach (var v in element.Value.Value.AsEnumerable ())
{
if (firstValue)
{
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/InterproceduralState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public void TrackMethod(MethodIL methodBody)
methodBody = GetInstantiatedMethodIL(methodBody);

// Work around the fact that ValueSet is readonly
var methodsList = new List<MethodBodyValue>(MethodBodies);
Debug.Assert (!MethodBodies.IsUnknown ());
var methodsList = new List<MethodBodyValue>(MethodBodies.GetKnownValues ());
methodsList.Add(new MethodBodyValue(methodBody));

// For state machine methods, also scan the state machine members.
Expand Down
18 changes: 10 additions & 8 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ private static void ValidateNoReferenceToReference(ValueBasicBlockPair?[] locals
continue;

MultiValue localValue = localVariable.Value.Value;
foreach (var val in localValue)
foreach (var val in localValue.AsEnumerable ())
{
if (val is LocalVariableReferenceValue localReference && localReference.ReferencedType.IsByRefOrPointer())
{
Expand Down Expand Up @@ -309,7 +309,8 @@ public virtual void InterproceduralScan(MethodIL startingMethodBody)

// Flow state through all methods encountered so far, as long as there
// are changes discovered in the hoisted local state on entry to any method.
foreach (var methodBodyValue in oldInterproceduralState.MethodBodies)
Debug.Assert (!oldInterproceduralState.MethodBodies.IsUnknown ());
foreach (var methodBodyValue in oldInterproceduralState.MethodBodies.GetKnownValues ())
Scan(methodBodyValue.MethodBody, ref interproceduralState);
}

Expand All @@ -327,7 +328,8 @@ public virtual void InterproceduralScan(MethodIL startingMethodBody)
}
else
{
Debug.Assert(interproceduralState.MethodBodies.Count() == 1);
Debug.Assert (!interproceduralState.MethodBodies.IsUnknown ());
Debug.Assert(interproceduralState.MethodBodies.GetKnownValues ().Count() == 1);
}
#endif
}
Expand Down Expand Up @@ -1018,7 +1020,7 @@ private void ScanIndirectStore(
/// <exception cref="LinkerFatalErrorException">Throws if <paramref name="target"/> is not a valid target for an indirect store.</exception>
protected void StoreInReference(MultiValue target, MultiValue source, MethodIL method, int offset, ValueBasicBlockPair?[] locals, int curBasicBlock, ref InterproceduralState ipState)
{
foreach (var value in target)
foreach (var value in target.AsEnumerable ())
{
switch (value)
{
Expand Down Expand Up @@ -1137,7 +1139,7 @@ private void ScanStfld(
return;
}

foreach (var value in HandleGetField(methodBody, offset, field))
foreach (var value in HandleGetField(methodBody, offset, field).AsEnumerable ())
{
// GetFieldValue may return different node types, in which case they can't be stored to.
// At least not yet.
Expand Down Expand Up @@ -1189,7 +1191,7 @@ internal MultiValue DereferenceValue(
ref InterproceduralState interproceduralState)
{
MultiValue dereferencedValue = MultiValueLattice.Top;
foreach (var value in maybeReferenceValue)
foreach (var value in maybeReferenceValue.AsEnumerable ())
{
switch (value)
{
Expand Down Expand Up @@ -1324,7 +1326,7 @@ private void HandleCall(

foreach (var param in methodArguments)
{
foreach (var v in param)
foreach (var v in param.AsEnumerable ())
{
if (v is ArrayValue arr)
{
Expand Down Expand Up @@ -1366,7 +1368,7 @@ private void ScanStelem(
StackSlot indexToStoreAt = PopUnknown(currentStack, 1, methodBody, offset);
StackSlot arrayToStoreIn = PopUnknown(currentStack, 1, methodBody, offset);
int? indexToStoreAtInt = indexToStoreAt.Value.AsConstInt();
foreach (var array in arrayToStoreIn.Value)
foreach (var array in arrayToStoreIn.Value.AsEnumerable ())
{
if (array is ArrayValue arrValue)
{
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ public static bool HandleCall(
// type instead).
//
// At least until we have shared enum code, this needs extra handling to get it right.
foreach (var value in argumentValues[0])
foreach (var value in argumentValues[0].AsEnumerable ())
{
if (value is SystemTypeValue systemTypeValue
&& !systemTypeValue.RepresentedType.Type.IsGenericDefinition
Expand Down Expand Up @@ -466,7 +466,7 @@ public static bool HandleCall(
? 0 : 1;

// We need the data to do struct marshalling.
foreach (var value in argumentValues[paramIndex])
foreach (var value in argumentValues[paramIndex].AsEnumerable ())
{
if (value is SystemTypeValue systemTypeValue
&& !systemTypeValue.RepresentedType.Type.IsGenericDefinition
Expand Down Expand Up @@ -497,7 +497,7 @@ public static bool HandleCall(
case IntrinsicId.Marshal_GetDelegateForFunctionPointer:
{
// We need the data to do delegate marshalling.
foreach (var value in argumentValues[1])
foreach (var value in argumentValues[1].AsEnumerable ())
{
if (value is SystemTypeValue systemTypeValue
&& !systemTypeValue.RepresentedType.Type.IsGenericDefinition
Expand All @@ -521,7 +521,7 @@ public static bool HandleCall(
//
case IntrinsicId.Object_GetType:
{
foreach (var valueNode in instanceValue)
foreach (var valueNode in instanceValue.AsEnumerable ())
{
// Note that valueNode can be statically typed in IL as some generic argument type.
// For example:
Expand Down Expand Up @@ -619,7 +619,7 @@ public static bool HandleCall(
// Validate that the return value has the correct annotations as per the method return value annotations
if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0)
{
foreach (var uniqueValue in methodReturnValue)
foreach (var uniqueValue in methodReturnValue.AsEnumerable ())
{
if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes)
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisAssignmentPattern.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ public void MarkAndProduceDiagnostics(ReflectionMarker reflectionMarker, Logger
logger.ShouldSuppressAnalysisWarningsForRequires(Origin.MemberDefinition, DiagnosticUtilities.RequiresAssemblyFilesAttribute),
logger);

foreach (var sourceValue in Source)
foreach (var sourceValue in Source.AsEnumerable ())
{
foreach (var targetValue in Target)
foreach (var targetValue in Target.AsEnumerable ())
{
if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers)
throw new NotImplementedException();
Expand Down
4 changes: 3 additions & 1 deletion src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/InterproceduralState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using ILLink.Shared.DataFlow;

Expand Down Expand Up @@ -50,7 +51,8 @@ public InterproceduralState<TValue, TValueLattice> Clone ()

public void TrackMethod (MethodBodyValue method)
{
var methodsList = new List<MethodBodyValue> (Methods);
Debug.Assert (!Methods.IsUnknown ());
var methodsList = new List<MethodBodyValue> (Methods.GetKnownValues ());
methodsList.Add (method);
Methods = new ValueSet<MethodBodyValue> (methodsList);
}
Expand Down
3 changes: 2 additions & 1 deletion src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public void InterproceduralAnalyze ()
while (!interproceduralState.Equals (oldInterproceduralState)) {
oldInterproceduralState = interproceduralState.Clone ();

foreach (var method in oldInterproceduralState.Methods) {
Debug.Assert (!oldInterproceduralState.Methods.IsUnknown ());
foreach (var method in oldInterproceduralState.Methods.GetKnownValues ()) {
if (method.Method.IsInRequiresUnreferencedCodeAttributeScope (out _))
continue;

Expand Down
10 changes: 6 additions & 4 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, ISimpleAssignm
// Single captured reference. Treat this as an overwriting assignment,
// unless the caller already told us to merge values because this is an
// assignment to one of multiple captured array element references.
var enumerator = capturedReferences.GetEnumerator ();
var enumerator = capturedReferences.GetKnownValues ().GetEnumerator ();
enumerator.MoveNext ();
var capture = enumerator.Current;
arrayRef = Visit (capture.Reference, state);
Expand All @@ -266,7 +266,8 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, ISimpleAssignm
// We treat this as possible write to each of the captured references,
// which requires merging with the previous values of each.

foreach (var capture in state.Current.CapturedReferences.Get (captureReference.Id)) {
Debug.Assert (!capturedReferences.IsUnknown ());
foreach (var capture in capturedReferences.GetKnownValues ()) {
arrayRef = Visit (capture.Reference, state);
HandleArrayElementWrite (arrayRef, index, value, operation, merge: true);
}
Expand Down Expand Up @@ -330,9 +331,10 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati
Debug.Assert (IsLValueFlowCapture (flowCaptureReference.Id));
Debug.Assert (!flowCaptureReference.GetValueUsageInfo (Method).HasFlag (ValueUsageInfo.Read));
var capturedReferences = state.Current.CapturedReferences.Get (flowCaptureReference.Id);
Debug.Assert (!capturedReferences.IsUnknown ());
if (!capturedReferences.HasMultipleValues) {
// Single captured reference. Treat this as an overwriting assignment.
var enumerator = capturedReferences.GetEnumerator ();
var enumerator = capturedReferences.GetKnownValues ().GetEnumerator ();
enumerator.MoveNext ();
targetOperation = enumerator.Current.Reference;
return ProcessSingleTargetAssignment (targetOperation, operation, state, merge: false);
Expand All @@ -349,7 +351,7 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati
// if the RHS has dataflow warnings.

TValue value = TopValue;
foreach (var capturedReference in capturedReferences) {
foreach (var capturedReference in capturedReferences.GetKnownValues ()) {
targetOperation = capturedReference.Reference;
var singleValue = ProcessSingleTargetAssignment (targetOperation, operation, state, merge: true);
value = LocalStateLattice.Lattice.ValueLattice.Meet (value, singleValue);
Expand Down
4 changes: 2 additions & 2 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ArrayValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal partial record ArrayValue
public static MultiValue Create (MultiValue size)
{
MultiValue result = MultiValueLattice.Top;
foreach (var sizeValue in size) {
foreach (var sizeValue in size.AsEnumerable ()) {
result = MultiValueLattice.Meet (result, new MultiValue (new ArrayValue (sizeValue)));
}

Expand Down Expand Up @@ -73,7 +73,7 @@ public override SingleValue DeepCopy ()
// Since it's possible to store a reference to array as one of its own elements
// simple deep copy could lead to endless recursion.
// So instead we simply disallow arrays as element values completely - and treat that case as "too complex to analyze".
foreach (SingleValue v in kvp.Value) {
foreach (SingleValue v in kvp.Value.AsEnumerable ()) {
System.Diagnostics.Debug.Assert (v is not ArrayValue);
}
#endif
Expand Down
4 changes: 2 additions & 2 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public TrimAnalysisAssignmentPattern Merge (ValueSetLattice<SingleValue> lattice
public IEnumerable<Diagnostic> CollectDiagnostics ()
{
var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ());
foreach (var sourceValue in Source) {
foreach (var targetValue in Target) {
foreach (var sourceValue in Source.AsEnumerable ()) {
foreach (var targetValue in Target.AsEnumerable ()) {
// The target should always be an annotated value, but the visitor design currently prevents
// declaring this in the type system.
if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers)
Expand Down
12 changes: 6 additions & 6 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public override MultiValue VisitArrayCreation (IArrayCreationOperation operation

var arrayValue = ArrayValue.Create (Visit (operation.DimensionSizes[0], state));
var elements = operation.Initializer?.ElementValues.Select (val => Visit (val, state)).ToArray () ?? System.Array.Empty<MultiValue> ();
foreach (var array in arrayValue.Cast<ArrayValue> ()) {
foreach (var array in arrayValue.AsEnumerable ().Cast<ArrayValue> ()) {
for (int i = 0; i < elements.Length; i++) {
array.IndexValues.Add (i, ArrayValue.SanitizeArrayElementValue(elements[i]));
}
Expand Down Expand Up @@ -155,11 +155,11 @@ operation.OperatorMethod is null &&
MultiValue rightValue = Visit (operation.RightOperand, argument);

MultiValue result = TopValue;
foreach (var left in leftValue) {
foreach (var left in leftValue.AsEnumerable ()) {
if (left is UnknownValue)
result = _multiValueLattice.Meet (result, left);
else if (left is ConstIntValue leftConstInt) {
foreach (var right in rightValue) {
foreach (var right in rightValue.AsEnumerable ()) {
if (right is UnknownValue)
result = _multiValueLattice.Meet (result, right);
else if (right is ConstIntValue rightConstInt) {
Expand Down Expand Up @@ -210,7 +210,7 @@ public override MultiValue HandleArrayElementRead (MultiValue arrayValue, MultiV
return UnknownValue.Instance;

MultiValue result = TopValue;
foreach (var value in arrayValue) {
foreach (var value in arrayValue.AsEnumerable ()) {
if (value is ArrayValue arr && arr.TryGetValueByIndex (index, out var elementValue))
result = _multiValueLattice.Meet (result, elementValue);
else
Expand All @@ -222,7 +222,7 @@ public override MultiValue HandleArrayElementRead (MultiValue arrayValue, MultiV
public override void HandleArrayElementWrite (MultiValue arrayValue, MultiValue indexValue, MultiValue valueToWrite, IOperation operation, bool merge)
{
int? index = indexValue.AsConstInt ();
foreach (var arraySingleValue in arrayValue) {
foreach (var arraySingleValue in arrayValue.AsEnumerable ()) {
if (arraySingleValue is ArrayValue arr) {
if (index == null) {
// Reset the array to all unknowns - since we don't know which index is being assigned
Expand Down Expand Up @@ -282,7 +282,7 @@ public override MultiValue HandleMethodCall (IMethodSymbol calledMethod, MultiVa
Method));

foreach (var argument in arguments) {
foreach (var argumentValue in argument) {
foreach (var argumentValue in argument.AsEnumerable ()) {
if (argumentValue is ArrayValue arrayValue)
arrayValue.IndexValues.Clear ();
}
Expand Down
Loading
Loading