Merge pull request #3206 from geoffw0/newfreefix

C++: Fix `cpp/new-free-mismatch` false positives

cpp/ql/src/Critical/MemoryFreed.qll

@@ -4,7 +4,7 @@ private predicate freed(Expr e) {
   e = any(DeallocationExpr de).getFreedExpr()
   or
   exists(ExprCall c |
-    // cautiously assume that any ExprCall could be a freeCall.
+    // cautiously assume that any `ExprCall` could be a deallocation expression.
     c.getAnArgument() = e
   )
 }

cpp/ql/src/Critical/NewDelete.qll

@@ -5,17 +5,34 @@
 import cpp
 import semmle.code.cpp.controlflow.SSA
 import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.models.implementations.Allocation
+import semmle.code.cpp.models.implementations.Deallocation
 
 /**
  * Holds if `alloc` is a use of `malloc` or `new`.  `kind` is
  * a string describing the type of the allocation.
  */
 predicate allocExpr(Expr alloc, string kind) {
-  isAllocationExpr(alloc) and
-  not alloc.isFromUninstantiatedTemplate(_) and
   (
-    alloc instanceof FunctionCall and
-    kind = "malloc"
+    exists(Function target |
+      alloc.(AllocationExpr).(FunctionCall).getTarget() = target and
+      (
+        target.getName() = "operator new" and
+        kind = "new" and
+        // exclude placement new and custom overloads as they
+        // may not conform to assumptions
+        not target.getNumberOfParameters() > 1
+        or
+        target.getName() = "operator new[]" and
+        kind = "new[]" and
+        // exclude placement new and custom overloads as they
+        // may not conform to assumptions
+        not target.getNumberOfParameters() > 1
+        or
+        not target instanceof OperatorNewAllocationFunction and
+        kind = "malloc"
+      )
+    )
     or
     alloc instanceof NewExpr and
     kind = "new" and
@@ -28,7 +45,8 @@ predicate allocExpr(Expr alloc, string kind) {
     // exclude placement new and custom overloads as they
     // may not conform to assumptions
     not alloc.(NewArrayExpr).getAllocatorCall().getTarget().getNumberOfParameters() > 1
-  )
+  ) and
+  not alloc.isFromUninstantiatedTemplate(_)
 }
 
 /**
@@ -110,8 +128,20 @@ predicate allocReaches(Expr e, Expr alloc, string kind) {
  * describing the type of that free or delete.
  */
 predicate freeExpr(Expr free, Expr freed, string kind) {
-  freeCall(free, freed) and
-  kind = "free"
+  exists(Function target |
+    freed = free.(DeallocationExpr).getFreedExpr() and
+    free.(FunctionCall).getTarget() = target and
+    (
+      target.getName() = "operator delete" and
+      kind = "delete"
+      or
+      target.getName() = "operator delete[]" and
+      kind = "delete[]"
+      or
+      not target instanceof OperatorDeleteDeallocationFunction and
+      kind = "free"
+    )
+  )
   or
   free.(DeleteExpr).getExpr() = freed and
   kind = "delete"

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

@@ -23,6 +23,8 @@ predicate freeFunction(Function f, int argNum) { argNum = f.(DeallocationFunctio
 
 /**
  * A call to a library routine that frees memory.
+ *
+ * DEPRECATED: Use `DeallocationExpr` instead (this also includes `delete` expressions).
  */
 predicate freeCall(FunctionCall fc, Expr arg) { arg = fc.(DeallocationExpr).getFreedExpr() }
 

cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll

@@ -1,3 +1,9 @@
+/**
+ * Provides implementation classes modelling various methods of allocation
+ * (`malloc`, `new` etc). See `semmle.code.cpp.models.interfaces.Allocation`
+ * for usage information.
+ */
+
 import semmle.code.cpp.models.interfaces.Allocation
 
 /**

cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll

@@ -1,4 +1,10 @@
-import semmle.code.cpp.models.interfaces.Allocation
+/**
+ * Provides implementation classes  modelling various methods of deallocation
+ * (`free`, `delete` etc). See `semmle.code.cpp.models.interfaces.Deallocation`
+ * for usage information.
+ */
+
+import semmle.code.cpp.models.interfaces.Deallocation
 
 /**
  * A deallocation function such as `free`.