[clang-tools-extra] r360032 - [clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

Adam Balogh via cfe-commits cfe-commits at lists.llvm.org
Mon May 6 03:41:37 PDT 2019


Author: baloghadamsoftware
Date: Mon May  6 03:41:37 2019
New Revision: 360032

URL: http://llvm.org/viewvc/llvm-project?rev=360032&view=rev
Log:
[clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

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.

Differential Revision: https://reviews.llvm.org/D61422


Modified:
    clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp?rev=360032&r1=360031&r2=360032&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp Mon May  6 03:41:37 2019
@@ -84,8 +84,11 @@ void SizeofExpressionCheck::registerMatc
   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 @@ void SizeofExpressionCheck::registerMatc
                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(
+          hasUnqualifiedDesugaredType(type().bind("left-ptr-type")))))))),
+      hasRHS(expr(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
+          hasUnqualifiedDesugaredType(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 @@ void SizeofExpressionCheck::check(const
   } 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");
+    }
   }
 }
 

Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp?rev=360032&r1=360031&r2=360032&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp Mon May  6 03:41:37 2019
@@ -231,6 +231,35 @@ int Test5() {
   return sum;
 }
 
+int Test6() {
+  int sum = 0;
+
+  struct S A = AsStruct(), B = AsStruct();
+  struct S *P = &A, *Q = &B;
+  sum += sizeof(struct S) == P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(S) != P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += sizeof(S) < P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(S) <= P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(*P) >= P - Q;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += Q - P > 3 * sizeof(*P);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += sizeof(S) + (P - Q);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += 5 * sizeof(S) - (P - Q);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += (P - Q) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic
+  sum += (P - Q) / sizeof(*Q);
+  // 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";




More information about the cfe-commits mailing list