Skip to content

Commit 38ec693

Browse files
jbjjbj
committed
C++: Improved ConstructorCall field flow
This commit changes C++ `ConstructorCall` to behave like `new`-expressions in Java: they are both `ExprNode`s and `PostUpdateNodes`, and there's a "pre-update node" (here called `PreConstructorCallNode`) to play the role of the qualifier argument when calling a constructor.
1 parent 1f1824c commit 38ec693

File tree

6 files changed

+62
-40
lines changed

6 files changed

+62
-40
lines changed

‎cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ private import DataFlowDispatch
66
private Node getInstanceArgument(Call call) {
77
result.asExpr() = call.getQualifier()
88
or
9-
// For constructors, there is no qualifier, so we pretend the call itself
10-
// is the instance argument.
11-
result.asExpr() = call.(ConstructorCall)
9+
result.(PreConstructorCallNode).getConstructorCall() = call
1210
// This does not include the implicit `this` argument on auto-generated
1311
// base class destructor calls as those do not have an AST element.
1412
}

‎cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ cached
1010
private newtype TNode =
1111
TExprNode(Expr e) or
1212
TPartialDefinitionNode(PartialDefinition pd) or
13-
TPostConstructorCallNode(ConstructorCall call) or
13+
TPreConstructorCallNode(ConstructorCall call) or
1414
TExplicitParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
1515
TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
1616
TUninitializedNode(LocalVariable v) { not v.hasInitializer() }
@@ -48,8 +48,8 @@ class Node extends TNode {
4848
*
4949
* Partial definitions are created for field stores (`x.y = taint();` is a partial
5050
* definition of `x`), and for calls that may change the value of an object (so
51-
* `x.set(taint())` is a partial definition of `x`, annd `transfer(&x, taint())` is
52-
* a partial definition of `&x`).s
51+
* `x.set(taint())` is a partial definition of `x`, and `transfer(&x, taint())` is
52+
* a partial definition of `&x`).
5353
*/
5454
Expr asPartialDefinition() {
5555
result = this.(PartialDefinitionNode).getPartialDefinition().getDefinedExpr()
@@ -226,8 +226,6 @@ abstract class PostUpdateNode extends Node {
226226
override Type getType() { result = getPreUpdateNode().getType() }
227227

228228
override Location getLocation() { result = getPreUpdateNode().getLocation() }
229-
230-
override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
231229
}
232230

233231
class PartialDefinitionNode extends PostUpdateNode, TPartialDefinitionNode {
@@ -240,14 +238,36 @@ class PartialDefinitionNode extends PostUpdateNode, TPartialDefinitionNode {
240238
override Location getLocation() { result = pd.getLocation() }
241239

242240
PartialDefinition getPartialDefinition() { result = pd }
241+
242+
override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
243+
}
244+
245+
private class PostConstructorCallNode extends PostUpdateNode, TExprNode {
246+
PostConstructorCallNode() { this = TExprNode(any(ConstructorCall c)) }
247+
248+
override PreConstructorCallNode getPreUpdateNode() {
249+
TExprNode(result.getConstructorCall()) = this
250+
}
243251
}
244252

245-
class PostConstructorCallNode extends PostUpdateNode, TPostConstructorCallNode {
246-
ConstructorCall call;
253+
/**
254+
* INTERNAL: do not use.
255+
*
256+
* A synthetic data-flow node that plays the role of the qualifier (or
257+
* `this`-argument) to a constructor call.
258+
*/
259+
class PreConstructorCallNode extends Node, TPreConstructorCallNode {
260+
PreConstructorCallNode() { this = TPreConstructorCallNode(_) }
261+
262+
ConstructorCall getConstructorCall() { this = TPreConstructorCallNode(result) }
263+
264+
override Function getFunction() { result = getConstructorCall().getEnclosingFunction() }
265+
266+
override Type getType() { result = getConstructorCall().getType() }
247267

248-
PostConstructorCallNode() { this = TPostConstructorCallNode(call) }
268+
override Location getLocation() { result = getConstructorCall().getLocation() }
249269

250-
override Node getPreUpdateNode() { result.asExpr() = call }
270+
override string toString() { result = getConstructorCall().toString() + " [pre constructor call]" }
251271
}
252272

253273
/**

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

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ edges
77
| A.cpp:55:5:55:5 | b [post update] [c, ... (1)] | A.cpp:56:10:56:10 | b [c, ... (1)] |
88
| A.cpp:55:12:55:19 | new [void] | A.cpp:55:5:55:5 | b [post update] [c, ... (1)] |
99
| A.cpp:56:10:56:10 | b [c, ... (1)] | A.cpp:56:13:56:15 | call to get |
10-
| A.cpp:57:11:57:24 | call to B [post update] [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
10+
| A.cpp:57:11:57:24 | call to B [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
1111
| A.cpp:57:11:57:24 | new [c, ... (1)] | A.cpp:57:28:57:30 | call to get |
12-
| A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [post update] [c, ... (1)] |
12+
| A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [c, ... (1)] |
1313
| A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] | A.cpp:66:10:66:11 | b2 [c, ... (1)] |
1414
| A.cpp:64:21:64:28 | new [void] | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] |
1515
| A.cpp:66:10:66:11 | b2 [c, ... (1)] | A.cpp:66:14:66:14 | c |
@@ -27,26 +27,26 @@ edges
2727
| A.cpp:142:7:142:7 | b [post update] [c, ... (1)] | A.cpp:143:7:143:31 | ... = ... [c, ... (1)] |
2828
| A.cpp:142:7:142:20 | ... = ... [void] | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] |
2929
| A.cpp:142:14:142:20 | new [void] | A.cpp:142:7:142:20 | ... = ... [void] |
30-
| A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
31-
| A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (2)] |
30+
| A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
31+
| A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [b, ... (2)] |
3232
| A.cpp:143:7:143:31 | ... = ... [c, ... (1)] | A.cpp:143:7:143:10 | this [post update] [b, ... (2)] |
3333
| A.cpp:143:7:143:31 | ... = ... [void] | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] |
3434
| A.cpp:143:25:143:31 | new [void] | A.cpp:143:7:143:31 | ... = ... [void] |
3535
| A.cpp:150:12:150:18 | new [void] | A.cpp:151:18:151:18 | b [void] |
36-
| A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
37-
| A.cpp:151:12:151:24 | call to D [post update] [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
38-
| A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
36+
| A.cpp:151:12:151:24 | call to D [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
37+
| A.cpp:151:12:151:24 | call to D [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
38+
| A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
3939
| A.cpp:152:10:152:10 | d [b, ... (1)] | A.cpp:152:13:152:13 | b |
4040
| A.cpp:153:10:153:10 | d [b, ... (2)] | A.cpp:153:13:153:13 | b [c, ... (1)] |
4141
| A.cpp:153:13:153:13 | b [c, ... (1)] | A.cpp:153:16:153:16 | c |
4242
| A.cpp:159:12:159:18 | new [void] | A.cpp:160:29:160:29 | b [void] |
43-
| A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
44-
| A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] |
45-
| A.cpp:161:18:161:40 | call to MyList [post update] [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
46-
| A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [post update] [next, ... (2)] |
47-
| A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
48-
| A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
49-
| A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] |
43+
| A.cpp:160:18:160:60 | call to MyList [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
44+
| A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [head, ... (1)] |
45+
| A.cpp:161:18:161:40 | call to MyList [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
46+
| A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [next, ... (2)] |
47+
| A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
48+
| A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
49+
| A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] |
5050
| A.cpp:165:10:165:11 | l3 [next, ... (3)] | A.cpp:165:14:165:17 | next [next, ... (2)] |
5151
| A.cpp:165:14:165:17 | next [next, ... (2)] | A.cpp:165:20:165:23 | next [head, ... (1)] |
5252
| A.cpp:165:20:165:23 | next [head, ... (1)] | A.cpp:165:26:165:29 | head |
@@ -56,28 +56,28 @@ edges
5656
| A.cpp:167:47:167:50 | next [next, ... (2)] | A.cpp:167:44:167:44 | l [next, ... (2)] |
5757
| A.cpp:169:12:169:12 | l [head, ... (1)] | A.cpp:169:15:169:18 | head |
5858
| B.cpp:6:15:6:24 | new [void] | B.cpp:7:25:7:25 | e [void] |
59-
| B.cpp:7:16:7:35 | call to Box1 [post update] [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
60-
| B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [post update] [elem1, ... (1)] |
61-
| B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
62-
| B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
63-
| B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] |
59+
| B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
60+
| B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] |
61+
| B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
62+
| B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
63+
| B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] |
6464
| B.cpp:9:10:9:11 | b2 [box1, ... (2)] | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] |
6565
| B.cpp:9:14:9:17 | box1 [elem1, ... (1)] | B.cpp:9:20:9:24 | elem1 |
6666
| B.cpp:10:10:10:11 | b2 [box1, ... (2)] | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] |
6767
| B.cpp:10:14:10:17 | box1 [elem2, ... (1)] | B.cpp:10:20:10:24 | elem2 |
6868
| B.cpp:15:15:15:27 | new [void] | B.cpp:16:37:16:37 | e [void] |
69-
| B.cpp:16:16:16:38 | call to Box1 [post update] [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
70-
| B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [post update] [elem2, ... (1)] |
71-
| B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
72-
| B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
73-
| B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] |
69+
| B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
70+
| B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] |
71+
| B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
72+
| B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
73+
| B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] |
7474
| B.cpp:18:10:18:11 | b2 [box1, ... (2)] | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] |
7575
| B.cpp:18:14:18:17 | box1 [elem1, ... (1)] | B.cpp:18:20:18:24 | elem1 |
7676
| B.cpp:19:10:19:11 | b2 [box1, ... (2)] | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] |
7777
| B.cpp:19:14:19:17 | box1 [elem2, ... (1)] | B.cpp:19:20:19:24 | elem2 |
78-
| C.cpp:18:12:18:18 | call to C [post update] [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
78+
| C.cpp:18:12:18:18 | call to C [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
7979
| C.cpp:19:5:19:5 | c [s3, ... (1)] | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] |
80-
| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [post update] [s3, ... (1)] |
80+
| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [s3, ... (1)] |
8181
| C.cpp:24:5:24:25 | ... = ... [void] | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] |
8282
| C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
8383
| C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | file://:0:0:0:0 | this [s3, ... (1)] |

‎cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,11 @@ void class_field_test() {
8787

8888
sink(mc1.a);
8989
sink(mc1.b); // tainted [NOT DETECTED]
90-
sink(mc1.c); // tainted [NOT DETECTED]
90+
sink(mc1.c); // tainted [NOT DETECTED with IR]
9191
sink(mc1.d); // tainted [NOT DETECTED with IR]
9292
sink(mc2.a);
9393
sink(mc2.b); // tainted [NOT DETECTED]
94-
sink(mc2.c); // tainted [NOT DETECTED]
94+
sink(mc2.c); // tainted [NOT DETECTED with IR]
9595
sink(mc2.d);
9696
}
9797

‎cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
| taint.cpp:41:7:41:13 | global7 | taint.cpp:35:12:35:17 | call to source |
55
| taint.cpp:42:7:42:13 | global8 | taint.cpp:35:12:35:17 | call to source |
66
| taint.cpp:43:7:43:13 | global9 | taint.cpp:37:22:37:27 | call to source |
7+
| taint.cpp:90:11:90:11 | c | taint.cpp:72:7:72:12 | call to source |
78
| taint.cpp:91:11:91:11 | d | taint.cpp:77:7:77:12 | call to source |
9+
| taint.cpp:94:11:94:11 | c | taint.cpp:72:7:72:12 | call to source |
810
| taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
911
| taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |
1012
| taint.cpp:137:7:137:9 | * ... | taint.cpp:120:11:120:16 | call to source |

‎cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
| taint.cpp:41:7:41:13 | taint.cpp:35:12:35:17 | AST only |
22
| taint.cpp:42:7:42:13 | taint.cpp:35:12:35:17 | AST only |
33
| taint.cpp:43:7:43:13 | taint.cpp:37:22:37:27 | AST only |
4+
| taint.cpp:90:11:90:11 | taint.cpp:72:7:72:12 | AST only |
45
| taint.cpp:91:11:91:11 | taint.cpp:77:7:77:12 | AST only |
6+
| taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
57
| taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
68
| taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
79
| taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |

0 commit comments

Comments
 (0)