[clan-tidy] Fix false positive in bugprone-infinite-loop
authorAdam Balogh <adam.balogh@ericsson.com>
Thu, 23 Jan 2020 14:13:30 +0000 (15:13 +0100)
committerAdam Balogh <adam.balogh@ericsson.com>
Mon, 27 Jan 2020 09:13:55 +0000 (10:13 +0100)
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

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

index 81ae45a..c771ba8 100644 (file)
@@ -170,17 +170,33 @@ 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;
 
-  diag(LoopStmt->getBeginLoc(),
-       "this loop is infinite; none of its condition variables (%0)"
-       " are updated in the loop body")
+  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
index 33d9482..d89bdad 100644 (file)
@@ -4,10 +4,20 @@ void simple_infinite_loop1() {
   int i = 0;
   int j = 0;
   while (i < 10) {
-    // 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]
+  // 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++;
   }
 
+  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);