Skip to content

Commit 5950af3

Browse files
paldepindpaldepind
authored
Merge pull request github#17351 from paldepind/swap-member-data-flow
C++: Make swap member functions data-flow functions
2 parents b6e38ff + f066f21 commit 5950af3

File tree

16 files changed

+97
-107
lines changed

16 files changed

+97
-107
lines changed

‎cpp/ql/lib/change-notes/2024-09-04-swap-data-flow.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added a data flow model for `swap` member functions, which were previously modeled as taint tracking functions. This change improves the precision of queries where flow through `swap` member functions might affect the results.

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ private class Swap extends DataFlowFunction {
2626
* obj1.swap(obj2)
2727
* ```
2828
*/
29-
private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction {
29+
private class MemberSwap extends DataFlowFunction, MemberFunction, AliasFunction {
3030
MemberSwap() {
3131
this.hasName("swap") and
3232
this.getNumberOfParameters() = 1 and
3333
this.getParameter(0).getType().(ReferenceType).getBaseType().getUnspecifiedType() =
3434
this.getDeclaringType()
3535
}
3636

37-
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
37+
override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
3838
input.isQualifierObject() and
3939
output.isParameterDeref(0)
4040
or

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ argHasPostUpdate
3333
| test.cpp:67:29:67:35 | source1 | ArgumentNode is missing PostUpdateNode. |
3434
| test.cpp:813:19:813:35 | * ... | ArgumentNode is missing PostUpdateNode. |
3535
| test.cpp:848:23:848:25 | rpx | ArgumentNode is missing PostUpdateNode. |
36-
| test.cpp:1065:19:1065:21 | * ... | ArgumentNode is missing PostUpdateNode. |
36+
| test.cpp:1093:19:1093:21 | * ... | ArgumentNode is missing PostUpdateNode. |
3737
postWithInFlow
3838
| BarrierGuard.cpp:49:6:49:6 | x [post update] | PostUpdateNode should not be the target of local flow. |
3939
| BarrierGuard.cpp:60:7:60:7 | x [post update] | PostUpdateNode should not be the target of local flow. |
@@ -167,15 +167,17 @@ postWithInFlow
167167
| test.cpp:932:5:932:19 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
168168
| test.cpp:932:6:932:19 | global_pointer [inner post update] | PostUpdateNode should not be the target of local flow. |
169169
| test.cpp:1045:9:1045:11 | ref arg buf | PostUpdateNode should not be the target of local flow. |
170-
| test.cpp:1059:5:1059:11 | content [post update] | PostUpdateNode should not be the target of local flow. |
171-
| test.cpp:1060:9:1060:9 | a [inner post update] | PostUpdateNode should not be the target of local flow. |
172-
| test.cpp:1064:5:1064:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
173-
| test.cpp:1064:6:1064:7 | pp [inner post update] | PostUpdateNode should not be the target of local flow. |
174-
| test.cpp:1070:53:1070:53 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
175-
| test.cpp:1080:3:1080:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
176-
| test.cpp:1080:4:1080:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
177-
| test.cpp:1081:3:1081:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
178-
| test.cpp:1081:4:1081:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
170+
| test.cpp:1066:5:1066:5 | i [post update] | PostUpdateNode should not be the target of local flow. |
171+
| test.cpp:1069:5:1069:5 | i [post update] | PostUpdateNode should not be the target of local flow. |
172+
| test.cpp:1087:5:1087:11 | content [post update] | PostUpdateNode should not be the target of local flow. |
173+
| test.cpp:1088:9:1088:9 | a [inner post update] | PostUpdateNode should not be the target of local flow. |
174+
| test.cpp:1092:5:1092:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
175+
| test.cpp:1092:6:1092:7 | pp [inner post update] | PostUpdateNode should not be the target of local flow. |
176+
| test.cpp:1098:53:1098:53 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
177+
| test.cpp:1108:3:1108:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
178+
| test.cpp:1108:4:1108:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
179+
| test.cpp:1109:3:1109:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
180+
| test.cpp:1109:4:1109:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
179181
viableImplInCallContextTooLarge
180182
uniqueParameterNodeAtPosition
181183
uniqueParameterNodePosition

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ postWithInFlow
2626
| test.cpp:400:10:400:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
2727
| test.cpp:407:10:407:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
2828
| test.cpp:1045:9:1045:11 | memset output argument | PostUpdateNode should not be the target of local flow. |
29+
| test.cpp:1076:2:1076:3 | swap output argument | PostUpdateNode should not be the target of local flow. |
30+
| test.cpp:1076:10:1076:11 | swap output argument | PostUpdateNode should not be the target of local flow. |
31+
| test.cpp:1077:2:1077:3 | swap output argument | PostUpdateNode should not be the target of local flow. |
32+
| test.cpp:1077:10:1077:11 | swap output argument | PostUpdateNode should not be the target of local flow. |
2933
viableImplInCallContextTooLarge
3034
uniqueParameterNodeAtPosition
3135
uniqueParameterNodePosition

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow-ir.expected

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,12 @@
202202
| test.cpp:489:23:489:29 | *content | test.cpp:490:8:490:17 | * ... |
203203
| test.cpp:489:23:489:29 | content | test.cpp:489:23:489:29 | content |
204204
| test.cpp:489:23:489:29 | content | test.cpp:490:9:490:17 | p_content |
205-
| test.cpp:1058:12:1058:12 | definition of a | test.cpp:1059:3:1059:3 | *a |
206-
| test.cpp:1059:3:1059:3 | *a | test.cpp:1060:8:1060:9 | *& ... |
207-
| test.cpp:1059:3:1059:3 | *a [post update] | test.cpp:1060:8:1060:9 | *& ... |
208-
| test.cpp:1059:3:1059:3 | a | test.cpp:1060:8:1060:9 | & ... |
209-
| test.cpp:1059:3:1059:3 | a [post update] | test.cpp:1060:8:1060:9 | & ... |
210-
| test.cpp:1059:15:1059:21 | 0 | test.cpp:1059:3:1059:21 | ... = ... |
211-
| test.cpp:1059:15:1059:21 | *0 | test.cpp:1059:3:1059:21 | *... = ... |
212-
| test.cpp:1060:9:1060:9 | *a | test.cpp:1060:8:1060:9 | *& ... |
213-
| test.cpp:1060:9:1060:9 | a | test.cpp:1060:8:1060:9 | & ... |
205+
| test.cpp:1086:12:1086:12 | definition of a | test.cpp:1087:3:1087:3 | *a |
206+
| test.cpp:1087:3:1087:3 | *a | test.cpp:1088:8:1088:9 | *& ... |
207+
| test.cpp:1087:3:1087:3 | *a [post update] | test.cpp:1088:8:1088:9 | *& ... |
208+
| test.cpp:1087:3:1087:3 | a | test.cpp:1088:8:1088:9 | & ... |
209+
| test.cpp:1087:3:1087:3 | a [post update] | test.cpp:1088:8:1088:9 | & ... |
210+
| test.cpp:1087:15:1087:21 | 0 | test.cpp:1087:3:1087:21 | ... = ... |
211+
| test.cpp:1087:15:1087:21 | *0 | test.cpp:1087:3:1087:21 | *... = ... |
212+
| test.cpp:1088:9:1088:9 | *a | test.cpp:1088:8:1088:9 | *& ... |
213+
| test.cpp:1088:9:1088:9 | a | test.cpp:1088:8:1088:9 | & ... |

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,10 @@ WARNING: module 'DataFlow' has been deprecated and may be removed in future (loc
8181
| test.cpp:488:21:488:21 | s [post update] | test.cpp:489:20:489:20 | s |
8282
| test.cpp:488:24:488:30 | ref arg content | test.cpp:489:23:489:29 | content |
8383
| test.cpp:489:23:489:29 | content | test.cpp:490:9:490:17 | p_content |
84-
| test.cpp:1058:12:1058:12 | a | test.cpp:1059:3:1059:3 | a |
85-
| test.cpp:1058:12:1058:12 | a | test.cpp:1060:9:1060:9 | a |
86-
| test.cpp:1059:3:1059:3 | a [post update] | test.cpp:1060:9:1060:9 | a |
87-
| test.cpp:1059:3:1059:21 | ... = ... | test.cpp:1059:5:1059:11 | content [post update] |
88-
| test.cpp:1059:15:1059:21 | 0 | test.cpp:1059:3:1059:21 | ... = ... |
89-
| test.cpp:1060:8:1060:9 | ref arg & ... | test.cpp:1060:9:1060:9 | a [inner post update] |
90-
| test.cpp:1060:9:1060:9 | a | test.cpp:1060:8:1060:9 | & ... |
84+
| test.cpp:1086:12:1086:12 | a | test.cpp:1087:3:1087:3 | a |
85+
| test.cpp:1086:12:1086:12 | a | test.cpp:1088:9:1088:9 | a |
86+
| test.cpp:1087:3:1087:3 | a [post update] | test.cpp:1088:9:1088:9 | a |
87+
| test.cpp:1087:3:1087:21 | ... = ... | test.cpp:1087:5:1087:11 | content [post update] |
88+
| test.cpp:1087:15:1087:21 | 0 | test.cpp:1087:3:1087:21 | ... = ... |
89+
| test.cpp:1088:8:1088:9 | ref arg & ... | test.cpp:1088:9:1088:9 | a [inner post update] |
90+
| test.cpp:1088:9:1088:9 | a | test.cpp:1088:8:1088:9 | & ... |

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,11 @@ astFlow
127127
| test.cpp:842:11:842:16 | call to source | test.cpp:844:8:844:8 | y |
128128
| test.cpp:846:13:846:27 | call to indirect_source | test.cpp:848:23:848:25 | rpx |
129129
| test.cpp:860:54:860:59 | call to source | test.cpp:861:10:861:37 | static_local_pointer_dynamic |
130-
| test.cpp:1058:12:1058:12 | a | test.cpp:1060:8:1060:9 | & ... |
130+
| test.cpp:1066:9:1066:14 | call to source | test.cpp:1072:10:1072:10 | i |
131+
| test.cpp:1066:9:1066:14 | call to source | test.cpp:1080:10:1080:10 | i |
132+
| test.cpp:1069:9:1069:14 | call to source | test.cpp:1074:10:1074:10 | i |
133+
| test.cpp:1069:9:1069:14 | call to source | test.cpp:1082:10:1082:10 | i |
134+
| test.cpp:1086:12:1086:12 | a | test.cpp:1088:8:1088:9 | & ... |
131135
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
132136
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |
133137
| true_upon_entry.cpp:33:11:33:16 | call to source | true_upon_entry.cpp:39:8:39:8 | x |
@@ -314,7 +318,11 @@ irFlow
314318
| test.cpp:1021:18:1021:32 | *call to indirect_source | test.cpp:1031:19:1031:28 | *translated |
315319
| test.cpp:1045:14:1045:19 | call to source | test.cpp:1046:7:1046:10 | * ... |
316320
| test.cpp:1052:13:1052:27 | *call to indirect_source | test.cpp:1054:7:1054:11 | * ... |
317-
| test.cpp:1089:27:1089:34 | call to source | test.cpp:1089:27:1089:34 | call to source |
321+
| test.cpp:1066:9:1066:14 | call to source | test.cpp:1072:10:1072:10 | i |
322+
| test.cpp:1066:9:1066:14 | call to source | test.cpp:1079:10:1079:10 | i |
323+
| test.cpp:1069:9:1069:14 | call to source | test.cpp:1074:10:1074:10 | i |
324+
| test.cpp:1069:9:1069:14 | call to source | test.cpp:1081:10:1081:10 | i |
325+
| test.cpp:1117:27:1117:34 | call to source | test.cpp:1117:27:1117:34 | call to source |
318326
| true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
319327
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
320328
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,34 @@ void test_realloc() {
10541054
sink(*dest); // $ ir, MISSING: ast
10551055
}
10561056

1057+
struct MyInt {
1058+
int i;
1059+
MyInt();
1060+
void swap(MyInt &j);
1061+
};
1062+
1063+
void test_member_swap() {
1064+
MyInt s1;
1065+
MyInt s2;
1066+
s2.i = source();
1067+
MyInt s3;
1068+
MyInt s4;
1069+
s4.i = source();
1070+
1071+
sink(s1.i);
1072+
sink(s2.i); // $ ast,ir
1073+
sink(s3.i);
1074+
sink(s4.i); // $ ast,ir
1075+
1076+
s1.swap(s2);
1077+
s4.swap(s3);
1078+
1079+
sink(s1.i); // $ ir
1080+
sink(s2.i); // $ SPURIOUS: ast
1081+
sink(s3.i); // $ ir
1082+
sink(s4.i); // $ SPURIOUS: ast
1083+
}
1084+
10571085
void flow_out_of_address_with_local_flow() {
10581086
MyStruct a;
10591087
a.content = nullptr;

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,5 @@ incorrectBaseType
5151
| test.cpp:848:23:848:25 | rpx | Expected 'Node.getType()' to be int, but it was int * |
5252
| test.cpp:854:10:854:36 | * ... | Expected 'Node.getType()' to be const int, but it was int |
5353
| test.cpp:867:10:867:30 | * ... | Expected 'Node.getType()' to be const int, but it was int |
54-
| test.cpp:1070:52:1070:53 | *& ... | Expected 'Node.getType()' to be char, but it was char * |
54+
| test.cpp:1098:52:1098:53 | *& ... | Expected 'Node.getType()' to be char, but it was char * |
5555
failures

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,5 @@
5454
| test.cpp:796:12:796:12 | a | test.cpp:797:20:797:20 | a |
5555
| test.cpp:796:12:796:12 | a | test.cpp:797:31:797:31 | a |
5656
| test.cpp:796:12:796:12 | a | test.cpp:798:17:798:17 | a |
57-
| test.cpp:1058:12:1058:12 | a | test.cpp:1059:3:1059:3 | a |
58-
| test.cpp:1058:12:1058:12 | a | test.cpp:1060:9:1060:9 | a |
57+
| test.cpp:1086:12:1086:12 | a | test.cpp:1087:3:1087:3 | a |
58+
| test.cpp:1086:12:1086:12 | a | test.cpp:1088:9:1088:9 | a |

0 commit comments

Comments
 (0)