Skip to content

Commit 4af6ba6

Browse files
authored
Merge pull request #22764 from geoffw0/destructorrelease
C++: Fix false positives for cpp/resource-not-released-in-destructor
2 parents 155341e + 337ed63 commit 4af6ba6

5 files changed

Lines changed: 84 additions & 3 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cpp/resource-not-released-in-destructor` query now recognizes resources released through local aliases, reducing false-positive results.

‎cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ Expr exprOrDereference(Expr e) {
8989
*/
9090
private predicate exprReleases(Expr e, Expr released, string kind) {
9191
// `e` is a call to a release function and `released` is the released argument
92-
releaseExpr(e, released, kind)
92+
releaseExpr(e, pragma[only_bind_into](globalValueNumber(released).getAnExpr()), kind)
9393
or
9494
exists(int arg, VariableAccess access, Function f |
9595
// `e` is a call to a function that releases one of it's parameters,

‎cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 79/AV Rule 79.expected‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
| DeleteThis.cpp:127:3:127:20 | ... = ... | Resource d is acquired by class MyClass9 but not released anywhere in this class. |
1515
| ExternalOwners.cpp:49:3:49:20 | ... = ... | Resource a is acquired by class MyScreen but not released anywhere in this class. |
1616
| Lambda.cpp:24:3:24:21 | ... = ... | Resource r4 is acquired by class testLambda but not released anywhere in this class. |
17-
| ListDelete.cpp:21:3:21:21 | ... = ... | Resource first is acquired by class MyThingColection but not released anywhere in this class. |
1817
| NoDestructor.cpp:23:3:23:20 | ... = ... | Resource n is acquired by class MyClass5 but not released anywhere in this class. |
1918
| PlacementNew.cpp:36:3:36:36 | ... = ... | Resource p1 is acquired by class MyTestForPlacementNew but not released anywhere in this class. |
2019
| SelfRegistering.cpp:25:3:25:24 | ... = ... | Resource side is acquired by class MyOwner but not released anywhere in this class. |
20+
| TemplateDestructor.cpp:55:5:55:23 | ... = ... | Resource ptr is acquired by class OverwrittenAlias but not released anywhere in this class. |
2121
| Variants.cpp:26:3:26:13 | ... = ... | Resource f is acquired by class MyClass4 but not released anywhere in this class. |
2222
| Variants.cpp:69:3:69:17 | ... = ... | Resource a is acquired by class MyClass6 but not released anywhere in this class. |
2323
| Variants.cpp:70:3:70:36 | ... = ... | Resource b is acquired by class MyClass6 but not released anywhere in this class. |

‎cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 79/ListDelete.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class MyThingColection
1818
{
1919
public:
2020
MyThingColection() {
21-
first = new MyThing; // $ SPURIOUS: Alert // GOOD (all deleted in destructor) [FALSE POSITIVE]
21+
first = new MyThing; // GOOD (all deleted in destructor)
2222

2323
first->next = new MyThing; // GOOD (all deleted in destructor)
2424

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
template <typename T>
2+
class TemplateArray {
3+
public:
4+
TemplateArray() : ptr(nullptr) {}
5+
6+
~TemplateArray() {
7+
reset();
8+
}
9+
10+
void init(unsigned size) {
11+
ptr = new T[size]; // GOOD
12+
}
13+
14+
void reset() {
15+
T *tmp = ptr;
16+
ptr = nullptr;
17+
delete[] tmp;
18+
}
19+
20+
private:
21+
T *ptr;
22+
};
23+
24+
class NonTemplateArray {
25+
public:
26+
NonTemplateArray() : ptr(nullptr) {}
27+
28+
~NonTemplateArray() {
29+
reset();
30+
}
31+
32+
void init(unsigned size) {
33+
ptr = new int[size]; // GOOD
34+
}
35+
36+
void reset() {
37+
int *tmp = ptr;
38+
ptr = nullptr;
39+
delete[] tmp;
40+
}
41+
42+
private:
43+
int *ptr;
44+
};
45+
46+
class OverwrittenAlias {
47+
public:
48+
OverwrittenAlias() : ptr(nullptr) {}
49+
50+
~OverwrittenAlias() {
51+
reset();
52+
}
53+
54+
void init(unsigned size) {
55+
ptr = new int[size]; // $ Alert
56+
}
57+
58+
void reset() {
59+
int *tmp = ptr;
60+
tmp = new int[1];
61+
delete[] tmp;
62+
}
63+
64+
private:
65+
int *ptr;
66+
};
67+
68+
void testArrays() {
69+
TemplateArray<int> templateArray;
70+
templateArray.init(10);
71+
72+
NonTemplateArray nonTemplateArray;
73+
nonTemplateArray.init(10);
74+
75+
OverwrittenAlias overwrittenAlias;
76+
overwrittenAlias.init(10);
77+
}

0 commit comments

Comments
 (0)