Merge pull request #1319 from geoffw0/av114

CPP: Improve locations for AV Rule 114.ql.

Author: Robert Marsh

change-notes/1.24/analysis-cpp.md

@@ -20,6 +20,7 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
 | Memory may not be freed (`cpp/memory-may-not-be-freed`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
 | Mismatching new/free or malloc/delete (`cpp/new-free-mismatch`) | Fewer false positive results | Fixed false positive results in template code. |
 | Missing return statement (`cpp/missing-return`) | Fewer false positive results | Functions containing `asm` statements are no longer highlighted by this query. |
+| Missing return statement (`cpp/missing-return`) | More accurate locations | Locations reported by this query are now more accurate in some cases. |
 | No space for zero terminator (`cpp/no-space-for-terminator`) | More correct results | String arguments to formatting functions are now (usually) expected to be null terminated strings. |
 | Hard-coded Japanese era start date (`cpp/japanese-era/exact-era-date`) |  | This query is no longer run on LGTM. |
 | No space for zero terminator (`cpp/no-space-for-terminator`) | Fewer false positive results | This query has been modified to be more conservative when identifying which pointers point to null-terminated strings.  This approach produces fewer, more accurate results. |

cpp/ql/src/jsf/4.13 Functions/AV Rule 114.ql

@@ -30,7 +30,13 @@ predicate functionsMissingReturnStmt(Function f, ControlFlowNode blame) {
   ) and
   exists(ReturnStmt s |
     f.getAPredecessor() = s and
-    blame = s.getAPredecessor()
+    (
+      blame = s.getAPredecessor() and
+      count(blame.getASuccessor()) = 1
+      or
+      blame = s and
+      exists(ControlFlowNode pred | pred = s.getAPredecessor() | count(pred.getASuccessor()) != 1)
+    )
   )
 }
 

cpp/ql/test/query-tests/jsf/4.13 Functions/AV Rule 114/AV Rule 114.expected

@@ -4,9 +4,9 @@
 | test.c:25:9:25:14 | ExprStmt | Function f4 should return a value of type int but does not return a value here |
 | test.c:39:9:39:14 | ExprStmt | Function f6 should return a value of type int but does not return a value here |
 | test.cpp:16:1:18:1 | { ... } | Function g2 should return a value of type MyValue but does not return a value here |
-| test.cpp:48:2:48:26 | if (...) ...  | Function g7 should return a value of type MyValue but does not return a value here |
+| test.cpp:52:1:52:1 | return ... | Function g7 should return a value of type MyValue but does not return a value here |
 | test.cpp:74:1:76:1 | { ... } | Function g10 should return a value of type second but does not return a value here |
 | test.cpp:86:1:88:1 | { ... } | Function g12 should return a value of type second but does not return a value here |
-| test.cpp:108:2:111:2 | if (...) ...  | Function g14 should return a value of type int but does not return a value here |
+| test.cpp:112:1:112:1 | return ... | Function g14 should return a value of type int but does not return a value here |
 | test.cpp:134:2:134:36 | ExprStmt | Function g16 should return a value of type int but does not return a value here |
 | test.cpp:141:3:141:37 | ExprStmt | Function g17 should return a value of type int but does not return a value here |

cpp/ql/test/query-tests/jsf/4.13 Functions/AV Rule 114/test.cpp

@@ -48,7 +48,7 @@ MyValue g7(bool c)
 	if (c) return MyValue(7);
 	DONOTHING
 	DONOTHING
-	// BAD [the alert here is unfortunately placed]
+	// BAD
 }
 
 typedef void MYVOID;