baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: alexfh, aaron.ballman, lebedev.ri.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: gamesh411, Szelethus, rnkovacs.
Herald added a project: clang.
Some programmers tend to forget that subtracting two pointers results in the
difference between them in number of elements of the pointee type instead of
bytes. This leads to codes such as `size_t size = (p - q) / sizeof(int)` where
`p` and `q` are of type `int*`. Or similarily, `if (p - q < buffer_size *
sizeof(int)) { ... }`. This patch extends `bugprone-sizeof-expression` to
detect such cases.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D61422
Files:
clang-tidy/bugprone/SizeofExpressionCheck.cpp
test/clang-tidy/bugprone-sizeof-expression.cpp
Index: test/clang-tidy/bugprone-sizeof-expression.cpp
===================================================================
--- test/clang-tidy/bugprone-sizeof-expression.cpp
+++ test/clang-tidy/bugprone-sizeof-expression.cpp
@@ -231,6 +231,29 @@
return sum;
}
+int Test6() {
+ int sum = 0;
+
+ int N = AsInt(), M = AsInt();
+ int *P = &N, *Q = &M;
+ sum += sizeof(int) == P - Q;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+ sum += 5 * sizeof(int) != P - Q;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+ sum += sizeof(int) < P - Q;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+ sum += 5 * sizeof(int) <= P - Q;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+ sum += sizeof(int) + (P - Q);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+ sum += 5 * sizeof(int) - (P - Q);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+ sum += (P - Q) / sizeof(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+
+ return sum;
+}
+
int ValidExpressions() {
int A[] = {1, 2, 3, 4};
static const char str[] = "hello";
Index: clang-tidy/bugprone/SizeofExpressionCheck.cpp
===================================================================
--- clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -84,8 +84,11 @@
const auto IntegerCallExpr = expr(ignoringParenImpCasts(
callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
unless(isInTemplateInstantiation()))));
- const auto SizeOfExpr =
- expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr()))));
+ const auto SizeOfExpr = expr(anyOf(
+ sizeOfExpr(
+ has(hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type")))),
+ sizeOfExpr(has(expr(hasType(
+ hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type"))))))));
const auto SizeOfZero = expr(
sizeOfExpr(has(ignoringParenImpCasts(expr(integerLiteral(equals(0)))))));
@@ -209,6 +212,36 @@
hasSizeOfDescendant(8, expr(SizeOfExpr, unless(SizeOfZero))))))))
.bind("sizeof-sizeof-expr"),
this);
+
+ // Detect sizeof in pointer aritmetic like: N * sizeof(S) == P1 - P2 or
+ // (P1 - P2) / sizeof(S) where P1 and P2 are pointers to type S
+ const auto PtrDiffExpr = binaryOperator(
+ hasOperatorName("-"),
+ hasLHS(expr(hasType(hasUnqualifiedDesugaredType(
+ pointerType(pointee(type().bind("left-ptr-type"))))))),
+ hasRHS(expr(hasType(hasUnqualifiedDesugaredType(
+ pointerType(pointee(type().bind("right-ptr-type"))))))));
+
+ Finder->addMatcher(
+ binaryOperator(
+ anyOf(hasOperatorName("=="), hasOperatorName("!="),
+ hasOperatorName("<"), hasOperatorName("<="),
+ hasOperatorName(">"), hasOperatorName(">="),
+ hasOperatorName("+"), hasOperatorName("-")),
+ hasEitherOperand(expr(anyOf(
+ ignoringParenImpCasts(SizeOfExpr),
+ ignoringParenImpCasts(binaryOperator(
+ hasOperatorName("*"),
+ hasEitherOperand(ignoringParenImpCasts(SizeOfExpr))))))),
+ hasEitherOperand(ignoringParenImpCasts(PtrDiffExpr)))
+ .bind("sizeof-in-ptr-arithmetic-mul"),
+ this);
+
+ Finder->addMatcher(binaryOperator(hasOperatorName("/"),
+ hasLHS(ignoringParenImpCasts(PtrDiffExpr)),
+ hasRHS(ignoringParenImpCasts(SizeOfExpr)))
+ .bind("sizeof-in-ptr-arithmetic-div"),
+ this);
}
void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
@@ -275,6 +308,26 @@
} else if (const auto *E =
Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
diag(E->getBeginLoc(), "suspicious 'sizeof' by 'sizeof' multiplication");
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-mul")) {
+ const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
+ const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
+ const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
+
+ if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
+ diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
+ "pointer arithmetic");
+ }
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-div")) {
+ const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
+ const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
+ const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
+
+ if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
+ diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
+ "pointer arithmetic");
+ }
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
