[clang-tools-extra] 0ca5993 - [clang-tidy] Add option WarnOnSizeOfPointerToAggregate.
Michael Benfield via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 22 10:10:40 PDT 2022
Author: Michael Benfield
Date: 2022-09-22T17:09:43Z
New Revision: 0ca5993741877ab7fd27a251cbc1895bd092d5ee
URL: https://github.com/llvm/llvm-project/commit/0ca5993741877ab7fd27a251cbc1895bd092d5ee
DIFF: https://github.com/llvm/llvm-project/commit/0ca5993741877ab7fd27a251cbc1895bd092d5ee.diff
LOG: [clang-tidy] Add option WarnOnSizeOfPointerToAggregate.
This is now an option under the check bugprone-sizeof-expression.
Differential Revision: https://reviews.llvm.org/D134381
Added:
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index fe5c80658f268..57086af9c7031 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -67,7 +67,9 @@ SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
Options.get("WarnOnSizeOfIntegerExpression", false)),
WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", true)),
WarnOnSizeOfCompareToConstant(
- Options.get("WarnOnSizeOfCompareToConstant", true)) {}
+ Options.get("WarnOnSizeOfCompareToConstant", true)),
+ WarnOnSizeOfPointerToAggregate(
+ Options.get("WarnOnSizeOfPointerToAggregate", true)) {}
void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
@@ -76,6 +78,8 @@ void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
Options.store(Opts, "WarnOnSizeOfCompareToConstant",
WarnOnSizeOfCompareToConstant);
+ Options.store(Opts, "WarnOnSizeOfPointerToAggregate",
+ WarnOnSizeOfPointerToAggregate);
}
void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
@@ -135,46 +139,48 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
// Detect sizeof(ptr) where ptr points to an aggregate (i.e. sizeof(&S)).
// Do not find it if RHS of a 'sizeof(arr) / sizeof(arr[0])' expression.
- const auto ArrayExpr =
- ignoringParenImpCasts(hasType(hasCanonicalType(arrayType())));
- const auto ArrayCastExpr = expr(anyOf(
- unaryOperator(hasUnaryOperand(ArrayExpr), unless(hasOperatorName("*"))),
- binaryOperator(hasEitherOperand(ArrayExpr)),
- castExpr(hasSourceExpression(ArrayExpr))));
- const auto PointerToArrayExpr = ignoringParenImpCasts(
- hasType(hasCanonicalType(pointerType(pointee(arrayType())))));
-
- const auto StructAddrOfExpr = unaryOperator(
- hasOperatorName("&"), hasUnaryOperand(ignoringParenImpCasts(
- hasType(hasCanonicalType(recordType())))));
- const auto PointerToStructType =
- hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
- const auto PointerToStructExpr = ignoringParenImpCasts(expr(
- hasType(hasCanonicalType(PointerToStructType)), unless(cxxThisExpr())));
-
- const auto ArrayOfPointersExpr = ignoringParenImpCasts(
- hasType(hasCanonicalType(arrayType(hasElementType(pointerType()))
- .bind("type-of-array-of-pointers"))));
- const auto ArrayOfSamePointersExpr =
- ignoringParenImpCasts(hasType(hasCanonicalType(
- arrayType(equalsBoundNode("type-of-array-of-pointers")))));
- const auto ZeroLiteral = ignoringParenImpCasts(integerLiteral(equals(0)));
- const auto ArrayOfSamePointersZeroSubscriptExpr =
- ignoringParenImpCasts(arraySubscriptExpr(hasBase(ArrayOfSamePointersExpr),
- hasIndex(ZeroLiteral)));
- const auto ArrayLengthExprDenom =
- expr(hasParent(expr(ignoringParenImpCasts(binaryOperator(
- hasOperatorName("/"), hasLHS(ignoringParenImpCasts(sizeOfExpr(
- has(ArrayOfPointersExpr)))))))),
- sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
-
- Finder->addMatcher(expr(anyOf(sizeOfExpr(has(ignoringParenImpCasts(anyOf(
- ArrayCastExpr, PointerToArrayExpr,
- StructAddrOfExpr, PointerToStructExpr)))),
- sizeOfExpr(has(PointerToStructType))),
- unless(ArrayLengthExprDenom))
- .bind("sizeof-pointer-to-aggregate"),
- this);
+ if (WarnOnSizeOfPointerToAggregate) {
+ const auto ArrayExpr =
+ ignoringParenImpCasts(hasType(hasCanonicalType(arrayType())));
+ const auto ArrayCastExpr = expr(anyOf(
+ unaryOperator(hasUnaryOperand(ArrayExpr), unless(hasOperatorName("*"))),
+ binaryOperator(hasEitherOperand(ArrayExpr)),
+ castExpr(hasSourceExpression(ArrayExpr))));
+ const auto PointerToArrayExpr = ignoringParenImpCasts(
+ hasType(hasCanonicalType(pointerType(pointee(arrayType())))));
+
+ const auto StructAddrOfExpr = unaryOperator(
+ hasOperatorName("&"), hasUnaryOperand(ignoringParenImpCasts(
+ hasType(hasCanonicalType(recordType())))));
+ const auto PointerToStructType =
+ hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
+ const auto PointerToStructExpr = ignoringParenImpCasts(expr(
+ hasType(hasCanonicalType(PointerToStructType)), unless(cxxThisExpr())));
+
+ const auto ArrayOfPointersExpr = ignoringParenImpCasts(
+ hasType(hasCanonicalType(arrayType(hasElementType(pointerType()))
+ .bind("type-of-array-of-pointers"))));
+ const auto ArrayOfSamePointersExpr =
+ ignoringParenImpCasts(hasType(hasCanonicalType(
+ arrayType(equalsBoundNode("type-of-array-of-pointers")))));
+ const auto ZeroLiteral = ignoringParenImpCasts(integerLiteral(equals(0)));
+ const auto ArrayOfSamePointersZeroSubscriptExpr =
+ ignoringParenImpCasts(arraySubscriptExpr(
+ hasBase(ArrayOfSamePointersExpr), hasIndex(ZeroLiteral)));
+ const auto ArrayLengthExprDenom =
+ expr(hasParent(expr(ignoringParenImpCasts(binaryOperator(
+ hasOperatorName("/"), hasLHS(ignoringParenImpCasts(sizeOfExpr(
+ has(ArrayOfPointersExpr)))))))),
+ sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
+
+ Finder->addMatcher(expr(anyOf(sizeOfExpr(has(ignoringParenImpCasts(anyOf(
+ ArrayCastExpr, PointerToArrayExpr,
+ StructAddrOfExpr, PointerToStructExpr)))),
+ sizeOfExpr(has(PointerToStructType))),
+ unless(ArrayLengthExprDenom))
+ .bind("sizeof-pointer-to-aggregate"),
+ this);
+ }
// Detect expression like: sizeof(expr) <= k for a suspicious constant 'k'.
if (WarnOnSizeOfCompareToConstant) {
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
index b1520dc1cafa2..a9291c40640de 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
@@ -31,6 +31,7 @@ class SizeofExpressionCheck : public ClangTidyCheck {
const bool WarnOnSizeOfIntegerExpression;
const bool WarnOnSizeOfThis;
const bool WarnOnSizeOfCompareToConstant;
+ const bool WarnOnSizeOfPointerToAggregate;
};
} // namespace bugprone
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp
new file mode 100644
index 0000000000000..095a3cc16b16d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- -config="{CheckOptions: [{key: bugprone-sizeof-expression.WarnOnSizeOfPointerToAggregate, value: false}]}" --
+
+class C {
+ int size() { return sizeof(this); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#pragma pack(1)
+struct S { char a, b, c; };
+
+int Test5() {
+ typedef int Array10[10];
+ typedef C ArrayC[10];
+
+ struct MyStruct {
+ Array10 arr;
+ Array10* ptr;
+ };
+ typedef const MyStruct TMyStruct;
+ typedef const MyStruct *PMyStruct;
+ typedef TMyStruct *PMyStruct2;
+
+ static TMyStruct kGlocalMyStruct = {};
+ static TMyStruct volatile * kGlocalMyStructPtr = &kGlocalMyStruct;
+
+ MyStruct S;
+ PMyStruct PS;
+ PMyStruct2 PS2;
+ Array10 A10;
+ C *PtrArray[10];
+ C *PC;
+
+ int sum = 0;
+ sum += sizeof(&S.arr);
+ // No warning.
+ sum += sizeof(&kGlocalMyStruct.arr);
+ // No warning.
+ sum += sizeof(&kGlocalMyStructPtr->arr);
+ // No warning.
+ sum += sizeof(S.arr + 0);
+ // No warning.
+ sum += sizeof(+ S.arr);
+ // No warning.
+ sum += sizeof((int*)S.arr);
+ // No warning.
+
+ sum += sizeof(S.ptr);
+ // No warning.
+ sum += sizeof(kGlocalMyStruct.ptr);
+ // No warning.
+ sum += sizeof(kGlocalMyStructPtr->ptr);
+ // No warning.
+
+ sum += sizeof(&kGlocalMyStruct);
+ // No warning.
+ sum += sizeof(&S);
+ // No warning.
+ sum += sizeof(MyStruct*);
+ sum += sizeof(PMyStruct);
+ sum += sizeof(PS);
+ // No warning.
+ sum += sizeof(PS2);
+ // No warning.
+ sum += sizeof(&A10);
+ // No warning.
+ sum += sizeof(PtrArray) / sizeof(PtrArray[1]);
+ // No warning.
+ sum += sizeof(A10) / sizeof(PtrArray[0]);
+ // No warning.
+ sum += sizeof(PC) / sizeof(PtrArray[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ sum += sizeof(ArrayC) / sizeof(PtrArray[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+
+ return sum;
+}
More information about the cfe-commits
mailing list