Refine query to properly handle cases in test.c

cpp/ql/src/Likely Bugs/Likely Typos/FutileParams.qhelp

@@ -5,18 +5,25 @@
 
 
 <overview>
-<p>A function is called with arguments despite having an empty parameter list. This may indicate
-that the incorrect function is being called, or that the author misunderstood the function.</p>
+<p>A function is called with arguments despite having been <i>declared</i> with an empty parameter list.
+In addition, we were either unable to find a <i>definition</i> of the function, or found one with
+an incompatible number of parameters.</p>
+
+<p>This may indicate that an incorrect function is being called, or that the signature (parameter list)
+of the called function is not known to the author.</p>
 
 <p>In C, a function declared with an empty parameter list <code>()</code> is considered to have an unknown
 parameter list, and therefore can be called with any set of arguments. To declare a function
 which takes no arguments, you must use <code>(void)</code> as the parameter list in any forward declarations.
-In C++, either style of declaration indicates that the function accepts no arguments.</p>
+In C++, <code>()</code> means the same as <code>(void)</code>, i.e., indicates that the function accepts no
+arguments.</p>
 
 </overview>
 <recommendation>
 <p>Call the function without arguments, or call a different function that expects the arguments
-being passed.</p>
+being passed.  If possible, change the forward declaration of the <code>()</code>-function to reflect the
+arguments being passed, or change the <code>()</code> to <code>(void)</code> to indicate that no arguments
+shall be passed.</p>
 
 </recommendation>
 <example><sample src="FutileParams.c" />

cpp/ql/src/Likely Bugs/Likely Typos/FutileParams.ql

@@ -12,11 +12,16 @@
 
 import cpp
 
-from Function f, FunctionCall fc
-where fc.getTarget() = f
-  and f.getNumberOfParameters() = 0
-  and not f.isVarargs()
-  and fc.getNumberOfArguments() != 0
-  and not f instanceof BuiltInFunction
-  and exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() | not fde.isImplicit())
-select fc, "This call has arguments, but $@ is not declared with any parameters.", f, f.toString()
+from FunctionCall fc, Function f
+  where f = fc.getTarget() and not f.isVarargs()
+  /* There must be a zero-parameter declaration */
+  and exists ( FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() | fde.getNumberOfParameters() = 0)
+  /* There must be a mismatch between number of call arguments and number of parameters in some
+   * declaration of Function f
+   */
+  and exists ( FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() | fde.getNumberOfParameters() != fc.getNumberOfArguments())
+  /* There must be no actual declaration of Function f whose number of parameters matches number of call arguments */
+  and not exists ( FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() | not fde.isImplicit() and fde.getNumberOfParameters() = fc.getNumberOfArguments())
+select fc, "This call has arguments, but $@ is not declared with any parameters.", f, f.toString()  
+
+

cpp/ql/src/Likely Bugs/Memory Management/AllocaInLoop.ql

@@ -8,6 +8,7 @@
  *       correctness
  *       external/cwe/cwe-770
  */
+
 import cpp
 
 Loop getAnEnclosingLoopOfExpr(Expr e) {
@@ -21,7 +22,15 @@ Loop getAnEnclosingLoopOfStmt(Stmt s) {
 }
 
 from Loop l, FunctionCall fc
-where getAnEnclosingLoopOfExpr(fc) = l
-  and fc.getTarget().getName() = "__builtin_alloca"
-  and not l.(DoStmt).getCondition().getValue() = "0"
-select fc, "Stack allocation is inside a $@ and could lead to overflow.", l, l.toString()
+where
+  getAnEnclosingLoopOfExpr(fc) = l and
+  (
+    fc.getTarget().getName() = "__builtin_alloca"
+    or
+    (
+      (fc.getTarget().getName() = "_alloca" or fc.getTarget().getName() = "_malloca") and
+      fc.getTarget().getADeclarationEntry().getFile().getBaseName() = "malloc.h"
+    )
+  ) and
+  not l.(DoStmt).getCondition().getValue() = "0"
+select fc, "Stack allocation is inside a $@ and could lead to stack overflow.", l, l.toString()

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/FutileParams/FutileParams.expected

@@ -1,3 +1,6 @@
-| test.c:8:3:8:16 | call to declared_empty | This call has arguments, but $@ is not declared with any parameters. | test.c:1:6:1:19 | declared_empty | declared_empty |
+| test.c:7:3:7:16 | call to declared_empty | This call has arguments, but $@ is not declared with any parameters. | test.c:1:6:1:19 | declared_empty | declared_empty |
+| test.c:12:3:12:12 | call to undeclared | This call has arguments, but $@ is not declared with any parameters. | test.c:11:3:11:3 | undeclared | undeclared |
 | test.c:14:3:14:19 | call to not_yet_declared1 | This call has arguments, but $@ is not declared with any parameters. | test.c:14:3:14:3 | not_yet_declared1 | not_yet_declared1 |
 | test.c:14:3:14:19 | call to not_yet_declared1 | This call has arguments, but $@ is not declared with any parameters. | test.c:25:6:25:22 | not_yet_declared1 | not_yet_declared1 |
+| test.c:17:3:17:29 | call to declared_empty_defined_with | This call has arguments, but $@ is not declared with any parameters. | test.c:27:6:27:32 | declared_empty_defined_with | declared_empty_defined_with |
+| test.c:22:3:22:29 | call to declared_empty_defined_with | This call has arguments, but $@ is not declared with any parameters. | test.c:27:6:27:32 | declared_empty_defined_with | declared_empty_defined_with |

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/FutileParams/test.c

@@ -1,25 +1,25 @@
 void declared_empty();
 void declared_void(void);
 void declared_with(int);
-void declared_empty_defined_with();
 
 void test() {
   declared_empty(); // GOOD
   declared_empty(1); // BAD
   declared_void(); // GOOD
   declared_with(1); // GOOD
 
-  undeclared(1); // GOOD
+  undeclared();  // GOOD
+  undeclared(1); // BAD
 
   not_yet_declared1(1); // BAD
   not_yet_declared2(1); // GOOD
 
-  declared_empty_defined_with(); // BAD [NOT DETECTED]
+  declared_empty_defined_with(); // BAD
   declared_empty_defined_with(1); // GOOD
 
   int x;
-  declared_empty_defined_with(&x); // BAD [NOT DETECTED]
-  declared_empty_defined_with(x, x); // BAD [NOT DETECTED]
+  declared_empty_defined_with(&x); // GOOD
+  declared_empty_defined_with(x, x); // BAD
 }
 
 void not_yet_declared1();

cpp/ql/test/query-tests/Likely Bugs/Memory Management/AllocaInLoop/AllocaInLoop.expected

@@ -0,0 +1,8 @@
+| AllocaInLoop1.cpp:31:18:31:23 | call to __builtin_alloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop1.cpp:22:2:39:2 | for(...;...;...) ... | for(...;...;...) ... |
+| AllocaInLoop1.cpp:55:19:55:24 | call to __builtin_alloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop1.cpp:45:2:64:2 | for(...;...;...) ... | for(...;...;...) ... |
+| AllocaInLoop1.cpp:80:19:80:24 | call to __builtin_alloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop1.cpp:71:3:88:3 | for(...;...;...) ... | for(...;...;...) ... |
+| AllocaInLoop1ms.cpp:28:18:28:24 | call to _alloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop1ms.cpp:19:2:36:2 | for(...;...;...) ... | for(...;...;...) ... |
+| AllocaInLoop1ms.cpp:52:19:52:26 | call to _malloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop1ms.cpp:42:2:63:2 | for(...;...;...) ... | for(...;...;...) ... |
+| AllocaInLoop1ms.cpp:79:19:79:25 | call to _alloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop1ms.cpp:70:3:87:3 | for(...;...;...) ... | for(...;...;...) ... |
+| AllocaInLoop2.c:39:30:39:35 | call to __builtin_alloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop2.c:29:5:48:19 | do (...) ... | do (...) ... |
+| AllocaInLoop3.cpp:45:23:45:28 | call to __builtin_alloca | Stack allocation is inside a $@ and could lead to stack overflow. | AllocaInLoop3.cpp:43:2:49:19 | do (...) ... | do (...) ... |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/AllocaInLoop/AllocaInLoop.qlref

@@ -0,0 +1 @@
+Likely Bugs/Memory Management/AllocaInLoop.ql

cpp/ql/test/query-tests/Likely Bugs/Memory Management/AllocaInLoop/AllocaInLoop1.cpp

@@ -0,0 +1,90 @@
+// semmle-extractor-options: --clang
+struct vtype {
+	int i1, i2;
+};
+extern int w1, w2;
+
+#ifdef _MSC_VER
+#define restrict __restrict
+#else
+#define restrict __restrict__
+#endif
+
+void *__builtin_alloca(int sz);
+#define alloca __builtin_alloca
+typedef unsigned long size_t;
+
+int printf(const char *restrict format, ...);
+void *memcpy(void *restrict s1, const void *restrict s2, size_t n);
+
+// case 1: alloca directly contained in an unbounded loop
+void foo(const struct vtype* vec, int count) {
+	for (int i = 0; i < count; i++) {
+		const vtype* v = vec + i;
+		char *b1 = 0;
+		if (b1 == nullptr) {
+			if (w1 > w2) {
+				// Allocate the buffer on heap
+				b1 = new char[w1];
+			} else {
+				// Allocate the buffer on stack
+				b1 = (char*) alloca(w1);  // BAD
+			}
+		}
+		memcpy(b1, v, w1);
+		printf("%s\n", b1);
+		if (w1 > w2) {
+			delete b1;
+		}
+	}
+}
+
+// case 2: alloca  contained in a do-while(0) that is in turn contained
+//         in an unbounded loop
+void bar(const struct vtype* vec, int count) {
+	for (int i = 0; i < count; i++) {
+		const vtype* v = vec + i;
+		char *b1 = 0;
+		do {
+			if (b1 == nullptr) {
+				if (w1 > w2) {
+					// Allocate the buffer on heap
+					b1 = new char[w1];
+				} else {
+					// Allocate the buffer on stack
+					b1 = (char*) alloca(w1);  // BAD
+				}
+			}
+		} while (0);
+		memcpy(b1, v, w1);
+		printf("%s\n", b1);
+		if (w1 > w2) {
+			delete b1;
+		}
+	}
+}
+
+// case 3: alloca  contained in an unbounded loop that is in turn contained
+//         in a do-while(0)
+void baz(const struct vtype* vec, int count) {
+	do {
+		for (int i = 0; i < count; i++) {
+			const vtype* v = vec + i;
+			char *b1 = 0;
+			if (b1 == nullptr) {
+				if (w1 > w2) {
+					// Allocate the buffer on heap
+					b1 = new char[w1];
+				} else {
+					// Allocate the buffer on stack
+					b1 = (char*) alloca(w1);  // BAD
+				}
+			}
+			memcpy(b1, v, w1);
+			printf("%s\n", b1);
+			if (w1 > w2) {
+				delete b1;
+			}
+		}
+	} while (0);
+}

cpp/ql/test/query-tests/Likely Bugs/Memory Management/AllocaInLoop/AllocaInLoop1ms.cpp

@@ -0,0 +1,89 @@
+// semmle-extractor-options: --clang
+#include "malloc.h"
+struct vtype {
+	int i1, i2;
+};
+extern int w1, w2;
+
+#ifdef _MSC_VER
+#define restrict __restrict
+#else
+#define restrict __restrict__
+#endif
+
+int printf(const char *restrict format, ...);
+void *memcpy(void *restrict s1, const void *restrict s2, size_t n);
+
+// case 1: _alloca directly contained in an unbounded loop
+void foo(const struct vtype* vec, int count) {
+	for (int i = 0; i < count; i++) {
+		const vtype* v = vec + i;
+		char *b1 = 0;
+		if (b1 == nullptr) {
+			if (w1 > w2) {
+				// Allocate the buffer on heap
+				b1 = new char[w1];
+			} else {
+				// Allocate the buffer on stack
+				b1 = (char*) _alloca(w1);  // BAD
+			}
+		}
+		memcpy(b1, v, w1);
+		printf("%s\n", b1);
+		if (w1 > w2) {
+			delete b1;
+		}
+	}
+}
+
+// case 2: _malloca  contained in a do-while(0) that is in turn contained
+//         in an unbounded loop
+void bar(const struct vtype* vec, int count) {
+	for (int i = 0; i < count; i++) {
+		const vtype* v = vec + i;
+		char *b1 = 0;
+		do {
+			if (b1 == nullptr) {
+				if (w1 > w2) {
+					// Allocate the buffer on heap
+					b1 = new char[w1];
+				} else {
+					// Allocate the buffer on stack
+					b1 = (char*) _malloca(w1);  // BAD
+				}
+			}
+		} while (0);
+		memcpy(b1, v, w1);
+		printf("%s\n", b1);
+		if (w1 > w2) {
+			delete b1;
+		} else {
+			_freea(b1);
+		}
+	}
+}
+
+// case 3: _alloca  contained in an unbounded loop that is in turn contained
+//         in a do-while(0)
+void baz(const struct vtype* vec, int count) {
+	do {
+		for (int i = 0; i < count; i++) {
+			const vtype* v = vec + i;
+			char *b1 = 0;
+			if (b1 == nullptr) {
+				if (w1 > w2) {
+					// Allocate the buffer on heap
+					b1 = new char[w1];
+				} else {
+					// Allocate the buffer on stack
+					b1 = (char*) _alloca(w1);  // BAD
+				}
+			}
+			memcpy(b1, v, w1);
+			printf("%s\n", b1);
+			if (w1 > w2) {
+				delete b1;
+			}
+		}
+	} while (0);
+}