diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h @@ -31,6 +31,7 @@ const bool WarnOnSizeOfIntegerExpression; const bool WarnOnSizeOfThis; const bool WarnOnSizeOfCompareToConstant; + const bool WarnOnSizeOfPointerToAggregate; }; } // namespace bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp @@ -67,7 +67,9 @@ 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 @@ Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis); Options.store(Opts, "WarnOnSizeOfCompareToConstant", WarnOnSizeOfCompareToConstant); + Options.store(Opts, "WarnOnSizeOfPointerToAggregate", + WarnOnSizeOfPointerToAggregate); } void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { @@ -135,46 +139,48 @@ // 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/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 --- /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; +}