Fix false positives of M0-1-10 (#711)

change_notes/2024-10-02-fix-fp-711-M0-1-10.md

@@ -0,0 +1,2 @@
+- `M0-1-10` - `UnusedFunction.ql`:
+  - Fixes #711. Excludes constexpr functions, considers functions from GoogleTest as an EntryPoint and does not consider special member functions. Another query called UnusedSplMemberFunction.ql is created that reports unused special member functions. This is done so as to enable deviations to be applied to this case.

cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql

@@ -26,5 +26,6 @@ where
     then name = unusedFunction.getQualifiedName()
     else name = unusedFunction.getName()
   ) and
-  not unusedFunction.isDeleted()
+  not unusedFunction.isDeleted() and
+  not UnusedFunctions::isASpecialMemberFunction(unusedFunction)
 select unusedFunction, "Function " + name + " is " + unusedFunction.getDeadCodeType()

cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql

@@ -0,0 +1,32 @@
+/**
+ * @id cpp/autosar/unused-spl-member-function
+ * @name M0-1-10: Every defined function should be called at least once
+ * @description Uncalled functions complicate the program and can indicate a possible mistake on the
+ *              part of the programmer. This query specifically looks for unused Special Member
+ *              Functions.
+ * @kind problem
+ * @precision medium
+ * @problem.severity warning
+ * @tags external/autosar/id/m0-1-10
+ *       readability
+ *       maintainability
+ *       external/autosar/allocated-target/implementation
+ *       external/autosar/enforcement/automated
+ *       external/autosar/obligation/advisory
+ */
+
+import cpp
+import codingstandards.cpp.autosar
+import codingstandards.cpp.deadcode.UnusedFunctions
+
+from UnusedFunctions::UnusedSplMemberFunction unusedSplMemFunction, string name
+where
+  not isExcluded(unusedSplMemFunction, DeadCodePackage::unusedFunctionQuery()) and
+  (
+    if exists(unusedSplMemFunction.getQualifiedName())
+    then name = unusedSplMemFunction.getQualifiedName()
+    else name = unusedSplMemFunction.getName()
+  ) and
+  not unusedSplMemFunction.isDeleted()
+select unusedSplMemFunction,
+  "Special member function " + name + " is " + unusedSplMemFunction.getDeadCodeType()

cpp/autosar/test/rules/M0-1-10/UnusedFunction.expected

@@ -10,4 +10,5 @@
 | test.cpp:50:5:50:6 | i3 | Function C<T>::i3 is never called. |
 | test.cpp:51:8:51:9 | i4 | Function C<T>::i4 is never called. |
 | test.cpp:52:15:52:16 | i5 | Function C<T>::i5 is never called. |
-| test.cpp:69:17:69:18 | g4 | Function g4 is never called. |
+| test.cpp:79:6:79:21 | anUnusedFunction | Function anUnusedFunction is never called. |
+| test.cpp:113:17:113:18 | g4 | Function g4 is never called. |

cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected

@@ -0,0 +1,2 @@
+| test.cpp:71:5:71:16 | ANestedClass | Special member function ANestedClass is never called. |
+| test.cpp:82:5:82:22 | AnotherNestedClass | Special member function AnotherNestedClass is never called from a main function or entry point. |

cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref

@@ -0,0 +1 @@
+rules/M0-1-10/UnusedSplMemberFunction.ql

cpp/autosar/test/rules/M0-1-10/test.cpp

@@ -52,6 +52,50 @@ template <class T> class C {
   inline void i5() {}     // NON_COMPLIANT - never used in any instantiation
 };
 
+#include "test.hpp"
+#include <type_traits>
+
+template <typename T1, typename T2>
+constexpr bool aConstExprFunc() noexcept { // COMPLIANT
+  static_assert(std::is_trivially_copy_constructible<T1>() &&
+                    std::is_trivially_copy_constructible<T2>(),
+                "assert");
+  return true;
+}
+
+template <typename T, int val> class AClass { T anArr[val]; };
+
+void aCalledFunc1() // COMPLIANT
+{
+  struct ANestedClass {
+    ANestedClass() noexcept(false) { // COMPLIANT: False Positive!
+      static_cast<void>(0);
+    }
+  };
+  static_assert(std::is_trivially_copy_constructible<AClass<ANestedClass, 5>>(),
+                "Must be trivially copy constructible");
+}
+
+void anUnusedFunction() // NON_COMPLIANT
+{
+  struct AnotherNestedClass {
+    AnotherNestedClass() noexcept(false) { // NON_COMPLAINT
+      static_cast<void>(0);
+    }
+  };
+  AnotherNestedClass d;
+}
+
+void aCalledFunc2() // COMPLIANT
+{
+  struct YetAnotherNestedClass {
+    YetAnotherNestedClass() noexcept(false) {
+      static_cast<void>(0);
+    } // COMPLIANT
+  };
+  YetAnotherNestedClass d;
+};
+
 int main() { // COMPLIANT - this is a main like function which acts as an entry
              // point
   f3();
@@ -88,8 +132,13 @@ int main() { // COMPLIANT - this is a main like function which acts as an entry
   c1.getAT();
   S s;
   c2.i1(s);
+
+  int aVar;
+  aConstExprFunc<decltype(aCalledFuncInHeader(aVar)), int>();
+  aCalledFunc1();
+  aCalledFunc2();
 }
 class M {
 public:
   M(const M &) = delete; // COMPLIANT - ignore if deleted
-};
+};

cpp/autosar/test/rules/M0-1-10/test.hpp

@@ -0,0 +1,4 @@
+template <class T>
+constexpr T aCalledFuncInHeader(T value) noexcept { // COMPLIANT
+  return static_cast<T>(value);
+}

cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll

@@ -18,6 +18,39 @@ class MainFunction extends MainLikeFunction {
   }
 }
 
+/**
+ * A test function from the GoogleTest infrastructure.
+ *
+ * Such functions can be treated as valid EntryPoint functions during analysis
+ * of "called" or "unused" functions. It is not straightforward to identify
+ * such functions, however, they have certain features that can be used for
+ * identification. This can be refined based on experiments/real-world use.
+ */
+class GTestFunction extends MainLikeFunction {
+  GTestFunction() {
+    // A GoogleTest function is named "TestBody" and
+    this.hasName("TestBody") and
+    // is enclosed by a class that inherits from a base class
+    this.getEnclosingAccessHolder() instanceof Class and
+    exists(Class base |
+      base = this.getEnclosingAccessHolder().(Class).getABaseClass() and
+      (
+        // called "Test" or
+        exists(Class c | base.getABaseClass() = c and c.hasName("Test"))
+        or
+        // defined under a namespace called "testing" or
+        exists(Namespace n | n = base.getNamespace() | n.hasName("testing"))
+        or
+        // is templatized by a parameter called "gtest_TypeParam_"
+        exists(TemplateParameter tp |
+          tp = base.getATemplateArgument() and
+          tp.hasName("gtest_TypeParam_")
+        )
+      )
+    )
+  }
+}
+
 /**
  * A "task main" function.
  */

cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll

@@ -75,7 +75,9 @@ module UnusedFunctions {
    */
 
   private class MainLikeFunctionEntryPoint extends EntryPoint, MainLikeFunction {
-    MainLikeFunctionEntryPoint() { this instanceof MainLikeFunction }
+    MainLikeFunctionEntryPoint() {
+      this instanceof MainLikeFunction or this instanceof GTestFunction
+    }
 
     override Function getAReachableFunction() { reachable*(this, result) }
   }
@@ -111,6 +113,26 @@ module UnusedFunctions {
     }
   }
 
+  /**
+   * A `MemberFunction` which is either a Default constructor, Destructor
+   * CopyConstructor, CopyAssingmentOperator, MoveConstructor or a
+   * MoveAssignmentOperator
+   */
+  predicate isASpecialMemberFunction(MemberFunction f) {
+    // Default constructor
+    f instanceof NoArgConstructor
+    or
+    f instanceof Destructor
+    or
+    f instanceof CopyConstructor
+    or
+    f instanceof CopyAssignmentOperator
+    or
+    f instanceof MoveConstructor
+    or
+    f instanceof MoveAssignmentOperator
+  }
+
   /**
    * A `Function` which is not used from an `EntryPoint`.
    *
@@ -119,7 +141,12 @@ module UnusedFunctions {
   class UnusedFunction extends UsableFunction {
     UnusedFunction() {
       // This function, or an equivalent function, is not reachable from any entry point
-      not exists(EntryPoint ep | getAnEquivalentFunction(this) = ep.getAReachableFunction())
+      not exists(EntryPoint ep | getAnEquivalentFunction(this) = ep.getAReachableFunction()) and
+      // and it is not a constexpr. Refer issue #646.
+      // The usages of constexpr is not well tracked and hence
+      // to avoid false positives, this is added. In case there is an improvement in
+      // handling constexpr in CodeQL, we can consider removing it.
+      not this.isConstexpr()
     }
 
     string getDeadCodeType() {
@@ -128,4 +155,13 @@ module UnusedFunctions {
       else result = "never called."
     }
   }
+
+  /**
+   * A Special `MemberFunction` which is an `UnusedFunction`.
+   *
+   * Refer isASpecialMemberFunction predicate.
+   */
+  class UnusedSplMemberFunction extends UnusedFunction {
+    UnusedSplMemberFunction() { isASpecialMemberFunction(this) }
+  }
 }