diff --git a/change_notes/2024-09-17-fix-fp-678-m0-1-9.md b/change_notes/2024-09-17-fix-fp-678-m0-1-9.md new file mode 100644 index 0000000000..e068825f4c --- /dev/null +++ b/change_notes/2024-09-17-fix-fp-678-m0-1-9.md @@ -0,0 +1,2 @@ +- `M0-1-9` - `DeadCode.qll`: + - Fixes #678. Remove dead code false positive when integer constant expression is used to define the size of an array. diff --git a/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql b/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql index 5956515e5b..e89e9ec135 100644 --- a/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql +++ b/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql @@ -18,12 +18,6 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.deadcode.UnusedVariables -/** Gets the constant value of a constexpr/const variable. */ -private string getConstExprValue(Variable v) { - result = v.getInitializer().getExpr().getValue() and - (v.isConst() or v.isConstexpr()) -} - // This predicate is similar to getUseCount for M0-1-4 except that it also // considers static_asserts. This was created to cater for M0-1-3 specifically // and hence, doesn't attempt to reuse the M0-1-4 specific predicate @@ -44,11 +38,7 @@ int getUseCountConservatively(Variable v) { count(StaticAssert s | s.getCondition().getAChild*().getValue() = getConstExprValue(v)) + // In case an array type uses a constant in the same scope as the constexpr variable, // consider it as used. - count(ArrayType at, LocalVariable arrayVariable | - arrayVariable.getType().resolveTypedefs() = at and - v.(PotentiallyUnusedLocalVariable).getFunction() = arrayVariable.getFunction() and - at.getArraySize().toString() = getConstExprValue(v) - ) + countUsesInLocalArraySize(v) } from PotentiallyUnusedLocalVariable v diff --git a/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll b/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll index f4607d82cb..912d2babcd 100644 --- a/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll +++ b/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll @@ -150,3 +150,21 @@ predicate maybeACompileTimeTemplateArgument(Variable v) { ) ) } + +/** Gets the constant value of a constexpr/const variable. */ +string getConstExprValue(Variable v) { + result = v.getInitializer().getExpr().getValue() and + (v.isConst() or v.isConstexpr()) +} + +/** + * Counts uses of `Variable` v in a local array of size `n` + */ +int countUsesInLocalArraySize(Variable v) { + result = + count(ArrayType at, LocalVariable arrayVariable | + arrayVariable.getType().resolveTypedefs() = at and + v.(PotentiallyUnusedLocalVariable).getFunction() = arrayVariable.getFunction() and + at.getArraySize().toString() = getConstExprValue(v) + ) +} diff --git a/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll b/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll index 4a008dc15a..2b5be15e80 100644 --- a/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll +++ b/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll @@ -16,6 +16,7 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.deadcode.UselessAssignments import codingstandards.cpp.deadcode.UnreachableCode +import codingstandards.cpp.deadcode.UnusedVariables abstract class DeadCodeSharedQuery extends Query { } @@ -39,6 +40,7 @@ predicate isDeadStmt(Stmt s) { // - All the declarations are variable declarations // - None of those variables are ever accessed in non-dead code // - The initializers for each of the variables are pure + // - It isn't constexpr and used to declare an array size exists(DeclStmt ds | ds = s and // Use forex so that we don't flag "fake" generated `DeclStmt`s (e.g. those generated by the @@ -50,7 +52,8 @@ predicate isDeadStmt(Stmt s) { not exists(VariableAccess va | va.getTarget() = v and not isDeadOrUnreachableStmt(va.getEnclosingStmt()) - ) + ) and + not countUsesInLocalArraySize(v) > 0 ) ) ) diff --git a/cpp/common/test/rules/deadcode/DeadCode.expected b/cpp/common/test/rules/deadcode/DeadCode.expected index 6c111d8a93..aec93e0914 100644 --- a/cpp/common/test/rules/deadcode/DeadCode.expected +++ b/cpp/common/test/rules/deadcode/DeadCode.expected @@ -12,3 +12,6 @@ | test.cpp:72:3:73:3 | try { ... } | This statement is dead code. | | test.cpp:73:17:74:3 | { ... } | This statement is dead code. | | test.cpp:79:17:80:3 | { ... } | This statement is dead code. | +| test.cpp:85:3:85:43 | declaration | This statement is dead code. | +| test.cpp:87:3:87:30 | declaration | This statement is dead code. | +| test.cpp:90:3:90:50 | declaration | This statement is dead code. | diff --git a/cpp/common/test/rules/deadcode/test.cpp b/cpp/common/test/rules/deadcode/test.cpp index ba5c59b07c..d9c0cab277 100644 --- a/cpp/common/test/rules/deadcode/test.cpp +++ b/cpp/common/test/rules/deadcode/test.cpp @@ -81,5 +81,13 @@ int test_dead_code(int x) { static_assert(1); // COMPLIANT - return live5 + live6; // COMPLIANT -} \ No newline at end of file + constexpr int constexpr_array_size{6}; // COMPLIANT + int unused_array[constexpr_array_size]{}; // NON_COMPLIANT + + constexpr int unused_int{2}; // NON_COMPLIANT + + constexpr int constexpr_used_array[]{3, 4, 5}; // COMPLIANT + constexpr int constexpr_unused_array[]{0, 1, 2}; // NON_COMPLIANT + + return live5 + live6 + constexpr_used_array[1]; // COMPLIANT +}