Merge pull request #790 from rdmarsh2/rdmarsh/cpp/futile-params

Approved by semmledocs-ac

cpp/config/suites/c/correctness

@@ -7,7 +7,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Conversion/NonzeroValueCastToPointer.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
-  # Consistent Use 
+  # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCallOnResult.ql: /Correctness/Consistent Use
@@ -15,6 +15,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/AssignWhereCompareMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/CompareWhereAssignMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Likely Typos/FutileParams.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ShortCircuitBitMask.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/MissingEnumCaseInSwitch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Arithmetic/FloatComparison.ql: /Correctness/Common Errors

cpp/config/suites/cpp/correctness

@@ -8,7 +8,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
-  # Consistent Use 
+  # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCallOnResult.ql: /Correctness/Consistent Use
@@ -16,6 +16,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/AssignWhereCompareMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/CompareWhereAssignMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Likely Typos/FutileParams.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ShortCircuitBitMask.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/MissingEnumCaseInSwitch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Arithmetic/FloatComparison.ql: /Correctness/Common Errors

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

@@ -0,0 +1,11 @@
+void no_argument();
+
+void one_argument(int x);
+
+void calls() {
+	no_argument(1) // BAD: `no_argument` will accept and ignore the argument
+	
+	one_argument(1); // GOOD: `one_argument` will accept and use the argument
+	
+	no_argument(); // GOOD: `no_argument` has not been passed an argument
+}

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

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<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>In C, a function declared with an empty parameter list `()` 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 `(void)` as the parameter list in any forward declarations.
+In C++, either style of declaration 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>
+
+</recommendation>
+<example><sample src="FutileParams.c" />
+
+</example>
+
+<references>
+<li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/DCL20-C.+Explicitly+specify+void+when+a+function+accepts+no+arguments"> DCL20-C. Explicitly specify void when a function accepts no arguments </a></li>
+</references>
+</qhelp>

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

@@ -0,0 +1,22 @@
+/**
+ * @name Non-empty call to function declared without parameters
+ * @description A call to a function declared without parameters has arguments, which may indicate
+ *              that the code does not follow the author's intent.
+ * @kind problem
+ * @problem.severity warning
+ * @precision very-high
+ * @id cpp/futile-params
+ * @tags correctness
+ *       maintainability
+ */
+
+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()

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

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

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

@@ -0,0 +1 @@
+Likely Bugs/Likely Typos/FutileParams.ql

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

@@ -0,0 +1,29 @@
+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
+  
+  not_yet_declared1(1); // BAD
+  not_yet_declared2(1); // GOOD
+
+  declared_empty_defined_with(); // BAD [NOT DETECTED]
+  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]
+}
+
+void not_yet_declared1();
+void not_yet_declared2(int);
+void declared_empty_defined_with(int x) {
+  // do nothing
+}

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

@@ -0,0 +1,8 @@
+void cpp_varargs(...);
+void bar();
+
+void test() {
+  cpp_varargs(); // GOOD
+  cpp_varargs(1); // GOOD
+  __builtin_constant_p("something"); // GOOD: builtin
+}