This revision was automatically updated to reflect the committed changes.
Closed by commit rG47518d6a0aed: [clang-tidy] Improving bugprone-sizeof-expr
check. (authored by balazske).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91543/new/
https://reviews.llvm.org/D91543
Files:
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
@@ -187,6 +187,7 @@
int Test5() {
typedef int Array10[10];
+ typedef C ArrayC[10];
struct MyStruct {
Array10 arr;
@@ -203,6 +204,8 @@
PMyStruct PS;
PMyStruct2 PS2;
Array10 A10;
+ C *PtrArray[10];
+ C *PC;
int sum = 0;
sum += sizeof(&S.arr);
@@ -239,6 +242,17 @@
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
sum += sizeof(&A10);
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(PtrArray) / sizeof(PtrArray[1]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(A10) / sizeof(PtrArray[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(PC) / sizeof(PtrArray[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ // CHECK-MESSAGES: :[[@LINE-3]]:23: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(ArrayC) / sizeof(PtrArray[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
return sum;
}
@@ -276,6 +290,10 @@
int A[] = {1, 2, 3, 4};
static const char str[] = "hello";
static const char* ptr[] { "aaa", "bbb", "ccc" };
+ typedef C *CA10[10];
+ C *PtrArray[10];
+ CA10 PtrArray1;
+
int sum = 0;
if (sizeof(A) < 10)
sum += sizeof(A);
@@ -293,5 +311,10 @@
sum += sizeof(str) / sizeof(str[0]);
sum += sizeof(ptr) / sizeof(ptr[0]);
sum += sizeof(ptr) / sizeof(*(ptr));
+ sum += sizeof(PtrArray) / sizeof(PtrArray[0]);
+ // Canonical type of PtrArray1 is same as PtrArray.
+ sum = sizeof(PtrArray) / sizeof(PtrArray1[0]);
+ // There is no warning for 'sizeof(T*)/sizeof(Q)' case.
+ sum += sizeof(PtrArray) / sizeof(A[0]);
return sum;
}
Index: clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -77,6 +77,10 @@
}
void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
+ // FIXME:
+ // Some of the checks should not match in template code to avoid false
+ // positives if sizeof is applied on template argument.
+
const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
const auto ConstantExpr = expr(ignoringParenImpCasts(
anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
@@ -132,6 +136,7 @@
this);
// 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 = expr(ignoringParenImpCasts(
expr(hasType(qualType(hasCanonicalType(arrayType()))))));
const auto ArrayCastExpr = expr(anyOf(
@@ -151,13 +156,31 @@
hasType(qualType(hasCanonicalType(PointerToStructType))),
unless(cxxThisExpr()))));
- Finder->addMatcher(
- expr(anyOf(sizeOfExpr(has(expr(ignoringParenImpCasts(
- anyOf(ArrayCastExpr, PointerToArrayExpr, StructAddrOfExpr,
- PointerToStructExpr))))),
- sizeOfExpr(has(PointerToStructType))))
- .bind("sizeof-pointer-to-aggregate"),
- this);
+ const auto ArrayOfPointersExpr = expr(ignoringParenImpCasts(expr(hasType(
+ qualType(hasCanonicalType(arrayType(hasElementType(pointerType()))
+ .bind("type-of-array-of-pointers")))))));
+ const auto ArrayOfSamePointersExpr =
+ expr(ignoringParenImpCasts(expr(hasType(qualType(hasCanonicalType(
+ arrayType(equalsBoundNode("type-of-array-of-pointers"))))))));
+ const auto ZeroLiteral =
+ expr(ignoringParenImpCasts(integerLiteral(equals(0))));
+ const auto ArrayOfSamePointersZeroSubscriptExpr =
+ expr(ignoringParenImpCasts(arraySubscriptExpr(
+ hasBase(ArrayOfSamePointersExpr), hasIndex(ZeroLiteral))));
+ const auto ArrayLengthExprDenom =
+ expr(hasParent(expr(ignoringParenImpCasts(
+ binaryOperator(hasOperatorName("/"),
+ hasLHS(expr(ignoringParenImpCasts(expr(
+ sizeOfExpr(has(ArrayOfPointersExpr)))))))))),
+ sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
+
+ Finder->addMatcher(expr(anyOf(sizeOfExpr(has(expr(ignoringParenImpCasts(anyOf(
+ ArrayCastExpr, PointerToArrayExpr,
+ StructAddrOfExpr, PointerToStructExpr))))),
+ sizeOfExpr(has(PointerToStructType))),
+ unless(ArrayLengthExprDenom))
+ .bind("sizeof-pointer-to-aggregate"),
+ this);
// Detect expression like: sizeof(epxr) <= k for a suspicious constant 'k'.
if (WarnOnSizeOfCompareToConstant) {
@@ -178,6 +201,12 @@
this);
// Detect sizeof(...) /sizeof(...));
+ // FIXME:
+ // Re-evaluate what cases to handle by the checker.
+ // Probably any sizeof(A)/sizeof(B) should be error if
+ // 'A' is not an array (type) and 'B' the (type of the) first element of it.
+ // Except if 'A' and 'B' are non-pointers, then use the existing size division
+ // rule.
const auto ElemType =
arrayType(hasElementType(recordType().bind("elem-type")));
const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
