This revision was automatically updated to reflect the committed changes. Closed by commit rGdcde8fdeeb3e: [clang-tidy] bugprone-infinite-loop: Fix false positives with volatile… (authored by dergachev.a).
Changed prior to commit: https://reviews.llvm.org/D108808?vs=369040&id=371202#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108808/new/ https://reviews.llvm.org/D108808 Files: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp @@ -1387,3 +1387,13 @@ } } } + +void volatile_concrete_address() { + // No warning. The value behind the volatile concrete address + // is beyond our control. It may change at any time. + if (*(volatile int *)0x1234) { + if (*(volatile int *)0x1234) { + doSomething(); + } + } +} Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp @@ -591,3 +591,34 @@ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (x) are updated in the loop body [bugprone-infinite-loop] } } + +void test_volatile_cast() { + // This is a no-op cast. Clang ignores the qualifier, we should too. + for (int i = 0; (volatile int)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] + } +} + +void test_volatile_concrete_address(int i, int size) { + // No warning. The value behind the volatile concrete address + // is beyond our control. It may change at any time. + for (; *((volatile int *)0x1234) < size;) { + } + + for (; *((volatile int *)(0x1234 + i)) < size;) { + } + + for (; **((volatile int **)0x1234) < size;) { + } + + volatile int *x = (volatile int *)0x1234; + for (; *x < 10;) { + } + + // FIXME: This one should probably also be suppressed. + // Whatever the developer is doing here, they can do that again anywhere else + // which basically makes it a global. + for (; *(int *)0x1234 < size;) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (size) are updated in the loop body [bugprone-infinite-loop] + } +} Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp @@ -65,6 +65,17 @@ ObjCIvarRefExpr, ObjCPropertyRefExpr, ObjCMessageExpr>(Cond)) { // FIXME: Handle MemberExpr. return true; + } else if (const auto *CE = dyn_cast<CastExpr>(Cond)) { + QualType T = CE->getType(); + while (true) { + if (T.isVolatileQualified()) + return true; + + if (!T->isAnyPointerType() && !T->isReferenceType()) + break; + + T = T->getPointeeType(); + } } return false;
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp @@ -1387,3 +1387,13 @@ } } } + +void volatile_concrete_address() { + // No warning. The value behind the volatile concrete address + // is beyond our control. It may change at any time. + if (*(volatile int *)0x1234) { + if (*(volatile int *)0x1234) { + doSomething(); + } + } +} Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp @@ -591,3 +591,34 @@ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (x) are updated in the loop body [bugprone-infinite-loop] } } + +void test_volatile_cast() { + // This is a no-op cast. Clang ignores the qualifier, we should too. + for (int i = 0; (volatile int)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] + } +} + +void test_volatile_concrete_address(int i, int size) { + // No warning. The value behind the volatile concrete address + // is beyond our control. It may change at any time. + for (; *((volatile int *)0x1234) < size;) { + } + + for (; *((volatile int *)(0x1234 + i)) < size;) { + } + + for (; **((volatile int **)0x1234) < size;) { + } + + volatile int *x = (volatile int *)0x1234; + for (; *x < 10;) { + } + + // FIXME: This one should probably also be suppressed. + // Whatever the developer is doing here, they can do that again anywhere else + // which basically makes it a global. + for (; *(int *)0x1234 < size;) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (size) are updated in the loop body [bugprone-infinite-loop] + } +} Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp @@ -65,6 +65,17 @@ ObjCIvarRefExpr, ObjCPropertyRefExpr, ObjCMessageExpr>(Cond)) { // FIXME: Handle MemberExpr. return true; + } else if (const auto *CE = dyn_cast<CastExpr>(Cond)) { + QualType T = CE->getType(); + while (true) { + if (T.isVolatileQualified()) + return true; + + if (!T->isAnyPointerType() && !T->isReferenceType()) + break; + + T = T->getPointeeType(); + } } return false;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits