Skip to content

M0-1-9: Fix dead-code false positive when constexpr integer is used in array size #690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 19, 2024
2 changes: 2 additions & 0 deletions change_notes/2024-09-17-fix-fp-678-m0-1-9.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 1 addition & 11 deletions cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 { }

Expand All @@ -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
Expand All @@ -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)
)
)
)
Expand Down
3 changes: 3 additions & 0 deletions cpp/common/test/rules/deadcode/DeadCode.expected
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
12 changes: 10 additions & 2 deletions cpp/common/test/rules/deadcode/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,13 @@ int test_dead_code(int x) {

static_assert(1); // COMPLIANT

return live5 + live6; // COMPLIANT
}
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
}
Loading