[clang-tools-extra] 546c816 - [clang-tidy] Improve sizeof(pointer) handling in bugprone-sizeof-expression (#94356)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 11 05:16:46 PDT 2024
Author: Donát Nagy
Date: 2024-06-11T14:16:42+02:00
New Revision: 546c816a529835a4cf89deecff957ea336a94fa2
URL: https://github.com/llvm/llvm-project/commit/546c816a529835a4cf89deecff957ea336a94fa2
DIFF: https://github.com/llvm/llvm-project/commit/546c816a529835a4cf89deecff957ea336a94fa2.diff
LOG: [clang-tidy] Improve sizeof(pointer) handling in bugprone-sizeof-expression (#94356)
This commit reimplements the functionality of the Clang Static Analyzer
checker `alpha.core.SizeofPointer` within clang-tidy by adding a new
(off-by-default) option to bugprone-sizeof-expression which activates
reporting all the `sizeof(ptr)` expressions (where ptr is an expression
that produces a pointer).
The main motivation for this change is that `alpha.core.SizeofPointer`
was an AST-based checker, which did not rely on the path sensitive
capabilities of the Static Analyzer, so there was no reason to keep it
in the Static Analyzer instead of the more lightweight clang-tidy.
After this commit I'm planning to create a separate commit that deletes
`alpha.core.SizeofPointer` from Clang Static Analyzer.
It was natural to place this moved logic in bugprone-sizeof-expression,
because that check already provided several heuristics that reported
various especially suspicious classes of `sizeof(ptr)` expressions.
The new mode `WarnOnSizeOfPointer` is off-by-default, so it won't
surprise the existing users; but it can provide a more through coverage
for the vulnerability CWE-467 ("Use of sizeof() on a Pointer Type") than
the existing partial heuristics.
Previously this checker had an exception that the RHS of a
`sizeof(array) / sizeof(array[0])` expression is not reported; I
generalized this to an exception that the check doesn't report
`sizeof(expr[0])` and `sizeof(*expr)`. This idea is taken from the
Static Analyzer checker `alpha.core.SizeofPointer` (which had an
exception for `*expr`), but analysis of open source projects confirmed
that this indeed eliminates lots of unwanted results.
Note that the suppression of `sizeof(expr[0])` and `sizeof(*expr)`
reports also affects the "old" mode `WarnOnSizeOfPointerToAggregate`
which is enabled by default.
This commit also replaces the old message "suspicious usage of
'sizeof(A*)'; pointer to aggregate" with two more concrete messages; but
I feel that this tidy check would deserve a through cleanup of all the
diagnostic messages that it can produce. (I added a FIXME to mark one
outright misleading message.)
Added:
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp
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-2.c
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 5e64d23874ec1..c25ee42d0899a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -67,7 +67,8 @@ SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
WarnOnSizeOfCompareToConstant(
Options.get("WarnOnSizeOfCompareToConstant", true)),
WarnOnSizeOfPointerToAggregate(
- Options.get("WarnOnSizeOfPointerToAggregate", true)) {}
+ Options.get("WarnOnSizeOfPointerToAggregate", true)),
+ WarnOnSizeOfPointer(Options.get("WarnOnSizeOfPointer", false)) {}
void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
@@ -78,6 +79,7 @@ void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
WarnOnSizeOfCompareToConstant);
Options.store(Opts, "WarnOnSizeOfPointerToAggregate",
WarnOnSizeOfPointerToAggregate);
+ Options.store(Opts, "WarnOnSizeOfPointer", WarnOnSizeOfPointer);
}
void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
@@ -127,17 +129,30 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
const auto ConstStrLiteralDecl =
varDecl(isDefinition(), hasType(hasCanonicalType(CharPtrType)),
hasInitializer(ignoringParenImpCasts(stringLiteral())));
+ const auto VarWithConstStrLiteralDecl = expr(
+ hasType(hasCanonicalType(CharPtrType)),
+ ignoringParenImpCasts(declRefExpr(hasDeclaration(ConstStrLiteralDecl))));
Finder->addMatcher(
- sizeOfExpr(has(ignoringParenImpCasts(
- expr(hasType(hasCanonicalType(CharPtrType)),
- ignoringParenImpCasts(declRefExpr(
- hasDeclaration(ConstStrLiteralDecl)))))))
+ sizeOfExpr(has(ignoringParenImpCasts(VarWithConstStrLiteralDecl)))
.bind("sizeof-charp"),
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.
- if (WarnOnSizeOfPointerToAggregate) {
+ // Detect sizeof(ptr) where ptr is a pointer (CWE-467).
+ //
+ // In WarnOnSizeOfPointerToAggregate mode only report cases when ptr points
+ // to an aggregate type or ptr is an expression that (implicitly or
+ // explicitly) casts an array to a pointer type. (These are more suspicious
+ // than other sizeof(ptr) expressions because they can appear as distorted
+ // forms of the common sizeof(aggregate) expressions.)
+ //
+ // To avoid false positives, the check doesn't report expressions like
+ // 'sizeof(pp[0])' and 'sizeof(*pp)' where `pp` is a pointer-to-pointer or
+ // array of pointers. (This filters out both `sizeof(arr) / sizeof(arr[0])`
+ // expressions and other cases like `p = realloc(p, newsize * sizeof(*p));`.)
+ //
+ // Moreover this generic message is suppressed in cases that are also matched
+ // by the more concrete matchers 'sizeof-this' and 'sizeof-charp'.
+ if (WarnOnSizeOfPointerToAggregate || WarnOnSizeOfPointer) {
const auto ArrayExpr =
ignoringParenImpCasts(hasType(hasCanonicalType(arrayType())));
const auto ArrayCastExpr = expr(anyOf(
@@ -149,32 +164,31 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
const auto PointerToStructType =
hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
- const auto PointerToStructExpr = 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 PointerToStructTypeWithBinding =
+ type(PointerToStructType).bind("struct-type");
+ const auto PointerToStructExpr =
+ expr(hasType(hasCanonicalType(PointerToStructType)));
+
+ const auto PointerToDetectedExpr =
+ WarnOnSizeOfPointer
+ ? expr(hasType(hasUnqualifiedDesugaredType(pointerType())))
+ : expr(anyOf(ArrayCastExpr, PointerToArrayExpr,
+ PointerToStructExpr));
+
const auto ZeroLiteral = ignoringParenImpCasts(integerLiteral(equals(0)));
- const auto ArrayOfSamePointersZeroSubscriptExpr =
- ignoringParenImpCasts(arraySubscriptExpr(
- hasBase(ArrayOfSamePointersExpr), hasIndex(ZeroLiteral)));
- const auto ArrayLengthExprDenom =
- expr(hasParent(binaryOperator(hasOperatorName("/"),
- hasLHS(ignoringParenImpCasts(sizeOfExpr(
- has(ArrayOfPointersExpr)))))),
- sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
+ const auto SubscriptExprWithZeroIndex =
+ arraySubscriptExpr(hasIndex(ZeroLiteral));
+ const auto DerefExpr =
+ ignoringParenImpCasts(unaryOperator(hasOperatorName("*")));
Finder->addMatcher(
- expr(sizeOfExpr(anyOf(
- has(ignoringParenImpCasts(anyOf(
- ArrayCastExpr, PointerToArrayExpr, PointerToStructExpr))),
- has(PointerToStructType))),
- unless(ArrayLengthExprDenom))
- .bind("sizeof-pointer-to-aggregate"),
+ expr(sizeOfExpr(anyOf(has(ignoringParenImpCasts(
+ expr(PointerToDetectedExpr, unless(DerefExpr),
+ unless(SubscriptExprWithZeroIndex),
+ unless(VarWithConstStrLiteralDecl),
+ unless(cxxThisExpr())))),
+ has(PointerToStructTypeWithBinding))))
+ .bind("sizeof-pointer"),
this);
}
@@ -292,11 +306,17 @@ 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<Expr>("sizeof-pointer-to-aggregate")) {
- diag(E->getBeginLoc(),
- "suspicious usage of 'sizeof(A*)'; pointer to aggregate")
- << E->getSourceRange();
+ } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-pointer")) {
+ if (Result.Nodes.getNodeAs<Type>("struct-type")) {
+ diag(E->getBeginLoc(),
+ "suspicious usage of 'sizeof(A*)' on pointer-to-aggregate type; did "
+ "you mean 'sizeof(A)'?")
+ << E->getSourceRange();
+ } else {
+ diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
+ "that results in a pointer")
+ << E->getSourceRange();
+ }
} else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
"sizeof-compare-constant")) {
diag(E->getOperatorLoc(),
@@ -332,18 +352,23 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
" numerator is not a multiple of denominator")
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
} else if (NumTy && DenomTy && NumTy == DenomTy) {
+ // FIXME: This message is wrong, it should not refer to sizeof "pointer"
+ // usage (and by the way, it would be to clarify all the messages).
diag(E->getOperatorLoc(),
"suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'")
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
- } else if (PointedTy && DenomTy && PointedTy == DenomTy) {
- diag(E->getOperatorLoc(),
- "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'")
- << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
- } else if (NumTy && DenomTy && NumTy->isPointerType() &&
- DenomTy->isPointerType()) {
- diag(E->getOperatorLoc(),
- "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'")
- << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+ } else if (!WarnOnSizeOfPointer) {
+ // When 'WarnOnSizeOfPointer' is enabled, these messages become redundant:
+ if (PointedTy && DenomTy && PointedTy == DenomTy) {
+ diag(E->getOperatorLoc(),
+ "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'")
+ << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+ } else if (NumTy && DenomTy && NumTy->isPointerType() &&
+ DenomTy->isPointerType()) {
+ diag(E->getOperatorLoc(),
+ "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'")
+ << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+ }
}
} else if (const auto *E =
Result.Nodes.getNodeAs<Expr>("sizeof-sizeof-expr")) {
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
index 55becdd4ecdba..9ca17bc9e6f12 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
@@ -30,6 +30,7 @@ class SizeofExpressionCheck : public ClangTidyCheck {
const bool WarnOnSizeOfThis;
const bool WarnOnSizeOfCompareToConstant;
const bool WarnOnSizeOfPointerToAggregate;
+ const bool WarnOnSizeOfPointer;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 0c0c10605a830..2dc39d0ad74af 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -237,6 +237,12 @@ Changes in existing checks
<clang-tidy/checks/bugprone/optional-value-conversion>` check by eliminating
false positives resulting from use of optionals in unevaluated context.
+- Improved :doc:`bugprone-sizeof-expression
+ <clang-tidy/checks/bugprone/sizeof-expression>` check by eliminating some
+ false positives and adding a new (off-by-default) option
+ `WarnOnSizeOfPointer` that reports all ``sizeof(pointer)`` expressions
+ (except for a few that are idiomatic).
+
- Improved :doc:`bugprone-suspicious-include
<clang-tidy/checks/bugprone/suspicious-include>` check by replacing the local
options `HeaderFileExtensions` and `ImplementationFileExtensions` by the
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 c37df1706eb4e..ed5bb4fbb89ba 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
@@ -190,6 +190,15 @@ Options
.. option:: WarnOnSizeOfPointerToAggregate
- When `true`, the check will warn on an expression like
- ``sizeof(expr)`` where the expression is a pointer
- to aggregate. Default is `true`.
+ When `true`, the check will warn when the argument of ``sizeof`` is either a
+ pointer-to-aggregate type, an expression returning a pointer-to-aggregate
+ value or an expression that returns a pointer from an array-to-pointer
+ conversion (that may be implicit or explicit, for example ``array + 2`` or
+ ``(int *)array``). Default is `true`.
+
+.. option:: WarnOnSizeOfPointer
+
+ When `true`, the check will report all expressions where the argument of
+ ``sizeof`` is an expression that produces a pointer (except for a few
+ idiomatic expressions that are probably intentional and correct).
+ This detects occurrences of CWE 467. Default is `false`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
index 8c4feb8f86169..aef930f2c8fda 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
@@ -34,24 +34,24 @@ int Test5() {
int sum = 0;
sum += sizeof(&S);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(__typeof(&S));
sum += sizeof(&TS);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(__typeof(&TS));
sum += sizeof(STRKWD MyStruct*);
sum += sizeof(__typeof(STRKWD MyStruct*));
sum += sizeof(TypedefStruct*);
sum += sizeof(__typeof(TypedefStruct*));
sum += sizeof(PTTS);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(PMyStruct);
sum += sizeof(PS);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(PS2);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(&A10);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
#ifdef __cplusplus
MyStruct &rS = S;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp
new file mode 100644
index 0000000000000..bfb2ec3a9eb02
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp
@@ -0,0 +1,241 @@
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- -config="{CheckOptions: {bugprone-sizeof-expression.WarnOnSizeOfIntegerExpression: true, bugprone-sizeof-expression.WarnOnSizeOfPointer: true}}" --
+
+class C {
+ int size() { return sizeof(this); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct S { char a, b, c; };
+
+enum E { E_VALUE = 0 };
+enum class EC { VALUE = 0 };
+
+bool AsBool() { return false; }
+int AsInt() { return 0; }
+E AsEnum() { return E_VALUE; }
+EC AsEnumClass() { return EC::VALUE; }
+S AsStruct() { return {}; }
+
+struct M {
+ int AsInt() { return 0; }
+ E AsEnum() { return E_VALUE; }
+ S AsStruct() { return {}; }
+};
+
+int Test1(const char* ptr) {
+ int sum = 0;
+ sum += sizeof(LEN);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+ sum += sizeof(LEN + 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+ sum += sizeof(sum, LEN);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(..., ...)'
+ sum += sizeof(AsBool());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(AsInt());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(AsEnum());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(AsEnumClass());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(M{}.AsInt());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(M{}.AsEnum());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + LEN + sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + (LEN + sizeof(X)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + -sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + - + -sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(char) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ sum += sizeof(A) / sizeof(S);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(char) / sizeof(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(char) / sizeof(A);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(B[0]) / sizeof(A);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(ptr) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(ptr) / sizeof(ptr[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(ptr) / sizeof(char*);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(ptr) / sizeof(void*);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(ptr) / sizeof(const void volatile*);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(ptr) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(int) * sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ sum += sizeof(ptr) * sizeof(ptr[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ sum += sizeof(int) * (2 * sizeof(char));
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ sum += (2 * sizeof(char)) * sizeof(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ if (sizeof(A) < 0x100000) sum += 42;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: suspicious comparison of 'sizeof(expr)' to a constant
+ if (sizeof(A) <= 0xFFFFFFFEU) sum += 42;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: suspicious comparison of 'sizeof(expr)' to a constant
+ return sum;
+}
+
+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;
+
+ char *PChar;
+ int *PInt, **PPInt;
+ MyStruct **PPMyStruct;
+
+ int sum = 0;
+ sum += sizeof(&S.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(&kGlocalMyStruct.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(&kGlocalMyStructPtr->arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(S.arr + 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(+ S.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof((int*)S.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+
+ sum += sizeof(S.ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(kGlocalMyStruct.ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(kGlocalMyStructPtr->ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+
+ sum += sizeof(&kGlocalMyStruct);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(&S);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(MyStruct*);
+ sum += sizeof(PMyStruct);
+ sum += sizeof(PS);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(PS2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(&A10);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(PtrArray) / sizeof(PtrArray[1]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(A10) / sizeof(PtrArray[0]);
+ sum += sizeof(PC) / sizeof(PtrArray[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ // CHECK-MESSAGES: :[[@LINE-2]]:21: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ sum += sizeof(ArrayC) / sizeof(PtrArray[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+
+ sum += sizeof(PChar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(PInt);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(PPInt);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(PPMyStruct);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+
+ return sum;
+}
+
+void some_generic_function(const void *arg, int argsize);
+int *IntP, **IntPP;
+C *ClassP, **ClassPP;
+
+void GenericFunctionTest() {
+ // The `sizeof(pointer)` checks ignore situations where the pointer is
+ // produced by dereferencing a pointer-to-pointer, because this is unlikely
+ // to be an accident and can appear in legitimate code that tries to call
+ // a generic function which emulates dynamic typing within C.
+ some_generic_function(IntPP, sizeof(*IntPP));
+ some_generic_function(ClassPP, sizeof(*ClassPP));
+ // Using `...[0]` instead of the dereference operator is another common
+ // variant, which is also widespread in the idiomatic array-size calculation:
+ // `sizeof(array) / sizeof(array[0])`.
+ some_generic_function(IntPP, sizeof(IntPP[0]));
+ some_generic_function(ClassPP, sizeof(ClassPP[0]));
+ // FIXME: There is a third common pattern where the generic function is
+ // called with `&Variable` and `sizeof(Variable)`. Right now these are
+ // reported by the `sizeof(pointer)` checks, but this causes some false
+ // positives, so it would be good to create an exception for them.
+ some_generic_function(&IntPP, sizeof(IntP));
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ some_generic_function(&ClassPP, sizeof(ClassP));
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+}
+
+int ValidExpressions() {
+ 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);
+ sum += sizeof(int);
+ sum += sizeof(AsStruct());
+ sum += sizeof(M{}.AsStruct());
+ sum += sizeof(A[sizeof(A) / sizeof(int)]);
+ // Here the outer sizeof is reported, but the inner ones are accepted:
+ sum += sizeof(&A[sizeof(A) / sizeof(int)]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+ sum += sizeof(sizeof(0)); // Special case: sizeof size_t.
+ sum += sizeof(void*);
+ sum += sizeof(void const *);
+ sum += sizeof(void const *) / 4;
+ sum += sizeof(str);
+ sum += sizeof(str) / sizeof(char);
+ 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;
+}
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 003a02209c3d2..064f31cb08c6b 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
@@ -124,8 +124,6 @@ int Test1(const char* ptr) {
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
sum += sizeof(ptr) / sizeof(char);
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
- sum += sizeof(ptr) / sizeof(ptr[0]);
- // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
sum += sizeof(int) * sizeof(char);
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious 'sizeof' by 'sizeof' multiplication
sum += sizeof(ptr) * sizeof(ptr[0]);
@@ -207,50 +205,57 @@ int Test5() {
C *PtrArray[10];
C *PC;
+ char *PChar;
+ int *PInt, **PPInt;
+ MyStruct **PPMyStruct;
+
int sum = 0;
sum += sizeof(&S.arr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(&kGlocalMyStruct.arr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(&kGlocalMyStructPtr->arr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(S.arr + 0);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(+ S.arr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof((int*)S.arr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(S.ptr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(kGlocalMyStruct.ptr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(kGlocalMyStructPtr->ptr);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(&kGlocalMyStruct);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(&S);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(MyStruct*);
sum += sizeof(PMyStruct);
sum += sizeof(PS);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(PS2);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(&A10);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
sum += sizeof(PtrArray) / sizeof(PtrArray[1]);
- // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
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-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
// CHECK-MESSAGES: :[[@LINE-2]]:21: 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]]:25: 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
+
+ // These pointers do not point to aggregate types, so they are not reported in this mode:
+ sum += sizeof(PChar);
+ sum += sizeof(PInt);
+ sum += sizeof(PPInt);
+ sum += sizeof(PPMyStruct);
return sum;
}
@@ -293,6 +298,32 @@ bool Baz() { return sizeof(A) < N; }
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: suspicious comparison of 'sizeof(expr)' to a constant
bool Test7() { return Baz<-1>(); }
+void some_generic_function(const void *arg, int argsize);
+int *IntP, **IntPP;
+C *ClassP, **ClassPP;
+
+void GenericFunctionTest() {
+ // The `sizeof(pointer)` checks ignore situations where the pointer is
+ // produced by dereferencing a pointer-to-pointer, because this is unlikely
+ // to be an accident and can appear in legitimate code that tries to call
+ // a generic function which emulates dynamic typing within C.
+ some_generic_function(IntPP, sizeof(*IntPP));
+ some_generic_function(ClassPP, sizeof(*ClassPP));
+ // Using `...[0]` instead of the dereference operator is another common
+ // variant, which is also widespread in the idiomatic array-size calculation:
+ // `sizeof(array) / sizeof(array[0])`.
+ some_generic_function(IntPP, sizeof(IntPP[0]));
+ some_generic_function(ClassPP, sizeof(ClassPP[0]));
+ // FIXME: There is a third common pattern where the generic function is
+ // called with `&Variable` and `sizeof(Variable)`. Right now these are
+ // reported by the `sizeof(pointer)` checks, but this causes some false
+ // positives, so it would be good to create an exception for them.
+ // NOTE: `sizeof(IntP)` is only reported with `WarnOnSizeOfPointer=true`.
+ some_generic_function(&IntPP, sizeof(IntP));
+ some_generic_function(&ClassPP, sizeof(ClassP));
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
+}
+
int ValidExpressions() {
int A[] = {1, 2, 3, 4};
static const char str[] = "hello";
More information about the cfe-commits
mailing list