[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp, xazax.hun, whisperity.
Herald added a project: clang.
balazske requested review of this revision.

Do not warn for "pointer to aggregate" in a `sizeof(A) / sizeof(A[0])`
expression if `A` is an array of pointers. This is the usual way of
calculating the array length even if the array is of pointers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91543

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
@@ -276,6 +276,10 @@
   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);
@@ -293,5 +297,7 @@
   sum += sizeof(str) / sizeof(str[0]);
   sum += sizeof(ptr) / sizeof(ptr[0]);
   sum += sizeof(ptr) / sizeof(*(ptr));
+  sum += sizeof(PtrArray) / sizeof(PtrArray[0]);
+  sum += sizeof(PtrArray) / sizeof(PtrArray1[0]);
   return sum;
 }
Index: clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -132,6 +132,7 @@
                      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.
   const auto ArrayExpr = expr(ignoringParenImpCasts(
       expr(hasType(qualType(hasCanonicalType(arrayType()))))));
   const auto ArrayCastExpr = expr(anyOf(
@@ -151,13 +152,31 @@
       hasType(qualType(hasCanonicalType(PointerToStructType))),
       unless(cxxThisExpr()))));
 
-  Finder->addMatcher(
-      expr(anyOf(sizeOfExpr(has(expr(ignoringParenImpCasts(
-               anyOf(ArrayCastExpr, PointerToArrayExpr, StructAddrOfExpr,
-                     PointerToStructExpr))))),
-                            sizeOfExpr(has(PointerToStructType))))
-          .bind("sizeof-pointer-to-aggregate"),
-      this);
+  const auto ArrayOfPointersExpr = expr(ignoringParenImpCasts(expr(hasType(
+      qualType(hasCanonicalType(arrayType(hasElementType(pointerType()))
+                                    .bind("type-of-array-of-pointers")))))));
+  const auto ArrayOfSamePointersExpr =
+      expr(ignoringParenImpCasts(expr(hasType(qualType(hasCanonicalType(
+          arrayType(equalsBoundNode("type-of-array-of-pointers"))))))));
+  const auto ZeroLiteral =
+      expr(ignoringParenImpCasts(integerLiteral(equals(0))));
+  const auto ArrayOfSamePointersZeroSubscriptExpr =
+      expr(ignoringParenImpCasts(arraySubscriptExpr(
+          hasBase(ArrayOfSamePointersExpr), hasIndex(ZeroLiteral))));
+  const auto ArrayLengthExprDenom =
+      expr(hasParent(expr(ignoringParenImpCasts(
+               binaryOperator(hasOperatorName("/"),
+                              hasLHS(expr(ignoringParenImpCasts(expr(
+                                  sizeOfExpr(has(ArrayOfPointersExpr)))))))))),
+           sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
+
+  Finder->addMatcher(expr(anyOf(sizeOfExpr(has(expr(ignoringParenImpCasts(anyOf(
+                                    ArrayCastExpr, PointerToArrayExpr,
+                                    StructAddrOfExpr, PointerToStructExpr))))),
+                                sizeOfExpr(has(PointerToStructType))),
+                          unless(ArrayLengthExprDenom))
+                         .bind("sizeof-pointer-to-aggregate"),
+                     this);
 
   // Detect expression like: sizeof(epxr) <= k for a suspicious constant 'k'.
   if (WarnOnSizeOfCompareToConstant) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91543.305512.patch
Type: text/x-patch
Size: 3557 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201116/20d126ab/attachment.bin>