Skip to content

Commit 882cea6

Browse files
committed
Make flow analysis more conservative in presence of entities that can point to multiple different objects
Fixes #6520 Now we store an additional optional EntityForInstanceLocation for analysis entities that can point to more than one possible locations across all control flow paths. This is required to distinguis from other entities that can point to the same set of potential locations, but the actual runtime value of the pointed to location may not be the same for both these entities.
1 parent 393efcc commit 882cea6

File tree

4 files changed

+56
-18
lines changed

4 files changed

+56
-18
lines changed

src/Microsoft.CodeAnalysis.AnalyzerUtilities/PublicAPI.Unshipped.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractAnalysisDomain<TAnalysisDat
3838
Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractBlockAnalysisResult
3939
Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractBlockAnalysisResult.AbstractBlockAnalysisResult(Microsoft.CodeAnalysis.FlowAnalysis.BasicBlock! basicBlock) -> void
4040
Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractBlockAnalysisResult.BasicBlock.get -> Microsoft.CodeAnalysis.FlowAnalysis.BasicBlock!
41+
Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity.EntityForInstanceLocation.get -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity?
4142
Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityFactory.TryGetCopyValueForFlowCapture(Microsoft.CodeAnalysis.FlowAnalysis.CaptureId captureId, out Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.CopyAnalysis.CopyAbstractValue! copyValue) -> bool
4243
Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.DataFlowAnalysisResult<TBlockAnalysisResult, TAbstractAnalysisValue>.LambdaAndLocalFunctionAnalysisInfo.get -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.LambdaAndLocalFunctionAnalysisInfo!
4344
Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.DataFlowOperationVisitor<TAnalysisData, TAnalysisContext, TAnalysisResult, TAbstractAnalysisValue>.StandaloneLocalFunctionAnalysisResultsMap.get -> System.Collections.Immutable.ImmutableDictionary<Microsoft.CodeAnalysis.IMethodSymbol!, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.IDataFlowAnalysisResult<TAbstractAnalysisValue>!>!
@@ -73,6 +74,7 @@ static Analyzer.Utilities.RoslynHashCode.Combine<T1, T2, T3, T4>(T1 value1, T2 v
7374
static Analyzer.Utilities.RoslynHashCode.Combine<T1, T2, T3>(T1 value1, T2 value2, T3 value3) -> int
7475
static Analyzer.Utilities.RoslynHashCode.Combine<T1, T2>(T1 value1, T2 value2) -> int
7576
static Analyzer.Utilities.RoslynHashCode.Combine<T1>(T1 value1) -> int
77+
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity.Create(Microsoft.CodeAnalysis.ISymbol? symbol, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractIndex!> indices, Microsoft.CodeAnalysis.ITypeSymbol! type, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAbstractValue! instanceLocation, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity? parent, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity? entityForInstanceLocation) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity!
7678
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.DisposeAnalysis.DisposeAnalysis.TryGetOrComputeResult(Microsoft.CodeAnalysis.FlowAnalysis.ControlFlowGraph! cfg, Microsoft.CodeAnalysis.ISymbol! owningSymbol, Analyzer.Utilities.WellKnownTypeProvider! wellKnownTypeProvider, Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions! analyzerOptions, Microsoft.CodeAnalysis.DiagnosticDescriptor! rule, System.Collections.Immutable.ImmutableHashSet<Microsoft.CodeAnalysis.INamedTypeSymbol!>! disposeOwnershipTransferLikelyTypes, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAnalysisKind defaultPointsToAnalysisKind, bool trackInstanceFields, bool exceptionPathsAnalysis, out Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAnalysisResult? pointsToAnalysisResult, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisKind interproceduralAnalysisKind = Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisKind.ContextSensitive, bool performCopyAnalysisIfNotUserConfigured = false, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisPredicate? interproceduralAnalysisPredicate = null, bool defaultDisposeOwnershipTransferAtConstructor = false, bool defaultDisposeOwnershipTransferAtMethodCall = false) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.DisposeAnalysis.DisposeAnalysisResult?
7779
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisConfiguration.Create(Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions! analyzerOptions, Microsoft.CodeAnalysis.DiagnosticDescriptor! rule, Microsoft.CodeAnalysis.FlowAnalysis.ControlFlowGraph! cfg, Microsoft.CodeAnalysis.Compilation! compilation, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisKind defaultInterproceduralAnalysisKind, uint defaultMaxInterproceduralMethodCallChain = 3, uint defaultMaxInterproceduralLambdaOrLocalFunctionCallChain = 3) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisConfiguration
7880
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisConfiguration.Create(Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions! analyzerOptions, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!> rules, Microsoft.CodeAnalysis.FlowAnalysis.ControlFlowGraph! cfg, Microsoft.CodeAnalysis.Compilation! compilation, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisKind defaultInterproceduralAnalysisKind, uint defaultMaxInterproceduralMethodCallChain = 3, uint defaultMaxInterproceduralLambdaOrLocalFunctionCallChain = 3) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralAnalysisConfiguration
@@ -655,7 +657,6 @@ static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractLocation.CreateThisO
655657
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractLocationDataFlowOperationVisitor<TAnalysisData, TAnalysisContext, TAnalysisResult, TAbstractAnalysisValue>.GetClonedAnalysisDataHelper(System.Collections.Generic.IDictionary<Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractLocation!, TAbstractAnalysisValue>! analysisData) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.DictionaryAnalysisData<Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractLocation!, TAbstractAnalysisValue>!
656658
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractLocationDataFlowOperationVisitor<TAnalysisData, TAnalysisContext, TAnalysisResult, TAbstractAnalysisValue>.GetEmptyAnalysisDataHelper() -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.DictionaryAnalysisData<Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractLocation!, TAbstractAnalysisValue>!
657659
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity.Create(Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.InterproceduralCaptureId interproceduralCaptureId, Microsoft.CodeAnalysis.ITypeSymbol! type, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAbstractValue! instanceLocation) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity!
658-
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity.Create(Microsoft.CodeAnalysis.ISymbol? symbol, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractIndex!> indices, Microsoft.CodeAnalysis.ITypeSymbol! type, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAbstractValue! instanceLocation, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity? parent) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity!
659660
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity.Create(Microsoft.CodeAnalysis.Operations.IInstanceReferenceOperation! instanceReferenceOperation, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAbstractValue! instanceLocation) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity!
660661
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity.CreateThisOrMeInstance(Microsoft.CodeAnalysis.INamedTypeSymbol! typeSymbol, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAbstractValue! instanceLocation) -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity!
661662
static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityDataFlowOperationVisitor<TAnalysisData, TAnalysisContext, TAnalysisResult, TAbstractAnalysisValue>.IsChildAnalysisEntity(Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity! entity, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity! ancestorEntity) -> bool

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/GlobalFlowStateAnalysis/GlobalFlowStateDataFlowOperationVisitor.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ private static AnalysisEntity GetGlobalEntity(TAnalysisContext analysisContext)
6060
ImmutableArray<AbstractIndex>.Empty,
6161
owningSymbol.GetMemberOrLocalOrParameterType()!,
6262
instanceLocation: PointsToAbstractValue.Unknown,
63-
parent: null);
63+
parent: null,
64+
entityForInstanceLocation: null);
6465
}
6566

6667
public sealed override DictionaryAnalysisData<AnalysisEntity, TAbstractAnalysisValue> Flow(

src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow
2626
/// 1. An associated non-null <see cref="Type"/> and
2727
/// 2. A non-null <see cref="InstanceLocation"/> indicating the abstract location at which the entity is located and
2828
/// 3. An optional parent key if this key has the same <see cref="InstanceLocation"/> as the parent (i.e. parent is a value type).
29+
/// 4. An optional entity for reference typed <see cref="InstanceLocation"/> if the points to value for the instance location has more than one possible locations.
2930
/// </para>
3031
/// </summary>
3132
public sealed class AnalysisEntity : CacheBasedEquatable<AnalysisEntity>
@@ -40,11 +41,14 @@ private AnalysisEntity(
4041
PointsToAbstractValue location,
4142
ITypeSymbol type,
4243
AnalysisEntity? parent,
44+
AnalysisEntity? entityForInstanceLocation,
4345
bool isThisOrMeInstance)
4446
{
4547
Debug.Assert(!indices.IsDefault);
4648
Debug.Assert(symbol != null || !indices.IsEmpty || instanceReferenceOperationSyntax != null || captureId.HasValue);
4749
Debug.Assert(parent == null || parent.Type.HasValueCopySemantics() || !indices.IsEmpty);
50+
Debug.Assert(entityForInstanceLocation == null || parent == null);
51+
Debug.Assert(entityForInstanceLocation == null || location.Kind == PointsToAbstractValueKind.KnownLocations);
4852

4953
Symbol = symbol;
5054
Indices = indices;
@@ -53,44 +57,46 @@ private AnalysisEntity(
5357
InstanceLocation = location;
5458
Type = type;
5559
Parent = parent;
60+
EntityForInstanceLocation = entityForInstanceLocation;
5661
IsThisOrMeInstance = isThisOrMeInstance;
5762

5863
_ignoringLocationHashCode = ComputeIgnoringLocationHashCode();
5964
EqualsIgnoringInstanceLocationId = _ignoringLocationHashCode;
6065
}
6166

62-
private AnalysisEntity(ISymbol? symbol, ImmutableArray<AbstractIndex> indices, PointsToAbstractValue location, ITypeSymbol type, AnalysisEntity? parent)
63-
: this(symbol, indices, instanceReferenceOperationSyntax: null, captureId: null, location: location, type: type, parent: parent, isThisOrMeInstance: false)
67+
private AnalysisEntity(ISymbol? symbol, ImmutableArray<AbstractIndex> indices, PointsToAbstractValue location, ITypeSymbol type, AnalysisEntity? parent, AnalysisEntity? entityForInstanceLocation)
68+
: this(symbol, indices, instanceReferenceOperationSyntax: null, captureId: null, location: location, type: type, parent: parent, entityForInstanceLocation: entityForInstanceLocation, isThisOrMeInstance: false)
6469
{
6570
Debug.Assert(symbol != null || !indices.IsEmpty);
6671
}
6772

6873
private AnalysisEntity(IInstanceReferenceOperation instanceReferenceOperation, PointsToAbstractValue location)
6974
: this(symbol: null, indices: ImmutableArray<AbstractIndex>.Empty, instanceReferenceOperationSyntax: instanceReferenceOperation.Syntax,
70-
captureId: null, location: location, type: instanceReferenceOperation.Type!, parent: null, isThisOrMeInstance: false)
75+
captureId: null, location: location, type: instanceReferenceOperation.Type!, parent: null, entityForInstanceLocation: null, isThisOrMeInstance: false)
7176
{
7277
Debug.Assert(instanceReferenceOperation != null);
7378
}
7479

7580
private AnalysisEntity(InterproceduralCaptureId captureId, ITypeSymbol capturedType, PointsToAbstractValue location)
7681
: this(symbol: null, indices: ImmutableArray<AbstractIndex>.Empty, instanceReferenceOperationSyntax: null,
77-
captureId: captureId, location: location, type: capturedType, parent: null, isThisOrMeInstance: false)
82+
captureId: captureId, location: location, type: capturedType, parent: null, entityForInstanceLocation: null, isThisOrMeInstance: false)
7883
{
7984
}
8085

8186
private AnalysisEntity(INamedTypeSymbol namedType, PointsToAbstractValue location, bool isThisOrMeInstance)
8287
: this(symbol: namedType, indices: ImmutableArray<AbstractIndex>.Empty, instanceReferenceOperationSyntax: null,
83-
captureId: null, location: location, type: namedType, parent: null, isThisOrMeInstance: isThisOrMeInstance)
88+
captureId: null, location: location, type: namedType, parent: null, entityForInstanceLocation: null, isThisOrMeInstance: isThisOrMeInstance)
8489
{
8590
}
8691

8792
public static AnalysisEntity Create(ISymbol? symbol, ImmutableArray<AbstractIndex> indices,
88-
ITypeSymbol type, PointsToAbstractValue instanceLocation, AnalysisEntity? parent)
93+
ITypeSymbol type, PointsToAbstractValue instanceLocation, AnalysisEntity? parent, AnalysisEntity? entityForInstanceLocation)
8994
{
9095
Debug.Assert(symbol != null || !indices.IsEmpty);
9196
Debug.Assert(parent == null || parent.InstanceLocation == instanceLocation);
97+
Debug.Assert(entityForInstanceLocation == null || instanceLocation.Kind == PointsToAbstractValueKind.KnownLocations);
9298

93-
return new AnalysisEntity(symbol, indices, instanceLocation, type, parent);
99+
return new AnalysisEntity(symbol, indices, instanceLocation, type, parent, entityForInstanceLocation);
94100
}
95101

96102
public static AnalysisEntity Create(IInstanceReferenceOperation instanceReferenceOperation, PointsToAbstractValue instanceLocation)
@@ -121,7 +127,7 @@ public AnalysisEntity WithMergedInstanceLocation(AnalysisEntity analysisEntityTo
121127
Debug.Assert(!InstanceLocation.Equals(analysisEntityToMerge.InstanceLocation));
122128

123129
var mergedInstanceLocation = PointsToAnalysis.PointsToAnalysis.ValueDomainInstance.Merge(InstanceLocation, analysisEntityToMerge.InstanceLocation);
124-
return new AnalysisEntity(Symbol, Indices, InstanceReferenceOperationSyntax, CaptureId, mergedInstanceLocation, Type, Parent, IsThisOrMeInstance);
130+
return new AnalysisEntity(Symbol, Indices, InstanceReferenceOperationSyntax, CaptureId, mergedInstanceLocation, Type, Parent, EntityForInstanceLocation, IsThisOrMeInstance);
125131
}
126132

127133
public bool IsChildOrInstanceMember
@@ -190,6 +196,7 @@ internal bool ShouldBeTrackedForAnalysis(bool hasCompletePointsToAnalysisResult)
190196
public SyntaxNode? InstanceReferenceOperationSyntax { get; }
191197
public InterproceduralCaptureId? CaptureId { get; }
192198
public PointsToAbstractValue InstanceLocation { get; }
199+
public AnalysisEntity? EntityForInstanceLocation { get; }
193200
public ITypeSymbol Type { get; }
194201
public AnalysisEntity? Parent { get; }
195202
public bool IsThisOrMeInstance { get; }
@@ -205,7 +212,7 @@ or PointsToAbstractValueKind.UnknownNull
205212
public bool IsLValueFlowCaptureEntity => CaptureId.HasValue && CaptureId.Value.IsLValueFlowCapture;
206213

207214
internal AnalysisEntity WithIndices(ImmutableArray<AbstractIndex> indices)
208-
=> new(Symbol, indices, InstanceReferenceOperationSyntax, CaptureId, InstanceLocation, Type, Parent, IsThisOrMeInstance);
215+
=> new(Symbol, indices, InstanceReferenceOperationSyntax, CaptureId, InstanceLocation, Type, Parent, EntityForInstanceLocation, IsThisOrMeInstance);
209216

210217
public bool EqualsIgnoringInstanceLocation(AnalysisEntity? other)
211218
{
@@ -228,6 +235,7 @@ public bool EqualsIgnoringInstanceLocation(AnalysisEntity? other)
228235
&& CaptureId.GetHashCodeOrDefault() == other.CaptureId.GetHashCodeOrDefault()
229236
&& Type.GetHashCodeOrDefault() == other.Type.GetHashCodeOrDefault()
230237
&& Parent.GetHashCodeOrDefault() == other.Parent.GetHashCodeOrDefault()
238+
&& EntityForInstanceLocation.GetHashCodeOrDefault() == other.EntityForInstanceLocation.GetHashCodeOrDefault()
231239
&& IsThisOrMeInstance.GetHashCode() == other.IsThisOrMeInstance.GetHashCode();
232240
}
233241

@@ -254,6 +262,7 @@ private void ComputeHashCodePartsIgnoringLocation(ref RoslynHashCode hashCode)
254262
hashCode.Add(CaptureId.GetHashCodeOrDefault());
255263
hashCode.Add(Type.GetHashCode());
256264
hashCode.Add(Parent.GetHashCodeOrDefault());
265+
hashCode.Add(EntityForInstanceLocation.GetHashCodeOrDefault());
257266
hashCode.Add(IsThisOrMeInstance.GetHashCode());
258267
}
259268

0 commit comments

Comments
 (0)