Skip to content

Commit 503cbf1

Browse files
committed
C++: Flow from parameters to ConstructorFieldInit
Because `ConstructorFieldInit` (member initializer lists) are not part of the control flow graph, there was no data flow from the initial value of parameters to their uses in member initializers. This commit adds the necessary flow under the assumption that parameters are not overwritten in member initializers.
1 parent 45eefdb commit 503cbf1

File tree

3 files changed

+43
-2
lines changed

3 files changed

+43
-2
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,9 @@ module FlowVar_internal {
249249
result = descendentDef.getAUse(v)
250250
)
251251
)
252+
or
253+
parameterUsedInConstructorFieldInit(v, result) and
254+
def.definedByParameter(v)
252255
}
253256

254257
override predicate definedByExpr(Expr e, ControlFlowNode node) {
@@ -314,6 +317,9 @@ module FlowVar_internal {
314317
override VariableAccess getAnAccess() {
315318
variableAccessInSBB(v, getAReachedBlockVarSBB(this), result) and
316319
result != sbb
320+
or
321+
parameterUsedInConstructorFieldInit(v, result) and
322+
sbb = v.(Parameter).getFunction().getEntryPoint()
317323
}
318324

319325
override predicate definedByInitialValue(LocalScopeVariable lsv) {
@@ -692,6 +698,21 @@ module FlowVar_internal {
692698
assignedExpr = init.getExpr()
693699
}
694700

701+
/**
702+
* Holds if `p` is a parameter to a constructor that is used in a
703+
* `ConstructorFieldInit` at `va`. This ignores the corner case that `p`
704+
* might have been overwritten to have a different value before this happens.
705+
*/
706+
predicate parameterUsedInConstructorFieldInit(Parameter p, VariableAccess va) {
707+
exists(Constructor constructor |
708+
constructor.getInitializer(_).(ConstructorFieldInit).getExpr().getAChild*() = va and
709+
va = p.getAnAccess()
710+
// We don't require that `constructor` has `p` as a parameter because
711+
// that follows from the two other conditions and would likely just
712+
// confuse the join orderer.
713+
)
714+
}
715+
695716
/**
696717
* A point in a basic block where a non-SSA variable may have a different value
697718
* than it did elsewhere in the same basic block. Extending this class

cpp/ql/test/library-tests/dataflow/fields/constructors.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ class Foo
2525

2626
void bar(Foo &f)
2727
{
28-
sink(f.a()); // flow (through `f` and `h`) [NOT DETECTED]
29-
sink(f.b()); // flow (through `g` and `h`) [NOT DETECTED]
28+
sink(f.a()); // flow (through `f` and `h`)
29+
sink(f.b()); // flow (through `g` and `h`)
3030
}
3131

3232
void foo()

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,22 @@ edges
8686
| C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
8787
| C.cpp:27:8:27:11 | `this` parameter in func [s1, ... (1)] | file://:0:0:0:0 | this [s1, ... (1)] |
8888
| C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | file://:0:0:0:0 | this [s3, ... (1)] |
89+
| constructors.cpp:26:15:26:15 | f [a_, ... (1)] | constructors.cpp:28:10:28:10 | f [a_, ... (1)] |
90+
| constructors.cpp:26:15:26:15 | f [b_, ... (1)] | constructors.cpp:29:10:29:10 | f [b_, ... (1)] |
91+
| constructors.cpp:28:10:28:10 | f [a_, ... (1)] | constructors.cpp:28:12:28:12 | call to a |
92+
| constructors.cpp:29:10:29:10 | f [b_, ... (1)] | constructors.cpp:29:12:29:12 | call to b |
93+
| constructors.cpp:34:11:34:20 | call to user_input [void] | constructors.cpp:34:11:34:26 | call to Foo [a_, ... (1)] |
94+
| constructors.cpp:34:11:34:26 | call to Foo [a_, ... (1)] | constructors.cpp:40:9:40:9 | f [a_, ... (1)] |
95+
| constructors.cpp:35:11:35:26 | call to Foo [b_, ... (1)] | constructors.cpp:43:9:43:9 | g [b_, ... (1)] |
96+
| constructors.cpp:35:14:35:23 | call to user_input [void] | constructors.cpp:35:11:35:26 | call to Foo [b_, ... (1)] |
97+
| constructors.cpp:36:11:36:20 | call to user_input [void] | constructors.cpp:36:11:36:37 | call to Foo [a_, ... (1)] |
98+
| constructors.cpp:36:11:36:37 | call to Foo [a_, ... (1)] | constructors.cpp:46:9:46:9 | h [a_, ... (1)] |
99+
| constructors.cpp:36:11:36:37 | call to Foo [b_, ... (1)] | constructors.cpp:46:9:46:9 | h [b_, ... (1)] |
100+
| constructors.cpp:36:25:36:34 | call to user_input [void] | constructors.cpp:36:11:36:37 | call to Foo [b_, ... (1)] |
101+
| constructors.cpp:40:9:40:9 | f [a_, ... (1)] | constructors.cpp:26:15:26:15 | f [a_, ... (1)] |
102+
| constructors.cpp:43:9:43:9 | g [b_, ... (1)] | constructors.cpp:26:15:26:15 | f [b_, ... (1)] |
103+
| constructors.cpp:46:9:46:9 | h [a_, ... (1)] | constructors.cpp:26:15:26:15 | f [a_, ... (1)] |
104+
| constructors.cpp:46:9:46:9 | h [b_, ... (1)] | constructors.cpp:26:15:26:15 | f [b_, ... (1)] |
89105
| file://:0:0:0:0 | this [s1, ... (1)] | C.cpp:29:10:29:11 | s1 |
90106
| file://:0:0:0:0 | this [s3, ... (1)] | C.cpp:31:10:31:11 | s3 |
91107
| simple.cpp:26:15:26:15 | f [a_, ... (1)] | simple.cpp:28:10:28:10 | f [a_, ... (1)] |
@@ -124,6 +140,10 @@ edges
124140
| B.cpp:19:20:19:24 | elem2 | B.cpp:15:15:15:27 | new [void] | B.cpp:19:20:19:24 | elem2 | elem2 flows from $@ | B.cpp:15:15:15:27 | new [void] | new [void] |
125141
| C.cpp:29:10:29:11 | s1 | C.cpp:22:12:22:21 | new [void] | C.cpp:29:10:29:11 | s1 | s1 flows from $@ | C.cpp:22:12:22:21 | new [void] | new [void] |
126142
| C.cpp:31:10:31:11 | s3 | C.cpp:24:16:24:25 | new [void] | C.cpp:31:10:31:11 | s3 | s3 flows from $@ | C.cpp:24:16:24:25 | new [void] | new [void] |
143+
| constructors.cpp:28:12:28:12 | call to a | constructors.cpp:34:11:34:20 | call to user_input [void] | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:34:11:34:20 | call to user_input [void] | call to user_input [void] |
144+
| constructors.cpp:28:12:28:12 | call to a | constructors.cpp:36:11:36:20 | call to user_input [void] | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:36:11:36:20 | call to user_input [void] | call to user_input [void] |
145+
| constructors.cpp:29:12:29:12 | call to b | constructors.cpp:35:14:35:23 | call to user_input [void] | constructors.cpp:29:12:29:12 | call to b | call to b flows from $@ | constructors.cpp:35:14:35:23 | call to user_input [void] | call to user_input [void] |
146+
| constructors.cpp:29:12:29:12 | call to b | constructors.cpp:36:25:36:34 | call to user_input [void] | constructors.cpp:29:12:29:12 | call to b | call to b flows from $@ | constructors.cpp:36:25:36:34 | call to user_input [void] | call to user_input [void] |
127147
| simple.cpp:28:12:28:12 | call to a | simple.cpp:39:12:39:21 | call to user_input [void] | simple.cpp:28:12:28:12 | call to a | call to a flows from $@ | simple.cpp:39:12:39:21 | call to user_input [void] | call to user_input [void] |
128148
| simple.cpp:28:12:28:12 | call to a | simple.cpp:41:12:41:21 | call to user_input [void] | simple.cpp:28:12:28:12 | call to a | call to a flows from $@ | simple.cpp:41:12:41:21 | call to user_input [void] | call to user_input [void] |
129149
| simple.cpp:29:12:29:12 | call to b | simple.cpp:40:12:40:21 | call to user_input [void] | simple.cpp:29:12:29:12 | call to b | call to b flows from $@ | simple.cpp:40:12:40:21 | call to user_input [void] | call to user_input [void] |

0 commit comments

Comments
 (0)