Skip to content

Commit fa762d9

Browse files
hvitvedhvitved
committed
C#: Fix flow steps into phi nodes
- Add missing flow from post-update nodes into phi nodes. - Prevent flow from reads into phi nodes when use-use flow is prohibited.
1 parent 5b5dd07 commit fa762d9

File tree

4 files changed

+124
-39
lines changed

4 files changed

+124
-39
lines changed

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -310,12 +310,27 @@ module LocalFlow {
310310
* Holds if `nodeFrom` is a last node referencing SSA definition `def`, which
311311
* can reach `next`.
312312
*/
313-
private predicate localFlowSsaInput(Node nodeFrom, Ssa::Definition def, Ssa::Definition next) {
314-
exists(ControlFlow::BasicBlock bb, int i | SsaImpl::lastRefBeforeRedef(def, bb, i, next) |
313+
private predicate localFlowSsaInputFromDef(
314+
Node nodeFrom, Ssa::Definition def, Ssa::Definition next
315+
) {
316+
exists(ControlFlow::BasicBlock bb, int i |
317+
SsaImpl::lastRefBeforeRedef(def, bb, i, next) and
315318
def.definesAt(_, bb, i) and
316319
def = getSsaDefinition(nodeFrom)
317-
or
318-
nodeFrom.asExprAtNode(bb.getNode(i)) instanceof AssignableRead
320+
)
321+
}
322+
323+
/**
324+
* Holds if `read` is a last node reading SSA definition `def`, which
325+
* can reach `next`.
326+
*/
327+
predicate localFlowSsaInputFromExpr(
328+
ControlFlow::Node read, Ssa::Definition def, Ssa::Definition next
329+
) {
330+
exists(ControlFlow::BasicBlock bb, int i |
331+
SsaImpl::lastRefBeforeRedef(def, bb, i, next) and
332+
read = bb.getNode(i) and
333+
read.getElement() instanceof AssignableRead
319334
)
320335
}
321336

@@ -351,18 +366,14 @@ module LocalFlow {
351366
// Flow from read to next read
352367
localSsaFlowStepUseUse(def, nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
353368
or
354-
// Flow into phi node
355-
exists(Ssa::PhiNode phi |
356-
localFlowSsaInput(nodeFrom, def, phi) and
357-
phi = nodeTo.(SsaDefinitionNode).getDefinition() and
358-
def = phi.getAnInput()
359-
)
360-
or
361-
// Flow into uncertain SSA definition
362-
exists(LocalFlow::UncertainExplicitSsaDefinition uncertain |
363-
localFlowSsaInput(nodeFrom, def, uncertain) and
364-
uncertain = nodeTo.(SsaDefinitionNode).getDefinition() and
365-
def = uncertain.getPriorDefinition()
369+
// Flow into phi/uncertain SSA definition node from def
370+
exists(Ssa::Definition next |
371+
localFlowSsaInputFromDef(nodeFrom, def, next) and
372+
next = nodeTo.(SsaDefinitionNode).getDefinition()
373+
|
374+
def = next.(Ssa::PhiNode).getAnInput()
375+
or
376+
def = next.(LocalFlow::UncertainExplicitSsaDefinition).getPriorDefinition()
366377
)
367378
}
368379

@@ -519,11 +530,26 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
519530
or
520531
exists(Ssa::Definition def |
521532
LocalFlow::localSsaFlowStepUseUse(def, nodeFrom, nodeTo) and
522-
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom,
523-
any(DataFlowSummarizedCallable sc)) and
533+
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _) and
524534
not LocalFlow::usesInstanceField(def)
525535
)
526536
or
537+
// Flow into phi/uncertain SSA definition node from read
538+
exists(Ssa::Definition def, ControlFlow::Node read, Ssa::Definition next |
539+
LocalFlow::localFlowSsaInputFromExpr(read, def, next) and
540+
next = nodeTo.(SsaDefinitionNode).getDefinition() and
541+
def =
542+
[
543+
next.(Ssa::PhiNode).getAnInput(),
544+
next.(LocalFlow::UncertainExplicitSsaDefinition).getPriorDefinition()
545+
]
546+
|
547+
exists(nodeFrom.asExprAtNode(read)) and
548+
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
549+
or
550+
exists(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExprAtNode(read))
551+
)
552+
or
527553
LocalFlow::localFlowCapturedVarStep(nodeFrom, nodeTo)
528554
or
529555
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, true)
@@ -859,6 +885,21 @@ private module Cached {
859885
LocalFlow::usesInstanceField(def)
860886
)
861887
or
888+
// Flow into phi/uncertain SSA definition node from read
889+
exists(Ssa::Definition def, ControlFlow::Node read, Ssa::Definition next |
890+
LocalFlow::localFlowSsaInputFromExpr(read, def, next) and
891+
next = nodeTo.(SsaDefinitionNode).getDefinition() and
892+
def =
893+
[
894+
next.(Ssa::PhiNode).getAnInput(),
895+
next.(LocalFlow::UncertainExplicitSsaDefinition).getPriorDefinition()
896+
]
897+
|
898+
exists(nodeFrom.asExprAtNode(read))
899+
or
900+
exists(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExprAtNode(read))
901+
)
902+
or
862903
// Simple flow through library code is included in the exposed local
863904
// step relation, even though flow is technically inter-procedural
864905
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo,

‎csharp/ql/test/library-tests/dataflow/fields/FieldFlow.expected

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
failures
2-
| J.cs:125:21:125:40 | // ... | Missing result:hasValueFlow=10 |
3-
| J.cs:140:14:140:17 | (...) ... | Unexpected result: hasValueFlow=11 |
42
edges
53
| A.cs:5:17:5:28 | call to method Source<C> : C | A.cs:6:24:6:24 | access to local variable c : C |
64
| A.cs:5:17:5:28 | call to method Source<C> : C | A.cs:6:24:6:24 | access to local variable c : C |
@@ -920,14 +918,14 @@ edges
920918
| J.cs:106:14:106:15 | access to local variable a3 [property X] : Object | J.cs:106:14:106:17 | access to property X |
921919
| J.cs:107:14:107:15 | access to local variable a3 [property Y] : Object | J.cs:107:14:107:17 | access to property Y |
922920
| J.cs:107:14:107:15 | access to local variable a3 [property Y] : Object | J.cs:107:14:107:17 | access to property Y |
923-
| J.cs:133:13:133:13 | [post] access to local variable a [element] : Int32 | J.cs:140:14:140:14 | access to local variable a [element] : Int32 |
924-
| J.cs:133:13:133:13 | [post] access to local variable a [element] : Int32 | J.cs:140:14:140:14 | access to local variable a [element] : Int32 |
925-
| J.cs:133:19:133:33 | call to method Source<Int32> : Int32 | J.cs:133:13:133:13 | [post] access to local variable a [element] : Int32 |
926-
| J.cs:133:19:133:33 | call to method Source<Int32> : Int32 | J.cs:133:13:133:13 | [post] access to local variable a [element] : Int32 |
927-
| J.cs:140:14:140:14 | access to local variable a [element] : Int32 | J.cs:140:14:140:17 | access to indexer : Int32 |
928-
| J.cs:140:14:140:14 | access to local variable a [element] : Int32 | J.cs:140:14:140:17 | access to indexer : Int32 |
929-
| J.cs:140:14:140:17 | access to indexer : Int32 | J.cs:140:14:140:17 | (...) ... |
930-
| J.cs:140:14:140:17 | access to indexer : Int32 | J.cs:140:14:140:17 | (...) ... |
921+
| J.cs:119:13:119:13 | [post] access to local variable a [element] : Int32 | J.cs:125:14:125:14 | access to local variable a [element] : Int32 |
922+
| J.cs:119:13:119:13 | [post] access to local variable a [element] : Int32 | J.cs:125:14:125:14 | access to local variable a [element] : Int32 |
923+
| J.cs:119:20:119:34 | call to method Source<Int32> : Int32 | J.cs:119:13:119:13 | [post] access to local variable a [element] : Int32 |
924+
| J.cs:119:20:119:34 | call to method Source<Int32> : Int32 | J.cs:119:13:119:13 | [post] access to local variable a [element] : Int32 |
925+
| J.cs:125:14:125:14 | access to local variable a [element] : Int32 | J.cs:125:14:125:17 | access to array element : Int32 |
926+
| J.cs:125:14:125:14 | access to local variable a [element] : Int32 | J.cs:125:14:125:17 | access to array element : Int32 |
927+
| J.cs:125:14:125:17 | access to array element : Int32 | J.cs:125:14:125:17 | (...) ... |
928+
| J.cs:125:14:125:17 | access to array element : Int32 | J.cs:125:14:125:17 | (...) ... |
931929
nodes
932930
| A.cs:5:17:5:28 | call to method Source<C> : C | semmle.label | call to method Source<C> : C |
933931
| A.cs:5:17:5:28 | call to method Source<C> : C | semmle.label | call to method Source<C> : C |
@@ -1935,16 +1933,16 @@ nodes
19351933
| J.cs:107:14:107:15 | access to local variable a3 [property Y] : Object | semmle.label | access to local variable a3 [property Y] : Object |
19361934
| J.cs:107:14:107:17 | access to property Y | semmle.label | access to property Y |
19371935
| J.cs:107:14:107:17 | access to property Y | semmle.label | access to property Y |
1938-
| J.cs:133:13:133:13 | [post] access to local variable a [element] : Int32 | semmle.label | [post] access to local variable a [element] : Int32 |
1939-
| J.cs:133:13:133:13 | [post] access to local variable a [element] : Int32 | semmle.label | [post] access to local variable a [element] : Int32 |
1940-
| J.cs:133:19:133:33 | call to method Source<Int32> : Int32 | semmle.label | call to method Source<Int32> : Int32 |
1941-
| J.cs:133:19:133:33 | call to method Source<Int32> : Int32 | semmle.label | call to method Source<Int32> : Int32 |
1942-
| J.cs:140:14:140:14 | access to local variable a [element] : Int32 | semmle.label | access to local variable a [element] : Int32 |
1943-
| J.cs:140:14:140:14 | access to local variable a [element] : Int32 | semmle.label | access to local variable a [element] : Int32 |
1944-
| J.cs:140:14:140:17 | (...) ... | semmle.label | (...) ... |
1945-
| J.cs:140:14:140:17 | (...) ... | semmle.label | (...) ... |
1946-
| J.cs:140:14:140:17 | access to indexer : Int32 | semmle.label | access to indexer : Int32 |
1947-
| J.cs:140:14:140:17 | access to indexer : Int32 | semmle.label | access to indexer : Int32 |
1936+
| J.cs:119:13:119:13 | [post] access to local variable a [element] : Int32 | semmle.label | [post] access to local variable a [element] : Int32 |
1937+
| J.cs:119:13:119:13 | [post] access to local variable a [element] : Int32 | semmle.label | [post] access to local variable a [element] : Int32 |
1938+
| J.cs:119:20:119:34 | call to method Source<Int32> : Int32 | semmle.label | call to method Source<Int32> : Int32 |
1939+
| J.cs:119:20:119:34 | call to method Source<Int32> : Int32 | semmle.label | call to method Source<Int32> : Int32 |
1940+
| J.cs:125:14:125:14 | access to local variable a [element] : Int32 | semmle.label | access to local variable a [element] : Int32 |
1941+
| J.cs:125:14:125:14 | access to local variable a [element] : Int32 | semmle.label | access to local variable a [element] : Int32 |
1942+
| J.cs:125:14:125:17 | (...) ... | semmle.label | (...) ... |
1943+
| J.cs:125:14:125:17 | (...) ... | semmle.label | (...) ... |
1944+
| J.cs:125:14:125:17 | access to array element : Int32 | semmle.label | access to array element : Int32 |
1945+
| J.cs:125:14:125:17 | access to array element : Int32 | semmle.label | access to array element : Int32 |
19481946
subpaths
19491947
| A.cs:6:24:6:24 | access to local variable c : C | A.cs:147:32:147:32 | c : C | A.cs:149:20:149:27 | object creation of type B [field c] : C | A.cs:6:17:6:25 | call to method Make [field c] : C |
19501948
| A.cs:6:24:6:24 | access to local variable c : C | A.cs:147:32:147:32 | c : C | A.cs:149:20:149:27 | object creation of type B [field c] : C | A.cs:6:17:6:25 | call to method Make [field c] : C |
@@ -2121,4 +2119,4 @@ subpaths
21212119
| J.cs:102:14:102:17 | access to property X | J.cs:97:17:97:33 | call to method Source<Object> : Object | J.cs:102:14:102:17 | access to property X | $@ | J.cs:97:17:97:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |
21222120
| J.cs:106:14:106:17 | access to property X | J.cs:97:17:97:33 | call to method Source<Object> : Object | J.cs:106:14:106:17 | access to property X | $@ | J.cs:97:17:97:33 | call to method Source<Object> : Object | call to method Source<Object> : Object |
21232121
| J.cs:107:14:107:17 | access to property Y | J.cs:105:32:105:48 | call to method Source<Object> : Object | J.cs:107:14:107:17 | access to property Y | $@ | J.cs:105:32:105:48 | call to method Source<Object> : Object | call to method Source<Object> : Object |
2124-
| J.cs:140:14:140:17 | (...) ... | J.cs:133:19:133:33 | call to method Source<Int32> : Int32 | J.cs:140:14:140:17 | (...) ... | $@ | J.cs:133:19:133:33 | call to method Source<Int32> : Int32 | call to method Source<Int32> : Int32 |
2122+
| J.cs:125:14:125:17 | (...) ... | J.cs:119:20:119:34 | call to method Source<Int32> : Int32 | J.cs:125:14:125:17 | (...) ... | $@ | J.cs:119:20:119:34 | call to method Source<Int32> : Int32 | call to method Source<Int32> : Int32 |

0 commit comments

Comments
 (0)