Merge pull request #523 from jbj/placement-new-never-freed

C++: Detect non-allocating placement new in cpp/memory-never-freed

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

@@ -78,8 +78,8 @@ predicate isStdLibAllocationExpr(Expr e)
  */
 predicate isAllocationExpr(Expr e) {
   allocationCall(e)
-  or e instanceof NewExpr
-  or e instanceof NewArrayExpr
+  or
+  e = any(NewOrNewArrayExpr new | not exists(new.getPlacementPointer()))
 }
 
 /**

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

@@ -664,6 +664,16 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
    * For `new int[5]` the result is `int[5]`.
    */
   abstract Type getAllocatedType();
+
+  /**
+   * Gets the pointer `p` if this expression is of the form `new(p) T...`.
+   * Invocations of this form are non-allocating `new` expressions that may
+   * call the constructor of `T` but will not allocate memory.
+   */
+  Expr getPlacementPointer() {
+    isStandardPlacementNewAllocator(this.getAllocator()) and
+    result = this.getAllocatorCall().getArgument(1)
+  }
 }
 
 /**
@@ -961,3 +971,9 @@ private predicate convparents(Expr child, int idx, Element parent) {
     child = astChild.getFullyConverted()
   )
 }
+
+private predicate isStandardPlacementNewAllocator(Function operatorNew) {
+  operatorNew.getName().matches("operator new%") and
+  operatorNew.getNumberOfParameters() = 2 and
+  operatorNew.getParameter(1).getType() instanceof VoidPointerType
+}

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

@@ -109,3 +109,31 @@ void TestFailedInit(int n) {
   new(1.0f) FailedInitOveraligned();
   new(1.0f) FailedInitOveraligned[10];
 }
+
+// --- non-allocating placement new ---
+
+namespace std {
+  typedef unsigned long size_t;
+  struct nothrow_t {};
+  extern const nothrow_t nothrow;
+}
+
+void* operator new  (std::size_t size, void* ptr) noexcept;
+void* operator new[](std::size_t size, void* ptr) noexcept;
+void* operator new(std::size_t size, const std::nothrow_t&) noexcept;
+void* operator new[](std::size_t size, const std::nothrow_t&) noexcept;
+
+int overloadedNew() {
+  char buf[sizeof(int)];
+
+  new(&buf[0]) int(5);
+  int five = *(int*)buf;
+
+  new(buf) int[1];
+  *(int*)buf = 4;
+
+  new(std::nothrow) int(3); // memory leak
+  new(std::nothrow) int[2]; // memory leak
+
+  return five;
+}

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

@@ -8,6 +8,8 @@ newExprs
 | allocators.cpp:55:3:55:25 | new | Overaligned | operator new(size_t, align_val_t, float) -> void * | 256 | 128 | aligned |
 | allocators.cpp:107:3:107:18 | new | FailedInit | FailedInit::operator new(size_t) -> void * | 1 | 1 |  |
 | allocators.cpp:109:3:109:35 | new | FailedInitOveraligned | FailedInitOveraligned::operator new(size_t, align_val_t, float) -> void * | 128 | 128 | aligned |
+| allocators.cpp:129:3:129:21 | new | int | operator new(size_t, void *) -> void * | 4 | 4 |  |
+| allocators.cpp:135:3:135:26 | new | int | operator new(size_t, const nothrow_t &) -> void * | 4 | 4 |  |
 newArrayExprs
 | allocators.cpp:68:3:68:12 | new[] | int | operator new[](unsigned long) -> void * | 4 | 4 |  |
 | allocators.cpp:69:3:69:18 | new[] | int | operator new[](size_t, float) -> void * | 4 | 4 |  |
@@ -16,6 +18,8 @@ newArrayExprs
 | allocators.cpp:72:3:72:16 | new[] | String | operator new[](unsigned long) -> void * | 8 | 8 |  |
 | allocators.cpp:108:3:108:19 | new[] | FailedInit | FailedInit::operator new[](size_t) -> void * | 1 | 1 |  |
 | allocators.cpp:110:3:110:37 | new[] | FailedInitOveraligned | FailedInitOveraligned::operator new[](size_t, align_val_t, float) -> void * | 128 | 128 | aligned |
+| allocators.cpp:132:3:132:17 | new[] | int | operator new[](size_t, void *) -> void * | 4 | 4 |  |
+| allocators.cpp:136:3:136:26 | new[] | int | operator new[](size_t, const nothrow_t &) -> void * | 4 | 4 |  |
 newExprDeallocators
 | allocators.cpp:52:3:52:14 | new | String | operator delete(void *, unsigned long) -> void | 8 | 8 | sized  |
 | allocators.cpp:53:3:53:27 | new | String | operator delete(void *, float) -> void | 8 | 8 |   |

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

@@ -0,0 +1,2 @@
+| allocators.cpp:129:3:129:21 | new | allocators.cpp:129:7:129:13 | & ... |
+| allocators.cpp:132:3:132:17 | new[] | allocators.cpp:132:7:132:9 | buf |

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

@@ -0,0 +1,4 @@
+import cpp
+
+from NewOrNewArrayExpr new
+select new, new.getPlacementPointer() as placement

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

@@ -8,3 +8,5 @@
 | test.cpp:42:18:42:23 | call to malloc | This memory is never freed |
 | test.cpp:73:18:73:23 | call to malloc | This memory is never freed |
 | test.cpp:89:18:89:23 | call to malloc | This memory is never freed |
+| test.cpp:156:3:156:26 | new | This memory is never freed |
+| test.cpp:157:3:157:26 | new[] | This memory is never freed |

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

@@ -130,3 +130,29 @@ int main()
 
 	return 0;
 }
+
+// --- placement new ---
+
+namespace std {
+  typedef unsigned long size_t;
+  struct nothrow_t {};
+  extern const nothrow_t nothrow;
+}
+
+void* operator new  (std::size_t size, void* ptr) noexcept;
+void* operator new[](std::size_t size, void* ptr) noexcept;
+void* operator new(std::size_t size, const std::nothrow_t&) noexcept;
+void* operator new[](std::size_t size, const std::nothrow_t&) noexcept;
+
+int overloadedNew() {
+  char buf[sizeof(int)];
+
+  new(&buf[0]) int(5); // GOOD
+  int five = *(int*)buf;
+
+  new(buf) int[1]; // GOOD
+  *(int*)buf = 4;
+
+  new(std::nothrow) int(3); // BAD
+  new(std::nothrow) int[2]; // BAD
+}