[clang-tools-extra] [clang-tidy] Fix crash in modernize-loop-convert when int is used as iterator (PR #78796)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 19 14:09:13 PST 2024


https://github.com/PiotrZSL created https://github.com/llvm/llvm-project/pull/78796

Fix crash when built-in type (like int) is used as iterator, or when call to begin() return integer.

Closes #78381

>From 812b49d8e5e6c07a9d26b36413b26e6f0dbe277f Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Fri, 19 Jan 2024 22:05:35 +0000
Subject: [PATCH] [clang-tidy] Fix crash in modernize-loop-convert when int is
 used as iterator

Fix crash when built-in type (like int) is used as iterator,
or when call to begin() return integer.

Closes #78381
---
 .../clang-tidy/modernize/LoopConvertCheck.cpp       | 12 ++++++++----
 clang-tools-extra/docs/ReleaseNotes.rst             |  5 +++--
 .../checkers/modernize/loop-convert-basic.cpp       | 13 +++++++++++++
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index 8beaa62c78ba0a..f0791da143ad9d 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -942,11 +942,15 @@ bool LoopConvertCheck::isConvertible(ASTContext *Context,
         CanonicalInitVarType->isPointerType()) {
       // If the initializer and the variable are both pointers check if the
       // un-qualified pointee types match, otherwise we don't use auto.
-      if (!Context->hasSameUnqualifiedType(
-              CanonicalBeginType->getPointeeType(),
-              CanonicalInitVarType->getPointeeType()))
-        return false;
+      return Context->hasSameUnqualifiedType(
+          CanonicalBeginType->getPointeeType(),
+          CanonicalInitVarType->getPointeeType());
     }
+
+    if (CanonicalBeginType->isBuiltinType() ||
+        CanonicalInitVarType->isBuiltinType())
+      return false;
+
   } else if (FixerKind == LFK_PseudoArray) {
     if (const auto *EndCall = Nodes.getNodeAs<CXXMemberCallExpr>(EndCallName)) {
       // This call is required to obtain the container.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d77267588db915..7f618e71afd1ce 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -410,7 +410,8 @@ Changes in existing checks
 - Improved :doc:`modernize-loop-convert
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``
-  and avoid crash for array of dependent array.
+  and avoid crash for array of dependent array and non-dereferenceable builtin
+  types used as iterators.
 
 - Improved :doc:`modernize-make-shared
   <clang-tidy/checks/modernize/make-shared>` check to support
@@ -502,7 +503,7 @@ Changes in existing checks
   <clang-tidy/checks/readability/implicit-bool-conversion>` check to take
   do-while loops into account for the `AllowIntegerConditions` and
   `AllowPointerConditions` options. It also now provides more consistent
-  suggestions when parentheses are added to the return value or expressions. 
+  suggestions when parentheses are added to the return value or expressions.
   It also ignores false-positives for comparison containing bool bitfield.
 
 - Improved :doc:`readability-misleading-indentation
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
index e2b9336d620f50..c29fbc9f9b23b7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -954,3 +954,16 @@ void dependenceArrayTest() {
 }
 
 } // namespace PseudoArray
+
+namespace PR78381 {
+  struct blocked_range {
+    int begin() const;
+    int end() const;
+  };
+
+  void test() {
+    blocked_range r;
+    for (auto i = r.begin(); i!=r.end(); ++i) {
+    }
+  }
+}



More information about the cfe-commits mailing list