`A4-7-1`: false positives in for loops

Open
#798 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
28/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
cpp
Domain
devtools

Research direction

Start by locating the CodeQL query for affected rule A4-7-1 and compare its behavior with the C++ examples in this issue. The work is done when the shown loop-counter, bounded-iteration, container-size, and auxiliary-span cases no longer produce false positives while genuine overflow risks remain reported.

Written by the indexing model from the issue text.

Description

Difficulty-High false positive/false negative Impact-Medium Standard-AUTOSAR
Affected rules
  • A4-7-1
Description

Typical increment by one in for loops cannot lead to overflow or wrap-around because the number of iterations isn't large enough. The example illustrates true negatives together with false positives:

  • in f the variable used as "loop counter" is defined inside or outside the scope of the loop,
  • and in g the number of iterations changes by a factor 100, although still not close to int limit - in general I think a query should take into account, if possible, the value used to initialize the counter and the (potential maximum) number of iterations.
  • iterating through a container through an auxiliary span of its values (rather than through the container directly).
Example

#include <array>

void f() {
  const int num_iter = 100000;

  for (int i = 0; i < num_iter; i++) {  // OK.
    ;
  }

  int j = 0;
  for (; j < num_iter; j++) {  // False Positive.
    ;
  }
}

void g() {
  constexpr int N1000 = 1000;
  std::array<int, N1000> arr1000;
  std::size_t idx = 0;
  for (; idx < arr1000.size(); ++idx) {  // OK.
    ;
  }

  constexpr int N100000 = 100000;
  std::array<int, N100000> arr100000;
  idx = 0;
  for (; idx < arr100000.size(); ++idx) {  // False Positive.
    ;
  }
}

namespace {

// AUTOSAR is C++14, so a quick C++20's span:
template<typename T>
class span
{
 public:
  span(T* data, std::size_t s) : data_member(data), size_(s) {}
  std::size_t size() { return size_; }
 private:
  T* data_member;
  std::size_t size_;
};

// This could be a member (non-template) function in a custom (template) container.
template<typename Container>
constexpr span<typename Container::value_type> values(Container& c) {
  return {c.data(), c.size()};
}

}

int main() {
  constexpr int N = 1;
  std::array<int, N> arr;
  auto values = ::values(arr);
  std::size_t idx = 0;
  for (; idx < values.size(); ++idx) {  // False Positive.
    ;                                   // Avoid assuming N is declared in the same scope, but OK that it is a
                                        // std::size_t NTTP and/or data member of the container argument to values().
  }
}

Dominant language
CodeQL
Stars
227
Forks
82
Avg merge
6d 7h
Merged PRs (30d)
9

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from github/codeql-coding-standards

All issues in github/codeql-coding-standards

Similar issues

More DevTools issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.