Skip to content

Commit 067c55a

Browse files
committed
C++: Fix ConditionDeclExpr data flow
Data flow probably never worked when a variable declared in a `ConditionDeclExpr` was modeled with `BlockVar`. That combination did not come up in testing before the last commit.
1 parent d7681bf commit 067c55a

File tree

2 files changed

+20
-7
lines changed

2 files changed

+20
-7
lines changed

cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ module FlowVar_internal {
244244
not v instanceof Field and // Fields are interprocedural data flow, not local
245245
reachable(sbb) and
246246
(
247-
initializer(sbb.getANode(), v, _)
247+
initializer(v, sbb.getANode())
248248
or
249249
assignmentLikeOperation(sbb, v, _, _)
250250
or
@@ -361,7 +361,12 @@ module FlowVar_internal {
361361
assignmentLikeOperation(node, v, _, e) and
362362
node = sbb
363363
or
364-
initializer(node, v, e) and
364+
// We pick the defining `ControlFlowNode` of an `Initializer` to be its
365+
// expression rather than the `Initializer` itself. That's because the
366+
// `Initializer` of a `ConditionDeclExpr` is for historical reasons not
367+
// part of the CFG and therefore ends up in the wrong basic block.
368+
initializer(v, e) and
369+
node = e and
365370
node = sbb.getANode()
366371
}
367372

@@ -719,13 +724,11 @@ module FlowVar_internal {
719724
}
720725

721726
/**
722-
* Holds if `v` is initialized by `init` to have value `assignedExpr`.
727+
* Holds if `v` is initialized to have value `assignedExpr`.
723728
*/
724-
predicate initializer(
725-
Initializer init, LocalVariable v, Expr assignedExpr)
729+
predicate initializer(LocalVariable v, Expr assignedExpr)
726730
{
727-
v = init.getDeclaration() and
728-
assignedExpr = init.getExpr()
731+
assignedExpr = v.getInitializer().getExpr()
729732
}
730733

731734
/**

cpp/ql/test/library-tests/dataflow/fields/flow.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ edges
1616
| A.cpp:73:10:73:19 | call to setOnBWrap [c, ... (1)] | A.cpp:75:10:75:11 | b2 [c, ... (1)] |
1717
| A.cpp:73:25:73:32 | new [void] | A.cpp:73:10:73:19 | call to setOnBWrap [c, ... (1)] |
1818
| A.cpp:75:10:75:11 | b2 [c, ... (1)] | A.cpp:75:14:75:14 | c |
19+
| A.cpp:98:12:98:18 | new [void] | A.cpp:100:5:100:13 | ... = ... [void] |
20+
| A.cpp:100:5:100:6 | c1 [post update] [a, ... (1)] | A.cpp:101:8:101:9 | c1 [a, ... (1)] |
21+
| A.cpp:100:5:100:13 | ... = ... [void] | A.cpp:100:5:100:6 | c1 [post update] [a, ... (1)] |
22+
| A.cpp:101:8:101:9 | c1 [a, ... (1)] | A.cpp:103:14:103:14 | c [a, ... (1)] |
23+
| A.cpp:103:14:103:14 | c [a, ... (1)] | A.cpp:107:12:107:13 | c1 [a, ... (1)] |
24+
| A.cpp:103:14:103:14 | c [a, ... (1)] | A.cpp:120:12:120:13 | c1 [a, ... (1)] |
25+
| A.cpp:107:12:107:13 | c1 [a, ... (1)] | A.cpp:107:16:107:16 | a |
26+
| A.cpp:120:12:120:13 | c1 [a, ... (1)] | A.cpp:120:16:120:16 | a |
1927
| A.cpp:126:5:126:5 | b [post update] [c, ... (1)] | A.cpp:131:8:131:8 | ref arg b [c, ... (1)] |
2028
| A.cpp:126:12:126:18 | new [void] | A.cpp:126:5:126:5 | b [post update] [c, ... (1)] |
2129
| A.cpp:131:8:131:8 | ref arg b [c, ... (1)] | A.cpp:132:10:132:10 | b [c, ... (1)] |
@@ -169,6 +177,8 @@ edges
169177
| A.cpp:57:28:57:30 | call to get | A.cpp:57:17:57:23 | new [void] | A.cpp:57:28:57:30 | call to get | call to get flows from $@ | A.cpp:57:17:57:23 | new [void] | new [void] |
170178
| A.cpp:66:14:66:14 | c | A.cpp:64:21:64:28 | new [void] | A.cpp:66:14:66:14 | c | c flows from $@ | A.cpp:64:21:64:28 | new [void] | new [void] |
171179
| A.cpp:75:14:75:14 | c | A.cpp:73:25:73:32 | new [void] | A.cpp:75:14:75:14 | c | c flows from $@ | A.cpp:73:25:73:32 | new [void] | new [void] |
180+
| A.cpp:107:16:107:16 | a | A.cpp:98:12:98:18 | new [void] | A.cpp:107:16:107:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new [void] | new [void] |
181+
| A.cpp:120:16:120:16 | a | A.cpp:98:12:98:18 | new [void] | A.cpp:120:16:120:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new [void] | new [void] |
172182
| A.cpp:132:13:132:13 | c | A.cpp:126:12:126:18 | new [void] | A.cpp:132:13:132:13 | c | c flows from $@ | A.cpp:126:12:126:18 | new [void] | new [void] |
173183
| A.cpp:152:13:152:13 | b | A.cpp:143:25:143:31 | new [void] | A.cpp:152:13:152:13 | b | b flows from $@ | A.cpp:143:25:143:31 | new [void] | new [void] |
174184
| A.cpp:152:13:152:13 | b | A.cpp:150:12:150:18 | new [void] | A.cpp:152:13:152:13 | b | b flows from $@ | A.cpp:150:12:150:18 | new [void] | new [void] |

0 commit comments

Comments
 (0)