Merge pull request #341 from geoffw0/av_114

CPP: Improve AV Rule 114.ql's understanding of return types.

Author: jbj

change-notes/1.19/analysis-cpp.md

@@ -17,6 +17,7 @@
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. Also fixed an issue where false positives could occur if the destructor body was not in the snapshot. |
 | Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. |
+| Missing return statement | Fewer false positive results | The query is now produces correct results when a function returns a template-dependent type. |
 | Call to memory access function may overflow buffer | More correct results | Array indexing with a negative index is now detected by this query. |
 | Suspicious add with sizeof | Fewer false positive results | Arithmetic with void pointers (where allowed) is now excluded from this query. |
 | Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed.  Expected argument types are determined more accurately, especially for wide string and pointer types.  Custom (non-standard) formatting functions are also identified more accurately. |

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

@@ -19,7 +19,11 @@ import cpp
 
 predicate functionsMissingReturnStmt(Function f, ControlFlowNode blame) {
                         f.fromSource() and
-      not f.getType().getUnderlyingType().getUnspecifiedType() instanceof VoidType and
+      exists(Type returnType |
+        returnType = f.getType().getUnderlyingType().getUnspecifiedType() and
+        not returnType instanceof VoidType and
+        not returnType instanceof TemplateParameter
+      ) and
       exists(ReturnStmt s | f.getAPredecessor() = s | blame = s.getAPredecessor())}
 
 /* If a function has a value-carrying return statement, but the extractor hit a snag
@@ -32,13 +36,11 @@ predicate functionImperfectlyExtracted(Function f) {
   exists(ErrorExpr ee | ee.getEnclosingFunction() = f)
 }
 
-from Stmt stmt, string msg
+from Stmt stmt, string msg, Function f, ControlFlowNode blame
 where
-  exists(Function f, ControlFlowNode blame |
-    functionsMissingReturnStmt(f, blame) and
-    reachable(blame) and
-    not functionImperfectlyExtracted(f) and
-    (blame = stmt or blame.(Expr).getEnclosingStmt() = stmt) and
-    msg = "Function " + f.getName() + " should return a value of type " + f.getType().getName() + " but does not return a value here"
-  )
+  functionsMissingReturnStmt(f, blame) and
+  reachable(blame) and
+  not functionImperfectlyExtracted(f) and
+  (blame = stmt or blame.(Expr).getEnclosingStmt() = stmt) and
+  msg = "Function " + f.getName() + " should return a value of type " + f.getType().getName() + " but does not return a value here"
 select stmt, msg

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

@@ -3,3 +3,5 @@
 | 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: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 |

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

@@ -50,3 +50,45 @@ MyValue g7(bool c)
 	DONOTHING
 	// BAD [the alert here is unfortunately placed]
 }
+
+typedef void MYVOID;
+MYVOID g8()
+{
+	// GOOD
+}
+
+template<class T, class U>
+class TypePair
+{
+public:
+	typedef T first;
+	typedef U second;
+};
+
+TypePair<void, int>::first g9()
+{
+	// GOOD (the return type amounts to void)
+}
+
+TypePair<void, int>::second g10()
+{
+	// BAD (the return type amounts to int)
+}
+
+template<class T>
+typename TypePair<void, T>::first g11()
+{
+	// GOOD (the return type amounts to void)
+}
+
+template<class T>
+typename TypePair<void, T>::second g12()
+{
+	// BAD (the return type amounts to T / int)
+}
+
+void instantiate()
+{
+	g11<int>();
+	g12<int>();
+}