github / codeql-coding-standards

This repository contains CodeQL queries and libraries which support various Coding Standards.
MIT License
129 stars 59 forks source link

`M0-1-9`: constexpr used in array size is detected as dead code #678

Closed fjatWbyT closed 1 month ago

fjatWbyT commented 2 months ago

Affected rules

Description

Integer constant expression used for the size in an array declaration produces dead-code false positive. In other words, the e.g. constexpr int is not dead code because it is used to define the array size. This is contrast with when the size is (static) const, which does not produce the alert.

Example

int main() {
    constexpr int constexpr_unused = 1;   // True Positives, these first three are indeed unused / dead code
    static const int static_const_unused = 2;
    int unused_variable = 3;                            

    constexpr int constexpr_size       = 7;  // dead code detection  <-- False Positive, it is used in array a

    static const int static_const_size = 8;  // True Negative, 
                                             // (static) const doesn't trigger dead code

    int a[constexpr_size]    = {};  // The remaining have no dead code issue either
    int c[static_const_size] = {};  // they are used at the end

    return a[0] + c[2];
}
fjatWbyT commented 2 months ago

Fix Strategy Proposal

Add an additional case to the predicate isDeadStmt in DeadCode.qll. The approach in #660 can be reused to determine if the constant expression is used in an array size.

There could be value avoiding a bit of code duplication so that the M0-1-3 fix from that PR and this proposed fix for M0-1-9 share a predicate that returns the count.

As I am still learning CodeQL and writing my first queries, I have already been playing with the fix in c765f9375 and unit-testing it. I tried first using the pure / maybePure builtin predicates, but I didn’t manage to separate the constexpr and const cases only with them.

lcartey commented 2 months ago

I think your fix sounds reasonable 👍

fjatWbyT commented 2 months ago

Thank you 😊 I have applied it in #690.