[clang-tools-extra] [clang-tidy] Add WarnOnSizeOfPointer mode to bugprone-sizeof-expression (PR #94356)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 6 07:22:18 PDT 2024
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.nagy at ericsson.com>
Date: Wed, 22 May 2024 11:57:50 +0200
Subject: [PATCH 1/3] [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.nagy at ericsson.com>
Date: Thu, 6 Jun 2024 16:17:33 +0200
Subject: [PATCH 2/3] 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.nagy at ericsson.com>
Date: Thu, 6 Jun 2024 16:22:10 +0200
Subject: [PATCH 3/3] Use single back-ticks for option name
Co-authored-by: EugeneZelenko <eugene.zelenko at 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
More information about the cfe-commits
mailing list