CPP: Use newer dataflow for the fix.

geoffw0 · geoffw0 · commit faeb326bf839 · 2019-03-20T15:47:48.000Z
diff --git a/cpp/ql/src/Critical/NewDelete.qll b/cpp/ql/src/Critical/NewDelete.qll
@@ -3,6 +3,7 @@
  */
 import cpp
 import semmle.code.cpp.controlflow.SSA
+import semmle.code.cpp.dataflow.DataFlow
 
 /**
  * Holds if `alloc` is a use of `malloc` or `new`.  `kind` is
@@ -46,10 +47,9 @@ predicate allocExprOrIndirect(Expr alloc, string kind) {
     alloc.(FunctionCall).getTarget() = rtn.getEnclosingFunction() and
     (
       allocExprOrIndirect(rtn.getExpr(), kind) or
-      exists(SsaDefinition def, LocalScopeVariable v |
-        // alloc via SSA
-        allocExprOrIndirect(def.getAnUltimateDefiningValue(v), kind) and
-        rtn.getExpr() = def.getAUse(v)
+      exists(Expr e |
+        allocExprOrIndirect(e, kind) and
+        DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(rtn.getExpr()))
       )
     )
   )
diff --git a/cpp/ql/test/query-tests/Critical/NewFree/NewFreeMismatch.expected b/cpp/ql/test/query-tests/Critical/NewFree/NewFreeMismatch.expected
@@ -6,6 +6,7 @@
 | test.cpp:91:3:91:11 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:88:28:88:36 | call to my_malloc | malloc |
 | test.cpp:99:3:99:11 | call to my_delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:96:28:96:33 | call to malloc | malloc |
 | test.cpp:138:2:138:9 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:135:12:135:22 | call to my_malloc_2 | malloc |
+| test.cpp:155:2:155:9 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:152:12:152:22 | call to my_malloc_3 | malloc |
 | test.cpp:232:2:232:9 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:226:7:226:12 | call to malloc | malloc |
 | test.cpp:233:2:233:12 | delete[] | There is a malloc/delete mismatch between this delete[] and the corresponding $@. | test.cpp:226:7:226:12 | call to malloc | malloc |
 | test.cpp:235:2:235:5 | call to free | There is a new/free mismatch between this free and the corresponding $@. | test.cpp:227:7:227:13 | new | new |
diff --git a/cpp/ql/test/query-tests/Critical/NewFree/test.cpp b/cpp/ql/test/query-tests/Critical/NewFree/test.cpp
@@ -152,7 +152,7 @@ void test3()
 	void *b = my_malloc_3(10);
 	
 	free(a); // GOOD
-	delete b; // BAD: malloc -> delete [NOT DETECTED]
+	delete b; // BAD: malloc -> delete
 }
 
 void test4(bool do_array_delete)

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`	`*/`
`4`	`4`	`import cpp`
`5`	`5`	`import semmle.code.cpp.controlflow.SSA`
	`6`	`+import semmle.code.cpp.dataflow.DataFlow`
`6`	`7`
`7`	`8`	`/**`
`8`	`9`	* Holds if `alloc` is a use of `malloc` or `new`. `kind` is
`@@ -46,10 +47,9 @@ predicate allocExprOrIndirect(Expr alloc, string kind) {`
`46`	`47`	`alloc.(FunctionCall).getTarget() = rtn.getEnclosingFunction() and`
`47`	`48`	`(`
`48`	`49`	`allocExprOrIndirect(rtn.getExpr(), kind) or`
`49`		`- exists(SsaDefinition def, LocalScopeVariable v \|`
`50`		`- // alloc via SSA`
`51`		`- allocExprOrIndirect(def.getAnUltimateDefiningValue(v), kind) and`
`52`		`- rtn.getExpr() = def.getAUse(v)`
	`50`	`+ exists(Expr e \|`
	`51`	`+ allocExprOrIndirect(e, kind) and`
	`52`	`+ DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(rtn.getExpr()))`
`53`	`53`	`)`
`54`	`54`	`)`
`55`	`55`	`)`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ void test3()`
`152`	`152`	`void *b = my_malloc_3(10);`
`153`	`153`
`154`	`154`	`free(a); // GOOD`
`155`		`- delete b; // BAD: malloc -> delete [NOT DETECTED]`
	`155`	`+ delete b; // BAD: malloc -> delete`
`156`	`156`	`}`
`157`	`157`
`158`	`158`	`void test4(bool do_array_delete)`