llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Malavika Samak (malavikasamak) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/143205.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp (+16-1) - (modified) clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h (+1) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp (+31) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp index f3d4c2255d86e..ee66a880792b8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp @@ -72,7 +72,8 @@ SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name, Options.get("WarnOnSizeOfPointerToAggregate", true)), WarnOnSizeOfPointer(Options.get("WarnOnSizeOfPointer", false)), WarnOnOffsetDividedBySizeOf( - Options.get("WarnOnOffsetDividedBySizeOf", true)) {} + Options.get("WarnOnOffsetDividedBySizeOf", true)), + WarnOnLoopExprSizeOf(Options.get("WarnOnLoopExprSizeOf", true)) {} void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant); @@ -86,6 +87,7 @@ void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "WarnOnSizeOfPointer", WarnOnSizeOfPointer); Options.store(Opts, "WarnOnOffsetDividedBySizeOf", WarnOnOffsetDividedBySizeOf); + Options.store(Opts, "WarnOnLoopExprSizeOf", WarnOnLoopExprSizeOf); } void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { @@ -93,6 +95,11 @@ 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 LoopExpr = + [](const ast_matchers::internal::Matcher<Stmt> &InnerMatcher) { + return stmt(anyOf(forStmt(InnerMatcher), whileStmt(InnerMatcher))); + }; + const auto IntegerExpr = ignoringParenImpCasts(integerLiteral()); const auto ConstantExpr = ignoringParenImpCasts( anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)), @@ -130,6 +137,11 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { this); } + if (WarnOnLoopExprSizeOf) { + Finder->addMatcher( + LoopExpr(has(binaryOperator(has(SizeOfExpr)))).bind("loop-expr"), this); + } + // Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"'; const auto CharPtrType = pointerType(pointee(isAnyCharacter())); const auto ConstStrLiteralDecl = @@ -353,6 +365,9 @@ 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")) { + diag(E->getBeginLoc(), "suspicious usage of 'sizeof' in the loop") + << E->getSourceRange(); } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-pointer")) { if (Result.Nodes.getNodeAs<Type>("struct-type")) { diag(E->getBeginLoc(), diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h index fbd62cb80fb2d..f7dccf39687a5 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 WarnOnLoopExprSizeOf; }; } // namespace clang::tidy::bugprone 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..52b71277466b1 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,37 @@ int Test2(MyConstChar* A) { return sum; } +struct A { + int array[10]; +}; + +struct B { + struct A a; +}; + +int square(int num, struct B b) { + struct A arr[10]; + // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression] + for(int i = 0; i < sizeof(arr); i++) { + struct A a = arr[i]; + } + // CHECK-MESSAGES: :[[@LINE+2]]:24: warning: suspicious usage of 'sizeof(K)'; did you mean 'K'? [bugprone-sizeof-expression] + // CHECK-MESSAGES: :[[@LINE+1]]:34: 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]; + } + + for(int i = 0; i < sizeof(arr)/sizeof(A); i++) { + struct A a = arr[i]; + } + + // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious usage of 'sizeof' in the loop [bugprone-sizeof-expression] + for(int j = 0; j < sizeof(b.a); j++) { + + } + return 2; +} + template <int T> int Foo() { int A[T]; return sizeof(T); } // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: suspicious usage of 'sizeof(K)' `````````` </details> https://github.com/llvm/llvm-project/pull/143205 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
