diff --git a/change_notes/2024-02-15-fix-fp-m0-1-3.md b/change_notes/2024-02-15-fix-fp-m0-1-3.md new file mode 100644 index 0000000000..e84f9fb6db --- /dev/null +++ b/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. diff --git a/cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql b/cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql index 617fbd5f8a..ba6b6df20a 100644 --- a/cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql +++ b/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." diff --git a/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql b/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql index 9ac58a6de8..f088bb1b74 100644 --- a/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql +++ b/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." diff --git a/cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql b/cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql index 5ebfac4d3f..a27f9cbcab 100644 --- a/cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql +++ b/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." diff --git a/cpp/autosar/test/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.expected b/cpp/autosar/test/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.expected index 8ee5d76bfa..97c3d17a84 100644 --- a/cpp/autosar/test/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.expected +++ b/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. | diff --git a/cpp/autosar/test/rules/M0-1-3/UnusedLocalVariable.expected b/cpp/autosar/test/rules/M0-1-3/UnusedLocalVariable.expected index 0d6f7de28b..77eb030716 100644 --- a/cpp/autosar/test/rules/M0-1-3/UnusedLocalVariable.expected +++ b/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. | diff --git a/cpp/autosar/test/rules/M0-1-3/UnusedMemberVariable.expected b/cpp/autosar/test/rules/M0-1-3/UnusedMemberVariable.expected index 14e0cb42ee..e424945d5b 100644 --- a/cpp/autosar/test/rules/M0-1-3/UnusedMemberVariable.expected +++ b/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. | diff --git a/cpp/autosar/test/rules/M0-1-3/test_global_or_namespace.cpp b/cpp/autosar/test/rules/M0-1-3/test_global_or_namespace.cpp index 92bb667c45..524830f1b4 100644 --- a/cpp/autosar/test/rules/M0-1-3/test_global_or_namespace.cpp +++ b/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; } \ No newline at end of file +int test_access_variable() { return N1::x5; } + +template 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 l1; + struct C1 l2; + + l1.array[0] = 1; + l2.array[0] = 1; +} \ No newline at end of file diff --git a/cpp/autosar/test/rules/M0-1-3/test_member.cpp b/cpp/autosar/test/rules/M0-1-3/test_member.cpp index 8c0ded8b4e..7aff9a4232 100644 --- a/cpp/autosar/test/rules/M0-1-3/test_member.cpp +++ b/cpp/autosar/test/rules/M0-1-3/test_member.cpp @@ -47,4 +47,18 @@ void test_d() { d.getT(); } +template 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 l1; + + l1.array[0] = 1; +} + } // namespace test \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll b/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll index 3437051cac..077c35a2aa 100644 --- a/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll +++ b/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() + ) + ) + ) +}