https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/94356
From c94feff726b48e7e3b5a46d5028cc5a6d0ac9beb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Wed, 22 May 2024 11:57:50 +0200
Subject: [PATCH 1/6] [clang-tidy] Add WarnOnSizeOfPointer mode to
bugprone-sizeof-expression
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.
I preserved the exception that the RHS of an expression that looks like
`sizeof(array) / sizeof(array[0])` is not reported; and I added another
exception which ensures that `sizeof(*pp)` is not reported when `pp` is
a pointer-to-pointer expression.
This second exception (which I also apply in the "old" on-by-default mode
`WarnOnSizeOfPointerToAggregate`) was present in the CSA checker
`alpha.core.SizeofPoionter` and I decided to copy it because it helped
to avoid several false positives on open-source code.
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.)
---
.../bugprone/SizeofExpressionCheck.cpp | 95 ++++---
.../bugprone/SizeofExpressionCheck.h | 1 +
.../checks/bugprone/sizeof-expression.rst | 5 +
.../checkers/bugprone/sizeof-expression-2.c | 12 +-
.../sizeof-expression-any-pointer.cpp | 231 ++++++++++++++++++
.../checkers/bugprone/sizeof-expression.cpp | 63 +++--
6 files changed, 351 insertions(+), 56 deletions(-)
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index 5e64d23874ec1..05ef666d4f01e 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, some idiomatic constructs are accepted:
+ // + the RHS of a 'sizeof(arr) / sizeof(arr[0])' expression;
+ // + 'sizeof(*pp)' where 'pp' a pointer-to-pointer value, because this is
+ // a natural solution when dynamical typing is emulated by passing
+ // arguments as `generic_function(..., (void *)pp, sizeof(*pp))`.
+ // 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,8 +164,16 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder
*Finder) {
const auto PointerToStructType =
hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
- const auto PointerToStructExpr = expr(
- hasType(hasCanonicalType(PointerToStructType)), unless(cxxThisExpr()));
+ 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 ArrayOfPointersExpr = ignoringParenImpCasts(
hasType(hasCanonicalType(arrayType(hasElementType(pointerType()))
@@ -167,14 +190,17 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder
*Finder) {
hasLHS(ignoringParenImpCasts(sizeOfExpr(
has(ArrayOfPointersExpr)))))),
sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
+ const auto DerefExpr =
+ ignoringParenImpCasts(unaryOperator(hasOperatorName("*")));
Finder->addMatcher(
- expr(sizeOfExpr(anyOf(
- has(ignoringParenImpCasts(anyOf(
- ArrayCastExpr, PointerToArrayExpr, PointerToStructExpr))),
- has(PointerToStructType))),
+ expr(sizeOfExpr(anyOf(has(ignoringParenImpCasts(
+ expr(PointerToDetectedExpr,
unless(DerefExpr),
+ unless(VarWithConstStrLiteralDecl),
+ unless(cxxThisExpr())))),
+ has(PointerToStructTypeWithBinding))),
unless(ArrayLengthExprDenom))
- .bind("sizeof-pointer-to-aggregate"),
+ .bind("sizeof-pointer"),
this);
}
@@ -292,11 +318,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 +364,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/clang-tidy/checks/bugprone/sizeof-expression.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
index c37df1706eb4e..8ddee3254c090 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
@@ -193,3 +193,8 @@ Options
When `true`, the check will warn on an expression like
``sizeof(expr)`` where the expression is a pointer
to aggregate. Default is `true`.
+
+.. option:: WarnOnSizeOfPointer
+
+ When `true`, the check will warn on an expression like ``sizeof(expr)``
+ where the expression is a pointer. 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..98ad2b385de7d
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp
@@ -0,0 +1,231 @@
+// 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]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
+ 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)'
+ // CHECK-MESSAGES: :[[@LINE-3]]:23: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
+ 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()'
on an expression that results in a pointer
+
+ 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 **IntPP;
+C **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));
+}
+
+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..2cb4d9ee52542 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,60 @@ 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
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
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
+ // CHECK-MESSAGES: :[[@LINE-3]]:23: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
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
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
+
+ // 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 +301,19 @@ 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 **IntPP;
+C **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));
+}
+
int ValidExpressions() {
int A[] = {1, 2, 3, 4};
static const char str[] = "hello";
From 4de254de01d844b5a2b827946ac9073151a8d964 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 6 Jun 2024 16:17:33 +0200
Subject: [PATCH 2/6] Update the documentation and release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++
.../checks/bugprone/sizeof-expression.rst | 14 +++++++++-----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 33b65caf2b02c..e21d056930ada 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -227,6 +227,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 8ddee3254c090..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,11 +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 warn on an expression like ``sizeof(expr)``
- where the expression is a pointer. Default is `false`.
+ 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`.
From b3a7f8b7ae48360341ca8f72c1d38352b82dcc93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 6 Jun 2024 16:22:10 +0200
Subject: [PATCH 3/6] Use single back-ticks for option name
Co-authored-by: EugeneZelenko <eugene.zele...@gmail.com>
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e21d056930ada..61e2b55a831e4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -230,7 +230,7 @@ Changes in existing checks
- 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
+ `WarnOnSizeOfPointer` that reports all ``sizeof(pointer)`` expressions
(except for a few that are idiomatic).
- Improved :doc:`bugprone-suspicious-include
From ce3a37610228ceb9a9e60575f87886bf8e183493 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Fri, 7 Jun 2024 20:20:17 +0200
Subject: [PATCH 4/6] Generalize the suppression to all `sizeof(array[0])`
expressions
...instead of just the RHS of the `sizeof(array) / sizeof(array[0])`
expression. This simplifies the code *and* will suppress many false
positives in various open source projects.
---
.../bugprone/SizeofExpressionCheck.cpp | 31 ++++++-------------
.../sizeof-expression-any-pointer.cpp | 5 ++-
.../checkers/bugprone/sizeof-expression.cpp | 5 ++-
3 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index 05ef666d4f01e..1cb721d89fc9b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -145,11 +145,11 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder
*Finder) {
// than other sizeof(ptr) expressions because they can appear as distorted
// forms of the common sizeof(aggregate) expressions.)
//
- // To avoid false positives, some idiomatic constructs are accepted:
- // + the RHS of a 'sizeof(arr) / sizeof(arr[0])' expression;
- // + 'sizeof(*pp)' where 'pp' a pointer-to-pointer value, because this is
- // a natural solution when dynamical typing is emulated by passing
- // arguments as `generic_function(..., (void *)pp, sizeof(*pp))`.
+ // 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) {
@@ -175,31 +175,20 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder
*Finder) {
: expr(anyOf(ArrayCastExpr, PointerToArrayExpr,
PointerToStructExpr));
- 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(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(
expr(PointerToDetectedExpr,
unless(DerefExpr),
+ unless(SubscriptExprWithZeroIndex),
unless(VarWithConstStrLiteralDecl),
unless(cxxThisExpr())))),
- has(PointerToStructTypeWithBinding))),
- unless(ArrayLengthExprDenom))
+ has(PointerToStructTypeWithBinding))))
.bind("sizeof-pointer"),
this);
}
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
index 98ad2b385de7d..48fe3de9951a9 100644
---
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
@@ -161,14 +161,11 @@ int Test5() {
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]);
- // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
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)'
- // CHECK-MESSAGES: :[[@LINE-3]]:23: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
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()'
on an expression that results in a pointer
sum += sizeof(PChar);
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
@@ -193,6 +190,8 @@ void GenericFunctionTest() {
// a generic function which emulates dynamic typing within C.
some_generic_function(IntPP, sizeof(*IntPP));
some_generic_function(ClassPP, sizeof(*ClassPP));
+ some_generic_function(IntPP, sizeof(IntPP[0]));
+ some_generic_function(ClassPP, sizeof(ClassPP[0]));
}
int ValidExpressions() {
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 2cb4d9ee52542..bda87b2c12537 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
@@ -245,14 +245,11 @@ int Test5() {
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]);
- // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
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)'
- // CHECK-MESSAGES: :[[@LINE-3]]:23: warning: suspicious usage of 'sizeof()'
on an expression that results in a pointer
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()'
on an expression that results in a pointer
// These pointers do not point to aggregate types, so they are not reported
in this mode:
sum += sizeof(PChar);
@@ -312,6 +309,8 @@ void GenericFunctionTest() {
// a generic function which emulates dynamic typing within C.
some_generic_function(IntPP, sizeof(*IntPP));
some_generic_function(ClassPP, sizeof(*ClassPP));
+ some_generic_function(IntPP, sizeof(IntPP[0]));
+ some_generic_function(ClassPP, sizeof(ClassPP[0]));
}
int ValidExpressions() {
From ae8123ad4a82339e8c1b0f3c9870a2392c5e0458 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Fri, 7 Jun 2024 20:22:47 +0200
Subject: [PATCH 5/6] Satisfy git-clang-format
---
.../clang-tidy/bugprone/SizeofExpressionCheck.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index 1cb721d89fc9b..c25ee42d0899a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -177,8 +177,7 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder
*Finder) {
const auto ZeroLiteral = ignoringParenImpCasts(integerLiteral(equals(0)));
const auto SubscriptExprWithZeroIndex =
- arraySubscriptExpr(
- hasIndex(ZeroLiteral));
+ arraySubscriptExpr(hasIndex(ZeroLiteral));
const auto DerefExpr =
ignoringParenImpCasts(unaryOperator(hasOperatorName("*")));
From 59bd1b83e4fa89b371f4d1a96c51fc7a1b4ad170 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Mon, 10 Jun 2024 10:03:50 +0200
Subject: [PATCH 6/6] Extend `GenericFunctionTest`, document a class of FPs
with a FIXME
---
.../bugprone/sizeof-expression-any-pointer.cpp | 15 +++++++++++++--
.../checkers/bugprone/sizeof-expression.cpp | 15 +++++++++++++--
2 files changed, 26 insertions(+), 4 deletions(-)
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
index 48fe3de9951a9..bfb2ec3a9eb02 100644
---
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
@@ -180,8 +180,8 @@ int Test5() {
}
void some_generic_function(const void *arg, int argsize);
-int **IntPP;
-C **ClassPP;
+int *IntP, **IntPP;
+C *ClassP, **ClassPP;
void GenericFunctionTest() {
// The `sizeof(pointer)` checks ignore situations where the pointer is
@@ -190,8 +190,19 @@ void GenericFunctionTest() {
// 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() {
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 bda87b2c12537..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
@@ -299,8 +299,8 @@ bool Baz() { return sizeof(A) < N; }
bool Test7() { return Baz<-1>(); }
void some_generic_function(const void *arg, int argsize);
-int **IntPP;
-C **ClassPP;
+int *IntP, **IntPP;
+C *ClassP, **ClassPP;
void GenericFunctionTest() {
// The `sizeof(pointer)` checks ignore situations where the pointer is
@@ -309,8 +309,19 @@ void GenericFunctionTest() {
// 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() {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
