Commit 70f4c6e7 authored by Adam Balogh's avatar Adam Balogh
Browse files

[clan-tidy] Fix false positive in bugprone-infinite-loop 

The checker bugprone-infinite-loop does not track changes of
variables in the initialization expression of a variable
declared inside the condition of the while statement. This
leads to false positives, similarly to the one in the bug
report https://bugs.llvm.org/show_bug.cgi?id=44618. This
patch fixes this issue by enabling tracking of the variables
of this expression as well.

Differential Revision: https://reviews.llvm.org/D73270
parent cbf03aee

clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp

+20 −4
Original line number Diff line number Diff line
@@ -170,18 +170,34 @@ void InfiniteLoopCheck::check(const MatchFinder::MatchResult &Result) { 
  const auto *LoopStmt = Result.Nodes.getNodeAs<Stmt>("loop-stmt");

  const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");



  bool ShouldHaveConditionVariables = true;

  if (const auto *While = dyn_cast<WhileStmt>(LoopStmt)) {

    if (const VarDecl *LoopVarDecl = While->getConditionVariable()) {

      if (const Expr *Init = LoopVarDecl->getInit()) {

        ShouldHaveConditionVariables = false;

        Cond = Init;

      }

    }

  }



  if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))

    return;



  std::string CondVarNames = getCondVarNames(Cond);

  if (CondVarNames.empty())

  if (ShouldHaveConditionVariables && CondVarNames.empty())

    return;



  if (CondVarNames.empty()) {

    diag(LoopStmt->getBeginLoc(),

         "this loop is infinite; it does not check any variables in the"

         " condition");

  } else {

    diag(LoopStmt->getBeginLoc(),

         "this loop is infinite; none of its condition variables (%0)"

         " are updated in the loop body")

      << CondVarNames;

  }

}



} // namespace bugprone

} // namespace tidy

clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp

+37 −1
Original line number Diff line number Diff line
@@ -8,6 +8,16 @@ void simple_infinite_loop1() { 
    j++;

  }



  while (int k = 10) {

    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; it does not check any variables in the condition [bugprone-infinite-loop]

    j--;

  }



  while (int k = 10) {

    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; it does not check any variables in the condition [bugprone-infinite-loop]

    k--;

  }



  do {

  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]

    j++;
@@ -27,6 +37,16 @@ void simple_infinite_loop2() { 
    j++;

  }



  while (int k = Limit) {

    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (Limit) are updated in the loop body [bugprone-infinite-loop]

    j--;

  }



  while (int k = Limit) {

    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (Limit) are updated in the loop body [bugprone-infinite-loop]

    k--;

  }



  do {

  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]

    j++;
@@ -44,6 +64,22 @@ void simple_not_infinite1() { 
    // Not an error since 'Limit' is updated.

    Limit--;

  }



  while (Limit--) {

    // Not an error since 'Limit' is updated.

    i++;

  }



  while (int k = Limit) {

    // Not an error since 'Limit' is updated.

    Limit--;

  }



  while (int k = Limit--) {

    // Not an error since 'Limit' is updated.

    i++;

  }



  do {

    Limit--;

  } while (i < Limit);