Skip to content

Commit dcc0481

Browse files
jbjjbj
authored
Merge pull request #4717 from criemen/escapetree-temp-objects
C++: Improve EscapesTree.qll analysis in the presence of temporary objects
2 parents aa45920 + 0b8403f commit dcc0481

File tree

10 files changed

+72
-2
lines changed

10 files changed

+72
-2
lines changed

‎cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,15 @@ private predicate addressMayEscapeMutablyAt(Expr e) {
222222
// If the address has been cast to an integral type, conservatively assume that it may eventually be cast back to a
223223
// pointer to non-const type.
224224
t instanceof IntegralType
225+
or
226+
// If we go through a temporary object step, we can take a reference to a temporary const pointer
227+
// object, where the pointer doesn't point to a const value
228+
exists(TemporaryObjectExpr temp, PointerType pt |
229+
temp.getConversion() = e.(ReferenceToExpr) and
230+
pt = temp.getType().stripTopLevelSpecifiers()
231+
|
232+
not pt.getBaseType().isConst()
233+
)
225234
)
226235
}
227236

@@ -249,7 +258,7 @@ private predicate addressFromVariableAccess(VariableAccess va, Expr e) {
249258
// `e` could be a pointer that is converted to a reference as the final step,
250259
// meaning that we pass a value that is two dereferences away from referring
251260
// to `va`. This happens, for example, with `void std::vector::push_back(T&&
252-
// value);` when called as `v.push_back(&x)`, for a static variable `x`. It
261+
// value);` when called as `v.push_back(&x)`, for a variable `x`. It
253262
// can also happen when taking a reference to a const pointer to a
254263
// (potentially non-const) value.
255264
exists(Expr pointerValue |

‎cpp/ql/test/library-tests/defuse/definition.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
| addressOf.cpp:23:9:23:9 | i | addressOf.cpp:24:22:24:30 | call to move |
22
| addressOf.cpp:23:9:23:9 | i | addressOf.cpp:25:25:25:26 | & ... |
3+
| addressOf.cpp:23:9:23:9 | i | addressOf.cpp:26:31:26:32 | & ... |
34
| addressOf.cpp:31:23:31:23 | i | addressOf.cpp:32:18:32:19 | & ... |
45
| addressOf.cpp:31:23:31:23 | i | addressOf.cpp:34:18:34:33 | ... ? ... : ... |
56
| addressOf.cpp:31:23:31:23 | i | addressOf.cpp:35:18:35:23 | & ... |
@@ -77,6 +78,11 @@
7778
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:46:11:46:11 | n |
7879
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:7 | ... ++ |
7980
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i |
81+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 |
82+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:34:61:35 | & ... |
83+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 |
84+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:74:10:74:11 | 2 |
85+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:75:35:75:36 | & ... |
8086
| test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... |
8187
| test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... |
8288
| test.cpp:4:7:4:7 | a | test.cpp:19:5:19:9 | ... = ... |

‎cpp/ql/test/library-tests/defuse/definitionUsePair.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@
7575
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:53:22:53:22 | i |
7676
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:7 | ... ++ | pass_by_ref.cpp:53:22:53:22 | i |
7777
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i | pass_by_ref.cpp:54:10:54:10 | i |
78+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:61:35:61:35 | x |
79+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:34:61:35 | & ... | pass_by_ref.cpp:62:10:62:10 | x |
80+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:67:34:67:34 | p |
81+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:68:10:68:10 | p |
82+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:74:10:74:11 | 2 | pass_by_ref.cpp:75:36:75:36 | x |
83+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:75:35:75:36 | & ... | pass_by_ref.cpp:76:10:76:10 | x |
7884
| test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | test.cpp:9:7:9:7 | a |
7985
| test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:14:7:14:7 | a |
8086
| test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:18:7:18:7 | a |

‎cpp/ql/test/library-tests/defuse/exprDefinition.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
| pass_by_ref.cpp:31:7:31:9 | arr | pass_by_ref.cpp:31:15:31:18 | {...} | pass_by_ref.cpp:31:15:31:18 | {...} |
1818
| pass_by_ref.cpp:37:7:37:9 | arr | pass_by_ref.cpp:37:15:37:18 | {...} | pass_by_ref.cpp:37:15:37:18 | {...} |
1919
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:46:11:46:11 | n |
20+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:60:10:60:11 | 2 |
21+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:66:12:66:18 | 0 |
22+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:74:10:74:11 | 2 | pass_by_ref.cpp:74:10:74:11 | 2 |
2023
| test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | test.cpp:5:7:5:8 | a0 |
2124
| test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:13:7:13:7 | b |
2225
| test.cpp:4:7:4:7 | a | test.cpp:19:5:19:9 | ... = ... | test.cpp:19:9:19:9 | 1 |

‎cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
| addressOf.cpp:14:28:14:29 | d_ | non-const address |
22
| addressOf.cpp:24:32:24:32 | i | non-const address |
33
| addressOf.cpp:25:26:25:26 | i | non-const address |
4-
| addressOf.cpp:26:32:26:32 | i | const address |
4+
| addressOf.cpp:26:32:26:32 | i | non-const address |
55
| addressOf.cpp:32:19:32:19 | i | non-const address |
66
| addressOf.cpp:34:18:34:18 | i | |
77
| addressOf.cpp:34:23:34:23 | i | non-const address |
@@ -151,6 +151,12 @@
151151
| pass_by_ref.cpp:49:5:49:5 | i | |
152152
| pass_by_ref.cpp:53:22:53:22 | i | non-const address |
153153
| pass_by_ref.cpp:54:10:54:10 | i | |
154+
| pass_by_ref.cpp:61:35:61:35 | x | non-const address |
155+
| pass_by_ref.cpp:62:10:62:10 | x | |
156+
| pass_by_ref.cpp:67:34:67:34 | p | const address |
157+
| pass_by_ref.cpp:68:10:68:10 | p | |
158+
| pass_by_ref.cpp:75:36:75:36 | x | non-const address |
159+
| pass_by_ref.cpp:76:10:76:10 | x | |
154160
| test.cpp:5:3:5:3 | a | |
155161
| test.cpp:5:7:5:8 | a0 | |
156162
| test.cpp:6:3:6:3 | b | |

‎cpp/ql/test/library-tests/defuse/pass_by_ref.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,25 @@ int afterIf(int n) {
5353
referenceParameter(i);
5454
return i;
5555
}
56+
57+
void constPointerReferenceParameter(int * const & pRef);
58+
59+
int temporaryObject() {
60+
int x = 2;
61+
constPointerReferenceParameter(&x);
62+
return x;
63+
}
64+
65+
int * noTemporaryObject() {
66+
int *p = nullptr;
67+
constPointerReferenceParameter(p);
68+
return p;
69+
}
70+
71+
void pointerRvalueReferenceParameter(int * && pRef);
72+
73+
int temporaryObject2() {
74+
int x = 2;
75+
pointerRvalueReferenceParameter(&x);
76+
return x;
77+
}

‎cpp/ql/test/library-tests/defuse/useOfVar.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@
118118
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:5 | i |
119119
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i |
120120
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:54:10:54:10 | i |
121+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:35:61:35 | x |
122+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:62:10:62:10 | x |
123+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:67:34:67:34 | p |
124+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:68:10:68:10 | p |
125+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:75:36:75:36 | x |
126+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:76:10:76:10 | x |
121127
| test.cpp:3:16:3:17 | a0 | test.cpp:5:7:5:8 | a0 |
122128
| test.cpp:3:24:3:25 | b0 | test.cpp:6:7:6:8 | b0 |
123129
| test.cpp:3:32:3:33 | c0 | test.cpp:7:7:7:8 | c0 |

‎cpp/ql/test/library-tests/defuse/useOfVarActual.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@
5656
| pass_by_ref.cpp:45:17:45:17 | n | pass_by_ref.cpp:48:7:48:7 | n |
5757
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:5 | i |
5858
| pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:54:10:54:10 | i |
59+
| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:62:10:62:10 | x |
60+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:68:10:68:10 | p |
61+
| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:76:10:76:10 | x |
5962
| test.cpp:3:16:3:17 | a0 | test.cpp:5:7:5:8 | a0 |
6063
| test.cpp:3:24:3:25 | b0 | test.cpp:6:7:6:8 | b0 |
6164
| test.cpp:3:32:3:33 | c0 | test.cpp:7:7:7:8 | c0 |

‎cpp/ql/test/library-tests/defuse/useUsePair.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
| pass_by_ref.cpp:21:7:21:8 | i2 | pass_by_ref.cpp:24:26:24:27 | i2 | pass_by_ref.cpp:27:39:27:40 | i2 |
2626
| pass_by_ref.cpp:21:7:21:8 | i2 | pass_by_ref.cpp:25:27:25:28 | i2 | pass_by_ref.cpp:27:39:27:40 | i2 |
2727
| pass_by_ref.cpp:45:17:45:17 | n | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:48:7:48:7 | n |
28+
| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:67:34:67:34 | p | pass_by_ref.cpp:68:10:68:10 | p |
2829
| test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:18:7:18:7 | a |
2930
| test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:24:7:24:7 | a |
3031
| test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:28:11:28:11 | a |

‎cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,11 @@ void negativeZero4(int val) {
116116
if (val == 0) // GOOD [NO LONGER REPORTED]
117117
;
118118
}
119+
120+
void f(int *const &ref_to_ptr);
121+
122+
void testTempObject() {
123+
int x = 0;
124+
f(&x);
125+
if (x > 0) {} // GOOD [NO LONGER REPORTED]
126+
}

0 commit comments

Comments
 (0)