Merge pull request #535 from rvermeulen/rvermeulen/fix-384

Fix FP reported in 384

Author: knewbury01

change_notes/2024-02-15-fix-fp-m0-1-3.md

@@ -0,0 +1,4 @@
+- `M0-1-3` - `UnusedMemberVariable.ql`, `UnusedGlobalOrNamespaceVariable.ql`:
+  - Address FP reported in #384. Exclude variables with compile time values that may have been used as a template argument.
+  - Exclude uninstantiated template members.
+  - Reformat the alert message to adhere to the style-guide.

cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql

@@ -22,5 +22,7 @@ from PotentiallyUnusedGlobalOrNamespaceVariable v
 where
   not isExcluded(v, DeadCodePackage::unusedGlobalOrNamespaceVariableQuery()) and
   // No variable access
-  not exists(v.getAnAccess())
-select v, "Variable " + v.getQualifiedName() + " is unused."
+  not exists(v.getAnAccess()) and
+  // Exclude members whose value is compile time and is potentially used to inintialize a template
+  not maybeACompileTimeTemplateArgument(v)
+select v, "Variable '" + v.getQualifiedName() + "' is unused."

cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql

@@ -50,4 +50,4 @@ where
   // Local variable is never accessed
   not exists(v.getAnAccess()) and
   getUseCountConservatively(v) = 0
-select v, "Local variable " + v.getName() + " in " + v.getFunction().getName() + " is not used."
+select v, "Local variable '" + v.getName() + "' in '" + v.getFunction().getName() + "' is not used."

cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql

@@ -25,5 +25,7 @@ where
   // No variable access
   not exists(v.getAnAccess()) and
   // No explicit initialization in a constructor
-  not exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v)
-select v, "Member variable " + v.getName() + " is unused."
+  not exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) and
+  // Exclude members whose value is compile time and is potentially used to inintialize a template
+  not maybeACompileTimeTemplateArgument(v)
+select v, "Member variable '" + v.getName() + "' is unused."

cpp/autosar/test/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.expected

@@ -1,4 +1,4 @@
-| test_global_or_namespace.cpp:3:5:3:6 | g3 | Variable g3 is unused. |
-| test_global_or_namespace.cpp:18:4:18:4 | a | Variable a is unused. |
-| test_global_or_namespace.cpp:26:5:26:6 | x3 | Variable N1::x3 is unused. |
-| test_global_or_namespace.cpp:36:5:36:5 | a | Variable N1::a is unused. |
+| test_global_or_namespace.cpp:3:5:3:6 | g3 | Variable 'g3' is unused. |
+| test_global_or_namespace.cpp:18:4:18:4 | a | Variable 'a' is unused. |
+| test_global_or_namespace.cpp:26:5:26:6 | x3 | Variable 'N1::x3' is unused. |
+| test_global_or_namespace.cpp:36:5:36:5 | a | Variable 'N1::a' is unused. |

cpp/autosar/test/rules/M0-1-3/UnusedLocalVariable.expected

@@ -1,6 +1,6 @@
-| test.cpp:7:7:7:7 | y | Local variable y in test_simple is not used. |
-| test.cpp:14:13:14:13 | y | Local variable y in test_const is not used. |
-| test.cpp:17:7:17:7 | z | Local variable z in test_const is not used. |
-| test.cpp:23:5:23:5 | t | Local variable t in f1 is not used. |
-| test.cpp:23:5:23:5 | t | Local variable t in f1 is not used. |
-| test.cpp:44:6:44:6 | a | Local variable a in test_side_effect_init is not used. |
+| test.cpp:7:7:7:7 | y | Local variable 'y' in 'test_simple' is not used. |
+| test.cpp:14:13:14:13 | y | Local variable 'y' in 'test_const' is not used. |
+| test.cpp:17:7:17:7 | z | Local variable 'z' in 'test_const' is not used. |
+| test.cpp:23:5:23:5 | t | Local variable 't' in 'f1' is not used. |
+| test.cpp:23:5:23:5 | t | Local variable 't' in 'f1' is not used. |
+| test.cpp:44:6:44:6 | a | Local variable 'a' in 'test_side_effect_init' is not used. |

cpp/autosar/test/rules/M0-1-3/UnusedMemberVariable.expected

@@ -1,4 +1,4 @@
-| test_member.cpp:4:7:4:8 | m1 | Member variable m1 is unused. |
-| test_member.cpp:17:9:17:11 | pad | Member variable pad is unused. |
-| test_member.cpp:19:9:19:11 | sm2 | Member variable sm2 is unused. |
-| test_member.cpp:31:7:31:8 | m1 | Member variable m1 is unused. |
+| test_member.cpp:4:7:4:8 | m1 | Member variable 'm1' is unused. |
+| test_member.cpp:17:9:17:11 | pad | Member variable 'pad' is unused. |
+| test_member.cpp:19:9:19:11 | sm2 | Member variable 'sm2' is unused. |
+| test_member.cpp:31:7:31:8 | m1 | Member variable 'm1' is unused. |

cpp/autosar/test/rules/M0-1-3/test_global_or_namespace.cpp

@@ -41,4 +41,22 @@ void test_ns() { x2 = 1; }
 m1(); // ignore dead code in macros
 } // namespace N1
 
-int test_access_variable() { return N1::x5; }
+int test_access_variable() { return N1::x5; }
+
+template <int t> struct C1 {
+  int array[t]; // COMPLIANT
+};
+
+constexpr int g5 = 1; // COMPLIANT - used as template parameter
+
+namespace ns1 {
+constexpr int m1 = 1; // COMPLIANT - used a template parameter
+}
+
+void test_fp_reported_in_384() {
+  struct C1<g5> l1;
+  struct C1<ns1::m1> l2;
+
+  l1.array[0] = 1;
+  l2.array[0] = 1;
+}

cpp/autosar/test/rules/M0-1-3/test_member.cpp

@@ -47,4 +47,18 @@ void test_d() {
   d.getT();
 }
 
+template <int t> struct C1 {
+  int array[t]; // COMPLIANT
+};
+
+struct C2 {
+  static constexpr int m1 = 1; // COMPLIANT - used as template parameter
+};
+
+void test_fp_reported_in_384() {
+  struct C1<C2::m1> l1;
+
+  l1.array[0] = 1;
+}
+
 } // namespace test

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

@@ -1,5 +1,6 @@
 import cpp
 import codingstandards.cpp.FunctionEquivalence
+import codingstandards.cpp.Scope
 
 /**
  * A type that contains a template parameter type (doesn't count pointers or references).
@@ -92,7 +93,9 @@ class PotentiallyUnusedMemberVariable extends MemberVariable {
     // Must be in a fully defined class, otherwise one of the undefined functions may use the variable
     getDeclaringType() instanceof FullyDefinedClass and
     // Lambda captures are not "real" member variables - it's an implementation detail that they are represented that way
-    not this = any(LambdaCapture lc).getField()
+    not this = any(LambdaCapture lc).getField() and
+    // exclude uninstantiated template members
+    not this.isFromUninstantiatedTemplate(_)
   }
 }
 
@@ -119,3 +122,21 @@ class UserProvidedConstructorFieldInit extends ConstructorFieldInit {
     not getEnclosingFunction().isCompilerGenerated()
   }
 }
+
+/**
+ * Holds if `v` may hold a compile time value and is accessible to a template instantiation that
+ * receives a constant value as an argument equal to the value of `v`.
+ */
+predicate maybeACompileTimeTemplateArgument(Variable v) {
+  v.isConstexpr() and
+  exists(ClassTemplateInstantiation cti, TranslationUnit tu |
+    cti.getATemplateArgument().(Expr).getValue() = v.getInitializer().getExpr().getValue() and
+    (
+      cti.getFile() = tu and
+      (
+        v.getADeclarationEntry().getFile() = tu or
+        tu.getATransitivelyIncludedFile() = v.getADeclarationEntry().getFile()
+      )
+    )
+  )
+}