Make flow analysis more conservative in presence of entities that can point to multiple different objects #6794
Conversation
… point to multiple different objects Fixes dotnet#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.
| literal = predicate.Left; | ||
| } | ||
|
|
||
| if (identifier.TypePrimitive != PredicateTypePrimitive.F1) |
There was a problem hiding this comment.
The core issue here was that we were correctly deducing that identifer and literal can point to the exact same set of two potential runtime objects stored in predicate.Left and predicate.Right. However, the value deduction from predicate in this line identifier.TypePrimitive != PredicateTypePrimitive.F1 shouldn't be applied to literal.TypePrimitive, which was the case prior to this fix. Now, for cases where an entity's instance location can point to more than one runtime location, we also store the entity for this instance location, i.e. entity for identifier in this case, in the analysis entity created for identifier.TypePrimitive. This allows us to avoid applying value deduction for identifier.TypePrimitive to literal.TypePrimitive.
| return predicate; | ||
| } | ||
|
|
||
| if (identifier.TypePrimitive == PredicateTypePrimitive.F2) |
There was a problem hiding this comment.
We are still able to successfully preserve the value deduction for identifier.TypePrimitive from the previous if statement after this fix.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6794 +/- ##
========================================
Coverage 96.34% 96.34%
========================================
Files 1386 1386
Lines 326057 326367 +310
Branches 10721 10733 +12
========================================
+ Hits 314141 314454 +313
- Misses 9210 9211 +1
+ Partials 2706 2702 -4 |
|
@Youssef1313 are you able to help with a quick review here? Thanks! |
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.