Skip to content

Commit cadcb20

Browse files
MathiasVPMathiasVP
authored
Merge pull request #19500 from MathiasVP/always-apply-manual-models-when-resolving-calls
C++: Update static call target resolution semantics in dataflow
2 parents ffc1c62 + d31ddad commit cadcb20

File tree

8 files changed

+127
-33
lines changed

8 files changed

+127
-33
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,10 @@ private newtype TDataFlowCall =
11431143
FlowSummaryImpl::Private::summaryCallbackRange(c, receiver)
11441144
}
11451145

1146+
private predicate summarizedCallableIsManual(SummarizedCallable sc) {
1147+
sc.asSummarizedCallable().applyManualModel()
1148+
}
1149+
11461150
/**
11471151
* A function call relevant for data flow. This includes calls from source
11481152
* code and calls inside library callables with a flow summary.
@@ -1164,15 +1168,27 @@ class DataFlowCall extends TDataFlowCall {
11641168
Function getStaticCallSourceTarget() { none() }
11651169

11661170
/**
1167-
* Gets the target of this call. If a summarized callable exists for the
1168-
* target this is chosen, and otherwise the callable is the implementation
1169-
* from the source code.
1171+
* Gets the target of this call. We use the following strategy for deciding
1172+
* between the source callable and a summarized callable:
1173+
* - If there is a manual summary then we always use the manual summary.
1174+
* - If there is a source callable and we only have generated summaries
1175+
* we use the source callable.
1176+
* - If there is no source callable then we use the summary regardless of
1177+
* whether is it manual or generated.
11701178
*/
1171-
DataFlowCallable getStaticCallTarget() {
1179+
final DataFlowCallable getStaticCallTarget() {
11721180
exists(Function target | target = this.getStaticCallSourceTarget() |
1173-
not exists(TSummarizedCallable(target)) and
1181+
// Don't use the source callable if there is a manual model for the
1182+
// target
1183+
not exists(SummarizedCallable sc |
1184+
sc.asSummarizedCallable() = target and
1185+
summarizedCallableIsManual(sc)
1186+
) and
11741187
result.asSourceCallable() = target
11751188
or
1189+
// When there is no function body, or when we have a manual model then
1190+
// we dispatch to the summary.
1191+
(not target.hasDefinition() or summarizedCallableIsManual(result)) and
11761192
result.asSummarizedCallable() = target
11771193
)
11781194
}

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

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,33 @@ edges
1010
| asio_streams.cpp:100:44:100:62 | call to buffer | asio_streams.cpp:103:29:103:39 | *send_buffer | provenance | Sink:MaD:6 |
1111
| asio_streams.cpp:100:64:100:71 | *send_str | asio_streams.cpp:56:18:56:23 | [summary param] *0 in buffer | provenance | |
1212
| asio_streams.cpp:100:64:100:71 | *send_str | asio_streams.cpp:100:44:100:62 | call to buffer | provenance | MaD:10 |
13-
| test.cpp:4:5:4:11 | [summary param] 0 in ymlStep | test.cpp:4:5:4:11 | [summary] to write: ReturnValue in ymlStep | provenance | MaD:969 |
14-
| test.cpp:7:10:7:18 | call to ymlSource | test.cpp:7:10:7:18 | call to ymlSource | provenance | Src:MaD:967 |
15-
| test.cpp:7:10:7:18 | call to ymlSource | test.cpp:11:10:11:10 | x | provenance | Sink:MaD:968 |
16-
| test.cpp:7:10:7:18 | call to ymlSource | test.cpp:13:18:13:18 | x | provenance | |
17-
| test.cpp:13:10:13:16 | call to ymlStep | test.cpp:13:10:13:16 | call to ymlStep | provenance | |
18-
| test.cpp:13:10:13:16 | call to ymlStep | test.cpp:15:10:15:10 | y | provenance | Sink:MaD:968 |
19-
| test.cpp:13:18:13:18 | x | test.cpp:4:5:4:11 | [summary param] 0 in ymlStep | provenance | |
20-
| test.cpp:13:18:13:18 | x | test.cpp:13:10:13:16 | call to ymlStep | provenance | MaD:969 |
13+
| test.cpp:4:5:4:17 | [summary param] 0 in ymlStepManual | test.cpp:4:5:4:17 | [summary] to write: ReturnValue in ymlStepManual | provenance | MaD:969 |
14+
| test.cpp:5:5:5:20 | [summary param] 0 in ymlStepGenerated | test.cpp:5:5:5:20 | [summary] to write: ReturnValue in ymlStepGenerated | provenance | MaD:970 |
15+
| test.cpp:6:5:6:27 | [summary param] 0 in ymlStepManual_with_body | test.cpp:6:5:6:27 | [summary] to write: ReturnValue in ymlStepManual_with_body | provenance | MaD:971 |
16+
| test.cpp:7:47:7:52 | value2 | test.cpp:7:64:7:69 | value2 | provenance | |
17+
| test.cpp:7:64:7:69 | value2 | test.cpp:7:5:7:30 | *ymlStepGenerated_with_body | provenance | |
18+
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:10:10:10:18 | call to ymlSource | provenance | Src:MaD:967 |
19+
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:14:10:14:10 | x | provenance | Sink:MaD:968 |
20+
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:17:24:17:24 | x | provenance | |
21+
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:21:27:21:27 | x | provenance | |
22+
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:25:35:25:35 | x | provenance | |
23+
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:32:41:32:41 | x | provenance | |
24+
| test.cpp:17:10:17:22 | call to ymlStepManual | test.cpp:17:10:17:22 | call to ymlStepManual | provenance | |
25+
| test.cpp:17:10:17:22 | call to ymlStepManual | test.cpp:18:10:18:10 | y | provenance | Sink:MaD:968 |
26+
| test.cpp:17:24:17:24 | x | test.cpp:4:5:4:17 | [summary param] 0 in ymlStepManual | provenance | |
27+
| test.cpp:17:24:17:24 | x | test.cpp:17:10:17:22 | call to ymlStepManual | provenance | MaD:969 |
28+
| test.cpp:21:10:21:25 | call to ymlStepGenerated | test.cpp:21:10:21:25 | call to ymlStepGenerated | provenance | |
29+
| test.cpp:21:10:21:25 | call to ymlStepGenerated | test.cpp:22:10:22:10 | z | provenance | Sink:MaD:968 |
30+
| test.cpp:21:27:21:27 | x | test.cpp:5:5:5:20 | [summary param] 0 in ymlStepGenerated | provenance | |
31+
| test.cpp:21:27:21:27 | x | test.cpp:21:10:21:25 | call to ymlStepGenerated | provenance | MaD:970 |
32+
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | test.cpp:25:11:25:33 | call to ymlStepManual_with_body | provenance | |
33+
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | test.cpp:26:10:26:11 | y2 | provenance | Sink:MaD:968 |
34+
| test.cpp:25:35:25:35 | x | test.cpp:6:5:6:27 | [summary param] 0 in ymlStepManual_with_body | provenance | |
35+
| test.cpp:25:35:25:35 | x | test.cpp:25:11:25:33 | call to ymlStepManual_with_body | provenance | MaD:971 |
36+
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | provenance | |
37+
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | test.cpp:33:10:33:11 | z2 | provenance | Sink:MaD:968 |
38+
| test.cpp:32:41:32:41 | x | test.cpp:7:47:7:52 | value2 | provenance | |
39+
| test.cpp:32:41:32:41 | x | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | provenance | |
2140
nodes
2241
| asio_streams.cpp:56:18:56:23 | [summary param] *0 in buffer | semmle.label | [summary param] *0 in buffer |
2342
| asio_streams.cpp:56:18:56:23 | [summary] to write: ReturnValue in buffer | semmle.label | [summary] to write: ReturnValue in buffer |
@@ -31,15 +50,37 @@ nodes
3150
| asio_streams.cpp:100:64:100:71 | *send_str | semmle.label | *send_str |
3251
| asio_streams.cpp:101:7:101:17 | send_buffer | semmle.label | send_buffer |
3352
| asio_streams.cpp:103:29:103:39 | *send_buffer | semmle.label | *send_buffer |
34-
| test.cpp:4:5:4:11 | [summary param] 0 in ymlStep | semmle.label | [summary param] 0 in ymlStep |
35-
| test.cpp:4:5:4:11 | [summary] to write: ReturnValue in ymlStep | semmle.label | [summary] to write: ReturnValue in ymlStep |
36-
| test.cpp:7:10:7:18 | call to ymlSource | semmle.label | call to ymlSource |
37-
| test.cpp:7:10:7:18 | call to ymlSource | semmle.label | call to ymlSource |
38-
| test.cpp:11:10:11:10 | x | semmle.label | x |
39-
| test.cpp:13:10:13:16 | call to ymlStep | semmle.label | call to ymlStep |
40-
| test.cpp:13:10:13:16 | call to ymlStep | semmle.label | call to ymlStep |
41-
| test.cpp:13:18:13:18 | x | semmle.label | x |
42-
| test.cpp:15:10:15:10 | y | semmle.label | y |
53+
| test.cpp:4:5:4:17 | [summary param] 0 in ymlStepManual | semmle.label | [summary param] 0 in ymlStepManual |
54+
| test.cpp:4:5:4:17 | [summary] to write: ReturnValue in ymlStepManual | semmle.label | [summary] to write: ReturnValue in ymlStepManual |
55+
| test.cpp:5:5:5:20 | [summary param] 0 in ymlStepGenerated | semmle.label | [summary param] 0 in ymlStepGenerated |
56+
| test.cpp:5:5:5:20 | [summary] to write: ReturnValue in ymlStepGenerated | semmle.label | [summary] to write: ReturnValue in ymlStepGenerated |
57+
| test.cpp:6:5:6:27 | [summary param] 0 in ymlStepManual_with_body | semmle.label | [summary param] 0 in ymlStepManual_with_body |
58+
| test.cpp:6:5:6:27 | [summary] to write: ReturnValue in ymlStepManual_with_body | semmle.label | [summary] to write: ReturnValue in ymlStepManual_with_body |
59+
| test.cpp:7:5:7:30 | *ymlStepGenerated_with_body | semmle.label | *ymlStepGenerated_with_body |
60+
| test.cpp:7:47:7:52 | value2 | semmle.label | value2 |
61+
| test.cpp:7:64:7:69 | value2 | semmle.label | value2 |
62+
| test.cpp:10:10:10:18 | call to ymlSource | semmle.label | call to ymlSource |
63+
| test.cpp:10:10:10:18 | call to ymlSource | semmle.label | call to ymlSource |
64+
| test.cpp:14:10:14:10 | x | semmle.label | x |
65+
| test.cpp:17:10:17:22 | call to ymlStepManual | semmle.label | call to ymlStepManual |
66+
| test.cpp:17:10:17:22 | call to ymlStepManual | semmle.label | call to ymlStepManual |
67+
| test.cpp:17:24:17:24 | x | semmle.label | x |
68+
| test.cpp:18:10:18:10 | y | semmle.label | y |
69+
| test.cpp:21:10:21:25 | call to ymlStepGenerated | semmle.label | call to ymlStepGenerated |
70+
| test.cpp:21:10:21:25 | call to ymlStepGenerated | semmle.label | call to ymlStepGenerated |
71+
| test.cpp:21:27:21:27 | x | semmle.label | x |
72+
| test.cpp:22:10:22:10 | z | semmle.label | z |
73+
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | semmle.label | call to ymlStepManual_with_body |
74+
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | semmle.label | call to ymlStepManual_with_body |
75+
| test.cpp:25:35:25:35 | x | semmle.label | x |
76+
| test.cpp:26:10:26:11 | y2 | semmle.label | y2 |
77+
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | semmle.label | call to ymlStepGenerated_with_body |
78+
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | semmle.label | call to ymlStepGenerated_with_body |
79+
| test.cpp:32:41:32:41 | x | semmle.label | x |
80+
| test.cpp:33:10:33:11 | z2 | semmle.label | z2 |
4381
subpaths
4482
| asio_streams.cpp:100:64:100:71 | *send_str | asio_streams.cpp:56:18:56:23 | [summary param] *0 in buffer | asio_streams.cpp:56:18:56:23 | [summary] to write: ReturnValue in buffer | asio_streams.cpp:100:44:100:62 | call to buffer |
45-
| test.cpp:13:18:13:18 | x | test.cpp:4:5:4:11 | [summary param] 0 in ymlStep | test.cpp:4:5:4:11 | [summary] to write: ReturnValue in ymlStep | test.cpp:13:10:13:16 | call to ymlStep |
83+
| test.cpp:17:24:17:24 | x | test.cpp:4:5:4:17 | [summary param] 0 in ymlStepManual | test.cpp:4:5:4:17 | [summary] to write: ReturnValue in ymlStepManual | test.cpp:17:10:17:22 | call to ymlStepManual |
84+
| test.cpp:21:27:21:27 | x | test.cpp:5:5:5:20 | [summary param] 0 in ymlStepGenerated | test.cpp:5:5:5:20 | [summary] to write: ReturnValue in ymlStepGenerated | test.cpp:21:10:21:25 | call to ymlStepGenerated |
85+
| test.cpp:25:35:25:35 | x | test.cpp:6:5:6:27 | [summary param] 0 in ymlStepManual_with_body | test.cpp:6:5:6:27 | [summary] to write: ReturnValue in ymlStepManual_with_body | test.cpp:25:11:25:33 | call to ymlStepManual_with_body |
86+
| test.cpp:32:41:32:41 | x | test.cpp:7:47:7:52 | value2 | test.cpp:7:5:7:30 | *ymlStepGenerated_with_body | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body |

‎cpp/ql/test/library-tests/dataflow/external-models/flow.ext.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,7 @@ extensions:
1313
pack: codeql/cpp-all
1414
extensible: summaryModel
1515
data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
16-
- ["", "", False, "ymlStep", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
16+
- ["", "", False, "ymlStepManual", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
17+
- ["", "", False, "ymlStepGenerated", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]
18+
- ["", "", False, "ymlStepManual_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
19+
- ["", "", False, "ymlStepGenerated_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]

‎cpp/ql/test/library-tests/dataflow/external-models/sinks.expected

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
| asio_streams.cpp:93:29:93:39 | *recv_buffer | remote-sink |
22
| asio_streams.cpp:103:29:103:39 | *send_buffer | remote-sink |
3-
| test.cpp:9:10:9:10 | 0 | test-sink |
4-
| test.cpp:11:10:11:10 | x | test-sink |
5-
| test.cpp:15:10:15:10 | y | test-sink |
3+
| test.cpp:12:10:12:10 | 0 | test-sink |
4+
| test.cpp:14:10:14:10 | x | test-sink |
5+
| test.cpp:18:10:18:10 | y | test-sink |
6+
| test.cpp:22:10:22:10 | z | test-sink |
7+
| test.cpp:26:10:26:11 | y2 | test-sink |
8+
| test.cpp:29:10:29:11 | y3 | test-sink |
9+
| test.cpp:33:10:33:11 | z2 | test-sink |
10+
| test.cpp:36:10:36:11 | z3 | test-sink |

‎cpp/ql/test/library-tests/dataflow/external-models/sources.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
| asio_streams.cpp:87:34:87:44 | read_until output argument | remote |
2-
| test.cpp:7:10:7:18 | call to ymlSource | local |
2+
| test.cpp:10:10:10:18 | call to ymlSource | local |

‎cpp/ql/test/library-tests/dataflow/external-models/steps.expected

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,7 @@
11
| asio_streams.cpp:100:64:100:71 | *send_str | asio_streams.cpp:100:44:100:62 | call to buffer |
2-
| test.cpp:13:18:13:18 | x | test.cpp:13:10:13:16 | call to ymlStep |
2+
| test.cpp:17:24:17:24 | x | test.cpp:17:10:17:22 | call to ymlStepManual |
3+
| test.cpp:21:27:21:27 | x | test.cpp:21:10:21:25 | call to ymlStepGenerated |
4+
| test.cpp:25:35:25:35 | x | test.cpp:25:11:25:33 | call to ymlStepManual_with_body |
5+
| test.cpp:28:35:28:35 | 0 | test.cpp:28:11:28:33 | call to ymlStepManual_with_body |
6+
| test.cpp:32:38:32:38 | 0 | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body |
7+
| test.cpp:35:38:35:38 | x | test.cpp:35:11:35:36 | call to ymlStepGenerated_with_body |

‎cpp/ql/test/library-tests/dataflow/external-models/steps.ext.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,7 @@ extensions:
33
pack: codeql/cpp-all
44
extensible: summaryModel
55
data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
6-
- ["", "", False, "ymlStep", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
6+
- ["", "", False, "ymlStepManual", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
7+
- ["", "", False, "ymlStepGenerated", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]
8+
- ["", "", False, "ymlStepManual_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
9+
- ["", "", False, "ymlStepGenerated_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]

‎cpp/ql/test/library-tests/dataflow/external-models/test.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11

22
int ymlSource();
33
void ymlSink(int value);
4-
int ymlStep(int value);
4+
int ymlStepManual(int value);
5+
int ymlStepGenerated(int value);
6+
int ymlStepManual_with_body(int value1, int value2) { return value2; }
7+
int ymlStepGenerated_with_body(int value, int value2) { return value2; }
58

69
void test() {
710
int x = ymlSource();
@@ -10,7 +13,25 @@ void test() {
1013

1114
ymlSink(x); // $ ir
1215

13-
int y = ymlStep(x);
14-
16+
// ymlStepManual is manually modeled so we should always use the model
17+
int y = ymlStepManual(x);
1518
ymlSink(y); // $ ir
19+
20+
// ymlStepGenerated is modeled by the model generator so we should use the model only if there is no body
21+
int z = ymlStepGenerated(x);
22+
ymlSink(z); // $ ir
23+
24+
// ymlStepManual_with_body is manually modeled so we should always use the model
25+
int y2 = ymlStepManual_with_body(x, 0);
26+
ymlSink(y2); // $ ir
27+
28+
int y3 = ymlStepManual_with_body(0, x);
29+
ymlSink(y3); // clean
30+
31+
// ymlStepGenerated_with_body is modeled by the model generator so we should use the model only if there is no body
32+
int z2 = ymlStepGenerated_with_body(0, x);
33+
ymlSink(z2); // $ ir
34+
35+
int z3 = ymlStepGenerated_with_body(x, 0);
36+
ymlSink(z3); // clean
1637
}

0 commit comments

Comments
 (0)