[clang-tools-extra] 86026ee - [clang-tidy] Warn about misuse of sizeof operator in loops. (#143205)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 25 10:04:14 PDT 2025
Author: Malavika Samak
Date: 2025-06-25T10:04:10-07:00
New Revision: 86026ee623cd9f02f4277a1f1ff0589b1b16fb30
URL: https://github.com/llvm/llvm-project/commit/86026ee623cd9f02f4277a1f1ff0589b1b16fb30
DIFF: https://github.com/llvm/llvm-project/commit/86026ee623cd9f02f4277a1f1ff0589b1b16fb30.diff
LOG: [clang-tidy] Warn about misuse of sizeof operator in loops. (#143205)
The sizeof operator misuses in loop conditionals can be a source of
bugs. The common misuse is attempting to retrieve the number of elements
in the array by using the sizeof expression and forgetting to divide the
value by the sizeof the array elements. This results in an incorrect
computation of the array length and requires a warning from the sizeof
checker.
Example:
```
int array[20];
void test_for_loop() {
// Needs warning.
for(int i = 0; i < sizeof(array); i++) {
array[i] = i;
}
}
void test_while_loop() {
int count = 0;
// Needs warning.
while(count < sizeof(array)) {
array[count] = 0;
count = count + 2;
}
}
```
rdar://151403083
---------
Co-authored-by: MalavikaSamak <malavika2 at apple.com>
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index 9eeba867f5211..e9ef0afc5ed67 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -72,7 +72,9 @@ SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
Options.get("WarnOnSizeOfPointerToAggregate", true)),
WarnOnSizeOfPointer(Options.get("WarnOnSizeOfPointer", false)),
WarnOnOffsetDividedBySizeOf(
- Options.get("WarnOnOffsetDividedBySizeOf", true)) {}
+ Options.get("WarnOnOffsetDividedBySizeOf", true)),
+ WarnOnSizeOfInLoopTermination(
+ Options.get("WarnOnSizeOfInLoopTermination", true)) {}
void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
@@ -86,6 +88,8 @@ void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnSizeOfPointer", WarnOnSizeOfPointer);
Options.store(Opts, "WarnOnOffsetDividedBySizeOf",
WarnOnOffsetDividedBySizeOf);
+ Options.store(Opts, "WarnOnSizeOfInLoopTermination",
+ WarnOnSizeOfInLoopTermination);
}
void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
@@ -93,6 +97,13 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
// Some of the checks should not match in template code to avoid false
// positives if sizeof is applied on template argument.
+ auto LoopCondExpr =
+ [](const ast_matchers::internal::Matcher<Stmt> &InnerMatcher) {
+ return stmt(anyOf(forStmt(hasCondition(InnerMatcher)),
+ whileStmt(hasCondition(InnerMatcher)),
+ doStmt(hasCondition(InnerMatcher))));
+ };
+
const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
const auto ConstantExpr = ignoringParenImpCasts(
anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
@@ -130,6 +141,14 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
this);
}
+ if (WarnOnSizeOfInLoopTermination) {
+ auto CondExpr = binaryOperator(
+ allOf(has(SizeOfExpr.bind("sizeof-expr")), isComparisonOperator()));
+ Finder->addMatcher(LoopCondExpr(anyOf(CondExpr, hasDescendant(CondExpr)))
+ .bind("loop-expr"),
+ this);
+ }
+
// Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"';
const auto CharPtrType = pointerType(pointee(isAnyCharacter()));
const auto ConstStrLiteralDecl =
@@ -349,6 +368,23 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
diag(E->getBeginLoc(),
"suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?")
<< E->getSourceRange();
+ } else if (const auto *E = Result.Nodes.getNodeAs<Stmt>("loop-expr")) {
+ auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
+ if (const auto member = dyn_cast<MemberPointerType>(SizeofArgTy))
+ SizeofArgTy = member->getPointeeType().getTypePtr();
+
+ const auto *SzOfExpr = Result.Nodes.getNodeAs<Expr>("sizeof-expr");
+
+ if (const auto type = dyn_cast<ArrayType>(SizeofArgTy)) {
+ // check if the array element size is larger than one. If true,
+ // the size of the array is higher than the number of elements
+ CharUnits sSize = Ctx.getTypeSizeInChars(type->getElementType());
+ if (!sSize.isOne()) {
+ diag(SzOfExpr->getBeginLoc(),
+ "suspicious usage of 'sizeof' in the loop")
+ << SzOfExpr->getSourceRange();
+ }
+ }
} else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-pointer")) {
diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
"of pointer type")
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
index fbd62cb80fb2d..e979b4723cf2e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
@@ -32,6 +32,7 @@ class SizeofExpressionCheck : public ClangTidyCheck {
const bool WarnOnSizeOfPointerToAggregate;
const bool WarnOnSizeOfPointer;
const bool WarnOnOffsetDividedBySizeOf;
+ const bool WarnOnSizeOfInLoopTermination;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fc51f3c9329ad..934d52086b3b9 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,11 @@ Changes in existing checks
<clang-tidy/checks/bugprone/optional-value-conversion>` check to detect
conversion in argument of ``std::make_optional``.
+- Improved :doc: `bugprone-sizeof-expression
+ <clang-tidy/checks/bugprone/bugprone-sizeof-expression>` check by adding
+ `WarnOnSizeOfInLoopTermination` option to detect misuses of ``sizeof``
+ expression in loop conditions.
+
- Improved :doc:`bugprone-string-constructor
<clang-tidy/checks/bugprone/string-constructor>` check to find suspicious
calls of ``std::string`` constructor with char pointer, start position and
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
index 29edb26ed7aa2..04824cc1fe0e4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
@@ -316,3 +316,12 @@ Options
When `true`, the check will warn on pointer arithmetic where the
element count is obtained from a division with ``sizeof(...)``,
e.g., ``Ptr + Bytes / sizeof(*T)``. Default is `true`.
+
+.. option:: WarnOnSizeOfInLoopTermination
+
+ When `true`, the check will warn about incorrect use of sizeof expression
+ in loop termination condition. The warning triggers if the ``sizeof``
+ expression appears to be incorrectly used to determine the number of
+ array/buffer elements.
+ e.g, ``long arr[10]; for(int i = 0; i < sizeof(arr); i++) { ... }``. Default
+ is `true`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
index 5e6f394152e9d..33cf1cbea8377 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -164,6 +164,69 @@ int Test2(MyConstChar* A) {
return sum;
}
+struct A {
+ int array[10];
+};
+
+struct B {
+ struct A a;
+};
+
+void loop_access_elements(int num, struct B b) {
+ struct A arr[10];
+ char buf[20];
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:22: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression]
+ for(int i = 0; i < sizeof(arr); i++) {
+ struct A a = arr[i];
+ }
+
+ // Loop warning should not trigger here, even though this code is incorrect
+ // CHECK-MESSAGES: :[[@LINE+2]]:22: warning: suspicious usage of 'sizeof(K)'; did you mean 'K'? [bugprone-sizeof-expression]
+ // CHECK-MESSAGES: :[[@LINE+1]]:32: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator [bugprone-sizeof-expression]
+ for(int i = 0; i < sizeof(10)/sizeof(A); i++) {
+ struct A a = arr[i];
+ }
+
+ // Should not warn here
+ for(int i = 0; i < sizeof(arr)/sizeof(A); i++) {}
+
+ // Should not warn here
+ for (int i = 0; i < 10; i++) {
+ if (sizeof(arr) != 0) {
+
+ }
+ }
+
+ for (int i = 0; i < 10; i++) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:25: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression]
+ for (int j = 0; j < sizeof(arr); j++) {
+ }
+ }
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:22: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression]
+ for(int j = 0; j < sizeof(b.a.array); j++) {}
+
+ // Should not warn here
+ for(int i = 0; i < sizeof(buf); i++) {}
+
+ // Should not warn here
+ for(int i = 0; i < (sizeof(arr) << 3); i++) {}
+
+ int i = 0;
+ // CHECK-MESSAGES: :[[@LINE+1]]:14: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression]
+ while(i <= sizeof(arr)) {i++;}
+
+ i = 0;
+ do {
+ i++;
+ // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression]
+ } while(i <= sizeof(arr));
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:29: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression]
+ for(int i = 0, j = 0; i < sizeof(arr) && j < sizeof(buf); i++, j++) {}
+}
+
template <int T>
int Foo() { int A[T]; return sizeof(T); }
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: suspicious usage of 'sizeof(K)'
More information about the cfe-commits
mailing list