Skip to content

C++: Update static call target resolution semantics in dataflow #19500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 16, 2025
Merged

C++: Update static call target resolution semantics in dataflow #19500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6df950
C++: Add tests to exercise the upcoming behavior of function dispatch…
MathiasVP May 15, 2025
69a1a87
C++: Update semantics of picking the static call target in dataflow.
MathiasVP May 15, 2025
e75dcd2
C++: Accept test changes.
MathiasVP May 15, 2025
d31ddad
C++: Small refactoring.
MathiasVP May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,10 @@ private newtype TDataFlowCall =
FlowSummaryImpl::Private::summaryCallbackRange(c, receiver)
}

private predicate summarizedCallableIsManual(SummarizedCallable sc) {
sc.asSummarizedCallable().applyManualModel()
}

/**
* A function call relevant for data flow. This includes calls from source
* code and calls inside library callables with a flow summary.
Expand All @@ -1164,15 +1168,27 @@ class DataFlowCall extends TDataFlowCall {
Function getStaticCallSourceTarget() { none() }

/**
* Gets the target of this call. If a summarized callable exists for the
* target this is chosen, and otherwise the callable is the implementation
* from the source code.
* Gets the target of this call. We use the following strategy for deciding
* between the source callable and a summarized callable:
* - If there is a manual summary then we always use the manual summary.
* - If there is a source callable and we only have generated summaries
* we use the source callable.
* - If there is no source callable then we use the summary regardless of
* whether is it manual or generated.
Comment on lines +1173 to +1177
Copy link
Preview

Copilot AI May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider rephrasing for clarity, e.g. 'If there is a source callable and no manual summary exists, use the source callable.'

Suggested change
* - If there is a manual summary then we always use the manual summary.
* - If there is a source callable and we only have generated summaries
* we use the source callable.
* - If there is no source callable then we use the summary regardless of
* whether is it manual or generated.
* - If there is a manual summary, then we always use the manual summary.
* - If there is a source callable and no manual summary exists, use the source callable.
* - If there is no source callable, then we use the summary regardless of
* whether it is manual or generated.

Copilot uses AI. Check for mistakes.

*/
DataFlowCallable getStaticCallTarget() {
final DataFlowCallable getStaticCallTarget() {
exists(Function target | target = this.getStaticCallSourceTarget() |
not exists(TSummarizedCallable(target)) and
// Don't use the source callable if there is a manual model for the
// target
not exists(SummarizedCallable sc |
sc.asSummarizedCallable() = target and
summarizedCallableIsManual(sc)
) and
result.asSourceCallable() = target
or
// When there is no function body, or when we have a manual model then
// we dispatch to the summary.
(not target.hasDefinition() or summarizedCallableIsManual(result)) and
result.asSummarizedCallable() = target
)
}
Expand Down
77 changes: 59 additions & 18 deletions cpp/ql/test/library-tests/dataflow/external-models/flow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,33 @@ edges
| asio_streams.cpp:100:44:100:62 | call to buffer | asio_streams.cpp:103:29:103:39 | *send_buffer | provenance | Sink:MaD:6 |
| asio_streams.cpp:100:64:100:71 | *send_str | asio_streams.cpp:56:18:56:23 | [summary param] *0 in buffer | provenance | |
| asio_streams.cpp:100:64:100:71 | *send_str | asio_streams.cpp:100:44:100:62 | call to buffer | provenance | MaD:10 |
| 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 |
| test.cpp:7:10:7:18 | call to ymlSource | test.cpp:7:10:7:18 | call to ymlSource | provenance | Src:MaD:967 |
| test.cpp:7:10:7:18 | call to ymlSource | test.cpp:11:10:11:10 | x | provenance | Sink:MaD:968 |
| test.cpp:7:10:7:18 | call to ymlSource | test.cpp:13:18:13:18 | x | provenance | |
| test.cpp:13:10:13:16 | call to ymlStep | test.cpp:13:10:13:16 | call to ymlStep | provenance | |
| test.cpp:13:10:13:16 | call to ymlStep | test.cpp:15:10:15:10 | y | provenance | Sink:MaD:968 |
| test.cpp:13:18:13:18 | x | test.cpp:4:5:4:11 | [summary param] 0 in ymlStep | provenance | |
| test.cpp:13:18:13:18 | x | test.cpp:13:10:13:16 | call to ymlStep | provenance | MaD:969 |
| 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 |
| 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 |
| 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 |
| test.cpp:7:47:7:52 | value2 | test.cpp:7:64:7:69 | value2 | provenance | |
| test.cpp:7:64:7:69 | value2 | test.cpp:7:5:7:30 | *ymlStepGenerated_with_body | provenance | |
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:10:10:10:18 | call to ymlSource | provenance | Src:MaD:967 |
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:14:10:14:10 | x | provenance | Sink:MaD:968 |
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:17:24:17:24 | x | provenance | |
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:21:27:21:27 | x | provenance | |
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:25:35:25:35 | x | provenance | |
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:32:41:32:41 | x | provenance | |
| test.cpp:17:10:17:22 | call to ymlStepManual | test.cpp:17:10:17:22 | call to ymlStepManual | provenance | |
| test.cpp:17:10:17:22 | call to ymlStepManual | test.cpp:18:10:18:10 | y | provenance | Sink:MaD:968 |
| test.cpp:17:24:17:24 | x | test.cpp:4:5:4:17 | [summary param] 0 in ymlStepManual | provenance | |
| test.cpp:17:24:17:24 | x | test.cpp:17:10:17:22 | call to ymlStepManual | provenance | MaD:969 |
| test.cpp:21:10:21:25 | call to ymlStepGenerated | test.cpp:21:10:21:25 | call to ymlStepGenerated | provenance | |
| test.cpp:21:10:21:25 | call to ymlStepGenerated | test.cpp:22:10:22:10 | z | provenance | Sink:MaD:968 |
| test.cpp:21:27:21:27 | x | test.cpp:5:5:5:20 | [summary param] 0 in ymlStepGenerated | provenance | |
| test.cpp:21:27:21:27 | x | test.cpp:21:10:21:25 | call to ymlStepGenerated | provenance | MaD:970 |
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | test.cpp:25:11:25:33 | call to ymlStepManual_with_body | provenance | |
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | test.cpp:26:10:26:11 | y2 | provenance | Sink:MaD:968 |
| test.cpp:25:35:25:35 | x | test.cpp:6:5:6:27 | [summary param] 0 in ymlStepManual_with_body | provenance | |
| test.cpp:25:35:25:35 | x | test.cpp:25:11:25:33 | call to ymlStepManual_with_body | provenance | MaD:971 |
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | provenance | |
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | test.cpp:33:10:33:11 | z2 | provenance | Sink:MaD:968 |
| test.cpp:32:41:32:41 | x | test.cpp:7:47:7:52 | value2 | provenance | |
| test.cpp:32:41:32:41 | x | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | provenance | |
nodes
| asio_streams.cpp:56:18:56:23 | [summary param] *0 in buffer | semmle.label | [summary param] *0 in buffer |
| asio_streams.cpp:56:18:56:23 | [summary] to write: ReturnValue in buffer | semmle.label | [summary] to write: ReturnValue in buffer |
Expand All @@ -31,15 +50,37 @@ nodes
| asio_streams.cpp:100:64:100:71 | *send_str | semmle.label | *send_str |
| asio_streams.cpp:101:7:101:17 | send_buffer | semmle.label | send_buffer |
| asio_streams.cpp:103:29:103:39 | *send_buffer | semmle.label | *send_buffer |
| test.cpp:4:5:4:11 | [summary param] 0 in ymlStep | semmle.label | [summary param] 0 in ymlStep |
| test.cpp:4:5:4:11 | [summary] to write: ReturnValue in ymlStep | semmle.label | [summary] to write: ReturnValue in ymlStep |
| test.cpp:7:10:7:18 | call to ymlSource | semmle.label | call to ymlSource |
| test.cpp:7:10:7:18 | call to ymlSource | semmle.label | call to ymlSource |
| test.cpp:11:10:11:10 | x | semmle.label | x |
| test.cpp:13:10:13:16 | call to ymlStep | semmle.label | call to ymlStep |
| test.cpp:13:10:13:16 | call to ymlStep | semmle.label | call to ymlStep |
| test.cpp:13:18:13:18 | x | semmle.label | x |
| test.cpp:15:10:15:10 | y | semmle.label | y |
| test.cpp:4:5:4:17 | [summary param] 0 in ymlStepManual | semmle.label | [summary param] 0 in ymlStepManual |
| test.cpp:4:5:4:17 | [summary] to write: ReturnValue in ymlStepManual | semmle.label | [summary] to write: ReturnValue in ymlStepManual |
| test.cpp:5:5:5:20 | [summary param] 0 in ymlStepGenerated | semmle.label | [summary param] 0 in ymlStepGenerated |
| test.cpp:5:5:5:20 | [summary] to write: ReturnValue in ymlStepGenerated | semmle.label | [summary] to write: ReturnValue in ymlStepGenerated |
| test.cpp:6:5:6:27 | [summary param] 0 in ymlStepManual_with_body | semmle.label | [summary param] 0 in ymlStepManual_with_body |
| test.cpp:6:5:6:27 | [summary] to write: ReturnValue in ymlStepManual_with_body | semmle.label | [summary] to write: ReturnValue in ymlStepManual_with_body |
| test.cpp:7:5:7:30 | *ymlStepGenerated_with_body | semmle.label | *ymlStepGenerated_with_body |
| test.cpp:7:47:7:52 | value2 | semmle.label | value2 |
| test.cpp:7:64:7:69 | value2 | semmle.label | value2 |
| test.cpp:10:10:10:18 | call to ymlSource | semmle.label | call to ymlSource |
| test.cpp:10:10:10:18 | call to ymlSource | semmle.label | call to ymlSource |
| test.cpp:14:10:14:10 | x | semmle.label | x |
| test.cpp:17:10:17:22 | call to ymlStepManual | semmle.label | call to ymlStepManual |
| test.cpp:17:10:17:22 | call to ymlStepManual | semmle.label | call to ymlStepManual |
| test.cpp:17:24:17:24 | x | semmle.label | x |
| test.cpp:18:10:18:10 | y | semmle.label | y |
| test.cpp:21:10:21:25 | call to ymlStepGenerated | semmle.label | call to ymlStepGenerated |
| test.cpp:21:10:21:25 | call to ymlStepGenerated | semmle.label | call to ymlStepGenerated |
| test.cpp:21:27:21:27 | x | semmle.label | x |
| test.cpp:22:10:22:10 | z | semmle.label | z |
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | semmle.label | call to ymlStepManual_with_body |
| test.cpp:25:11:25:33 | call to ymlStepManual_with_body | semmle.label | call to ymlStepManual_with_body |
| test.cpp:25:35:25:35 | x | semmle.label | x |
| test.cpp:26:10:26:11 | y2 | semmle.label | y2 |
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | semmle.label | call to ymlStepGenerated_with_body |
| test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body | semmle.label | call to ymlStepGenerated_with_body |
| test.cpp:32:41:32:41 | x | semmle.label | x |
| test.cpp:33:10:33:11 | z2 | semmle.label | z2 |
subpaths
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
5 changes: 4 additions & 1 deletion cpp/ql/test/library-tests/dataflow/external-models/flow.ext.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@ extensions:
pack: codeql/cpp-all
extensible: summaryModel
data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
- ["", "", False, "ymlStep", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["", "", False, "ymlStepManual", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["", "", False, "ymlStepGenerated", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]
- ["", "", False, "ymlStepManual_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["", "", False, "ymlStepGenerated_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]
11 changes: 8 additions & 3 deletions cpp/ql/test/library-tests/dataflow/external-models/sinks.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
| asio_streams.cpp:93:29:93:39 | *recv_buffer | remote-sink |
| asio_streams.cpp:103:29:103:39 | *send_buffer | remote-sink |
| test.cpp:9:10:9:10 | 0 | test-sink |
| test.cpp:11:10:11:10 | x | test-sink |
| test.cpp:15:10:15:10 | y | test-sink |
| test.cpp:12:10:12:10 | 0 | test-sink |
| test.cpp:14:10:14:10 | x | test-sink |
| test.cpp:18:10:18:10 | y | test-sink |
| test.cpp:22:10:22:10 | z | test-sink |
| test.cpp:26:10:26:11 | y2 | test-sink |
| test.cpp:29:10:29:11 | y3 | test-sink |
| test.cpp:33:10:33:11 | z2 | test-sink |
| test.cpp:36:10:36:11 | z3 | test-sink |
2 changes: 1 addition & 1 deletion cpp/ql/test/library-tests/dataflow/external-models/sources.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
| asio_streams.cpp:87:34:87:44 | read_until output argument | remote |
| test.cpp:7:10:7:18 | call to ymlSource | local |
| test.cpp:10:10:10:18 | call to ymlSource | local |
7 changes: 6 additions & 1 deletion cpp/ql/test/library-tests/dataflow/external-models/steps.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
| asio_streams.cpp:100:64:100:71 | *send_str | asio_streams.cpp:100:44:100:62 | call to buffer |
| test.cpp:13:18:13:18 | x | test.cpp:13:10:13:16 | call to ymlStep |
| test.cpp:17:24:17:24 | x | test.cpp:17:10:17:22 | call to ymlStepManual |
| test.cpp:21:27:21:27 | x | test.cpp:21:10:21:25 | call to ymlStepGenerated |
| test.cpp:25:35:25:35 | x | test.cpp:25:11:25:33 | call to ymlStepManual_with_body |
| test.cpp:28:35:28:35 | 0 | test.cpp:28:11:28:33 | call to ymlStepManual_with_body |
| test.cpp:32:38:32:38 | 0 | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body |
| test.cpp:35:38:35:38 | x | test.cpp:35:11:35:36 | call to ymlStepGenerated_with_body |
5 changes: 4 additions & 1 deletion cpp/ql/test/library-tests/dataflow/external-models/steps.ext.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@ extensions:
pack: codeql/cpp-all
extensible: summaryModel
data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
- ["", "", False, "ymlStep", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["", "", False, "ymlStepManual", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["", "", False, "ymlStepGenerated", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]
- ["", "", False, "ymlStepManual_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["", "", False, "ymlStepGenerated_with_body", "", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]
27 changes: 24 additions & 3 deletions cpp/ql/test/library-tests/dataflow/external-models/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@

int ymlSource();
void ymlSink(int value);
int ymlStep(int value);
int ymlStepManual(int value);
int ymlStepGenerated(int value);
int ymlStepManual_with_body(int value1, int value2) { return value2; }
int ymlStepGenerated_with_body(int value, int value2) { return value2; }

void test() {
int x = ymlSource();
Expand All @@ -10,7 +13,25 @@ void test() {

ymlSink(x); // $ ir

int y = ymlStep(x);

// ymlStepManual is manually modeled so we should always use the model
int y = ymlStepManual(x);
ymlSink(y); // $ ir

// ymlStepGenerated is modeled by the model generator so we should use the model only if there is no body
int z = ymlStepGenerated(x);
ymlSink(z); // $ ir

// ymlStepManual_with_body is manually modeled so we should always use the model
int y2 = ymlStepManual_with_body(x, 0);
ymlSink(y2); // $ ir

int y3 = ymlStepManual_with_body(0, x);
ymlSink(y3); // clean

// ymlStepGenerated_with_body is modeled by the model generator so we should use the model only if there is no body
int z2 = ymlStepGenerated_with_body(0, x);
ymlSink(z2); // $ ir

int z3 = ymlStepGenerated_with_body(x, 0);
ymlSink(z3); // clean
}