Skip to content

Commit 0a27022

Browse files
authored
Merge pull request #523 from jbj/placement-new-never-freed
C++: Detect non-allocating placement new in cpp/memory-never-freed
2 parents 4ad5923 + 75873bb commit 0a27022

File tree

8 files changed

+84
-2
lines changed

8 files changed

+84
-2
lines changed

cpp/ql/src/semmle/code/cpp/commons/Alloc.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ predicate isStdLibAllocationExpr(Expr e)
7878
*/
7979
predicate isAllocationExpr(Expr e) {
8080
allocationCall(e)
81-
or e instanceof NewExpr
82-
or e instanceof NewArrayExpr
81+
or
82+
e = any(NewOrNewArrayExpr new | not exists(new.getPlacementPointer()))
8383
}
8484

8585
/**

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,16 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
664664
* For `new int[5]` the result is `int[5]`.
665665
*/
666666
abstract Type getAllocatedType();
667+
668+
/**
669+
* Gets the pointer `p` if this expression is of the form `new(p) T...`.
670+
* Invocations of this form are non-allocating `new` expressions that may
671+
* call the constructor of `T` but will not allocate memory.
672+
*/
673+
Expr getPlacementPointer() {
674+
isStandardPlacementNewAllocator(this.getAllocator()) and
675+
result = this.getAllocatorCall().getArgument(1)
676+
}
667677
}
668678

669679
/**
@@ -961,3 +971,9 @@ private predicate convparents(Expr child, int idx, Element parent) {
961971
child = astChild.getFullyConverted()
962972
)
963973
}
974+
975+
private predicate isStandardPlacementNewAllocator(Function operatorNew) {
976+
operatorNew.getName().matches("operator new%") and
977+
operatorNew.getNumberOfParameters() = 2 and
978+
operatorNew.getParameter(1).getType() instanceof VoidPointerType
979+
}

cpp/ql/test/library-tests/allocators/allocators.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,31 @@ void TestFailedInit(int n) {
109109
new(1.0f) FailedInitOveraligned();
110110
new(1.0f) FailedInitOveraligned[10];
111111
}
112+
113+
// --- non-allocating placement new ---
114+
115+
namespace std {
116+
typedef unsigned long size_t;
117+
struct nothrow_t {};
118+
extern const nothrow_t nothrow;
119+
}
120+
121+
void* operator new (std::size_t size, void* ptr) noexcept;
122+
void* operator new[](std::size_t size, void* ptr) noexcept;
123+
void* operator new(std::size_t size, const std::nothrow_t&) noexcept;
124+
void* operator new[](std::size_t size, const std::nothrow_t&) noexcept;
125+
126+
int overloadedNew() {
127+
char buf[sizeof(int)];
128+
129+
new(&buf[0]) int(5);
130+
int five = *(int*)buf;
131+
132+
new(buf) int[1];
133+
*(int*)buf = 4;
134+
135+
new(std::nothrow) int(3); // memory leak
136+
new(std::nothrow) int[2]; // memory leak
137+
138+
return five;
139+
}

cpp/ql/test/library-tests/allocators/allocators.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ newExprs
88
| allocators.cpp:55:3:55:25 | new | Overaligned | operator new(size_t, align_val_t, float) -> void * | 256 | 128 | aligned |
99
| allocators.cpp:107:3:107:18 | new | FailedInit | FailedInit::operator new(size_t) -> void * | 1 | 1 | |
1010
| allocators.cpp:109:3:109:35 | new | FailedInitOveraligned | FailedInitOveraligned::operator new(size_t, align_val_t, float) -> void * | 128 | 128 | aligned |
11+
| allocators.cpp:129:3:129:21 | new | int | operator new(size_t, void *) -> void * | 4 | 4 | |
12+
| allocators.cpp:135:3:135:26 | new | int | operator new(size_t, const nothrow_t &) -> void * | 4 | 4 | |
1113
newArrayExprs
1214
| allocators.cpp:68:3:68:12 | new[] | int | operator new[](unsigned long) -> void * | 4 | 4 | |
1315
| allocators.cpp:69:3:69:18 | new[] | int | operator new[](size_t, float) -> void * | 4 | 4 | |
@@ -16,6 +18,8 @@ newArrayExprs
1618
| allocators.cpp:72:3:72:16 | new[] | String | operator new[](unsigned long) -> void * | 8 | 8 | |
1719
| allocators.cpp:108:3:108:19 | new[] | FailedInit | FailedInit::operator new[](size_t) -> void * | 1 | 1 | |
1820
| allocators.cpp:110:3:110:37 | new[] | FailedInitOveraligned | FailedInitOveraligned::operator new[](size_t, align_val_t, float) -> void * | 128 | 128 | aligned |
21+
| allocators.cpp:132:3:132:17 | new[] | int | operator new[](size_t, void *) -> void * | 4 | 4 | |
22+
| allocators.cpp:136:3:136:26 | new[] | int | operator new[](size_t, const nothrow_t &) -> void * | 4 | 4 | |
1923
newExprDeallocators
2024
| allocators.cpp:52:3:52:14 | new | String | operator delete(void *, unsigned long) -> void | 8 | 8 | sized |
2125
| allocators.cpp:53:3:53:27 | new | String | operator delete(void *, float) -> void | 8 | 8 | |
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| allocators.cpp:129:3:129:21 | new | allocators.cpp:129:7:129:13 | & ... |
2+
| allocators.cpp:132:3:132:17 | new[] | allocators.cpp:132:7:132:9 | buf |
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import cpp
2+
3+
from NewOrNewArrayExpr new
4+
select new, new.getPlacementPointer() as placement

cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryNeverFreed.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,5 @@
88
| test.cpp:42:18:42:23 | call to malloc | This memory is never freed |
99
| test.cpp:73:18:73:23 | call to malloc | This memory is never freed |
1010
| test.cpp:89:18:89:23 | call to malloc | This memory is never freed |
11+
| test.cpp:156:3:156:26 | new | This memory is never freed |
12+
| test.cpp:157:3:157:26 | new[] | This memory is never freed |

cpp/ql/test/query-tests/Critical/MemoryFreed/test.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,29 @@ int main()
130130

131131
return 0;
132132
}
133+
134+
// --- placement new ---
135+
136+
namespace std {
137+
typedef unsigned long size_t;
138+
struct nothrow_t {};
139+
extern const nothrow_t nothrow;
140+
}
141+
142+
void* operator new (std::size_t size, void* ptr) noexcept;
143+
void* operator new[](std::size_t size, void* ptr) noexcept;
144+
void* operator new(std::size_t size, const std::nothrow_t&) noexcept;
145+
void* operator new[](std::size_t size, const std::nothrow_t&) noexcept;
146+
147+
int overloadedNew() {
148+
char buf[sizeof(int)];
149+
150+
new(&buf[0]) int(5); // GOOD
151+
int five = *(int*)buf;
152+
153+
new(buf) int[1]; // GOOD
154+
*(int*)buf = 4;
155+
156+
new(std::nothrow) int(3); // BAD
157+
new(std::nothrow) int[2]; // BAD
158+
}

0 commit comments

Comments
 (0)