Skip to content

Commit 75873bb

Browse files
committed
C++: Detect non-allocating placement new
This adds a `NewOrNewArrayExpr.getPlacementPointer` predicate and uses it in `Alloc.qll` to detect when a `new`-expression is not an allocation. User-defined replacements for `operator new` may not be allocations either, but the code continues to assume that they are. It's possible that we want to change this assumption in the future or leave it up to individual queries to decide on which side to err. It's hard to statically tell whether `operator new` has been overloaded in a particular file because it can be overloaded by a definition that is not in scope but is only linked together with that file.
1 parent a17deba commit 75873bb

File tree

8 files changed

+58
-6
lines changed

8 files changed

+58
-6
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: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +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:150:3:150:21 | new | This memory is never freed |
12-
| test.cpp:153:3:153:17 | new[] | This memory is never freed |
1311
| test.cpp:156:3:156:26 | new | This memory is never freed |
1412
| test.cpp:157:3:157:26 | new[] | This memory is never freed |

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,10 @@ void* operator new[](std::size_t size, const std::nothrow_t&) noexcept;
147147
int overloadedNew() {
148148
char buf[sizeof(int)];
149149

150-
new(&buf[0]) int(5); // GOOD [FALSE POSITIVE]
150+
new(&buf[0]) int(5); // GOOD
151151
int five = *(int*)buf;
152152

153-
new(buf) int[1]; // GOOD [FALSE POSITIVE]
153+
new(buf) int[1]; // GOOD
154154
*(int*)buf = 4;
155155

156156
new(std::nothrow) int(3); // BAD

0 commit comments

Comments
 (0)