[clang-tools-extra] [wip][clang-tidy] Add new `bugprone-suspicious-pointer-arithmetics-using-sizeof` (`cert-arr39-c`) check (PR #106061)

via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 26 08:51:20 PDT 2024


https://github.com/whisperity updated https://github.com/llvm/llvm-project/pull/106061

>From 0202caa773928bfc395b850f52191ab15afe0eb4 Mon Sep 17 00:00:00 2001
From: zporky <zporky at gmail.com>
Date: Thu, 26 Oct 2023 20:40:39 +0200
Subject: [PATCH 01/11] Initial pointer arithmetics using sizeof matcher

---
 .../bugprone/BugproneTidyModule.cpp           |  3 ++
 .../clang-tidy/bugprone/CMakeLists.txt        |  1 +
 ...iousPointerArithmeticsUsingSizeofCheck.cpp | 36 +++++++++++++++++++
 ...iciousPointerArithmeticsUsingSizeofCheck.h | 30 ++++++++++++++++
 .../clang-tidy/cert/CERTTidyModule.cpp        |  4 +++
 5 files changed, 74 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 543c522899d7a5..4f74fd9fd50543 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -67,6 +67,7 @@
 #include "SuspiciousMemoryComparisonCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
+#include "SuspiciousPointerArithmeticsUsingSizeofCheck.h"
 #include "SuspiciousReallocUsageCheck.h"
 #include "SuspiciousSemicolonCheck.h"
 #include "SuspiciousStringCompareCheck.h"
@@ -204,6 +205,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-suspicious-memset-usage");
     CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(
         "bugprone-suspicious-missing-comma");
+    CheckFactories.registerCheck<SuspiciousPointerArithmeticsUsingSizeofCheck>(
+        "bugprone-suspicious-pointer-arithmetics-using-sizeof");
     CheckFactories.registerCheck<SuspiciousReallocUsageCheck>(
         "bugprone-suspicious-realloc-usage");
     CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 0df9e439b715e5..c9877e6f5a7ddc 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -64,6 +64,7 @@ add_clang_library(clangTidyBugproneModule
   SuspiciousMemoryComparisonCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   SuspiciousMissingCommaCheck.cpp
+  SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
   SuspiciousReallocUsageCheck.cpp
   SuspiciousSemicolonCheck.cpp
   SuspiciousStringCompareCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
new file mode 100644
index 00000000000000..abfa1bc7f037ea
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
@@ -0,0 +1,36 @@
+//===--- SuspiciousPointerArithmeticsUsingSizeofCheck.cpp - clang-tidy --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuspiciousPointerArithmeticsUsingSizeofCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+SuspiciousPointerArithmeticsUsingSizeofCheck::SuspiciousPointerArithmeticsUsingSizeofCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {
+}
+
+void SuspiciousPointerArithmeticsUsingSizeofCheck::registerMatchers(MatchFinder *Finder) {
+    Finder->addMatcher(
+		   sizeOfExpr(expr())
+            .bind("sizeof-expression"),
+        this);
+}
+
+void SuspiciousPointerArithmeticsUsingSizeofCheck::check(
+    const MatchFinder::MatchResult &Result) {
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h
new file mode 100644
index 00000000000000..d39e15f0ccc3a0
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h
@@ -0,0 +1,30 @@
+//===--- SuspiciousPointerArithmeticsUsingSizeofCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSSIZEOFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSSIZEOFCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Find suspicious calls to string compare functions.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-sizeof.html
+class SuspiciousPointerArithmeticsUsingSizeofCheck : public ClangTidyCheck {
+public:
+  SuspiciousPointerArithmeticsUsingSizeofCheck(StringRef Name, ClangTidyContext *Context);
+//  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSSIZEOFCHECK_H
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index d448d9ba614548..17a7e4bc51049d 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -15,6 +15,7 @@
 #include "../bugprone/SignedCharMisuseCheck.h"
 #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h"
 #include "../bugprone/SuspiciousMemoryComparisonCheck.h"
+#include "../bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h"
 #include "../bugprone/UnhandledSelfAssignmentCheck.h"
 #include "../bugprone/UnsafeFunctionsCheck.h"
 #include "../bugprone/UnusedReturnValueCheck.h"
@@ -278,6 +279,9 @@ class CERTModule : public ClangTidyModule {
         "cert-oop58-cpp");
 
     // C checkers
+    // ARR
+    CheckFactories.registerCheck<bugprone::SuspiciousPointerArithmeticsUsingSizeofCheck>(
+        "cert-arr39-c");		    
     // CON
     CheckFactories.registerCheck<bugprone::SpuriouslyWakeUpFunctionsCheck>(
         "cert-con36-c");

>From f2d7d25549101d3250e65dc3a76dac766ccfc476 Mon Sep 17 00:00:00 2001
From: zporky <zporky at gmail.com>
Date: Fri, 3 Nov 2023 01:15:17 +0100
Subject: [PATCH 02/11] Working version of the checker.

---
 ...iousPointerArithmeticsUsingSizeofCheck.cpp | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
index abfa1bc7f037ea..de7c7e62e99282 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
+#include <iostream>
 
 using namespace clang::ast_matchers;
 
@@ -24,13 +25,25 @@ SuspiciousPointerArithmeticsUsingSizeofCheck::SuspiciousPointerArithmeticsUsingS
 
 void SuspiciousPointerArithmeticsUsingSizeofCheck::registerMatchers(MatchFinder *Finder) {
     Finder->addMatcher(
-		   sizeOfExpr(expr())
-            .bind("sizeof-expression"),
+	     expr(anyOf(
+                    binaryOperator(hasAnyOperatorName("+","-"),
+                      hasEitherOperand(hasType(pointerType())),
+		      hasEitherOperand(sizeOfExpr(expr())),
+		      unless(allOf(hasLHS(hasType(pointerType())),
+				   hasRHS(hasType(pointerType()))))
+		      ).bind("ptr-sizeof-expression"),
+		    binaryOperator(hasAnyOperatorName("+=","-="),
+	              hasLHS(hasType(pointerType())),
+		      hasRHS(sizeOfExpr(expr()))
+		      ).bind("ptr-sizeof-expression")
+            )),
         this);
 }
 
-void SuspiciousPointerArithmeticsUsingSizeofCheck::check(
-    const MatchFinder::MatchResult &Result) {
+void SuspiciousPointerArithmeticsUsingSizeofCheck::check(const MatchFinder::MatchResult &Result) {
+    static const char *diag_msg	= "Suspicious pointer arithmetics using sizeof() operator";
+    auto Matched = Result.Nodes.getNodeAs<BinaryOperator>("ptr-sizeof-expression");
+    diag(Matched->getExprLoc(),diag_msg)<< Matched->getSourceRange();
 }
 
 } // namespace clang::tidy::bugprone

>From 330dc16f6b7db1a34596f03105c1560bf94fea07 Mon Sep 17 00:00:00 2001
From: zporky <zporky at gmail.com>
Date: Fri, 3 Nov 2023 21:23:26 +0100
Subject: [PATCH 03/11] Code simplification with elevating string constants.

---
 .../SuspiciousPointerArithmeticsUsingSizeofCheck.cpp   | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
index de7c7e62e99282..90a8d308596cf6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
@@ -12,10 +12,12 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
-#include <iostream>
 
 using namespace clang::ast_matchers;
 
+namespace {
+  static const char *bin_op_bind = "ptr-sizeof-expression";	
+}
 namespace clang::tidy::bugprone {
 
 SuspiciousPointerArithmeticsUsingSizeofCheck::SuspiciousPointerArithmeticsUsingSizeofCheck(
@@ -31,18 +33,18 @@ void SuspiciousPointerArithmeticsUsingSizeofCheck::registerMatchers(MatchFinder
 		      hasEitherOperand(sizeOfExpr(expr())),
 		      unless(allOf(hasLHS(hasType(pointerType())),
 				   hasRHS(hasType(pointerType()))))
-		      ).bind("ptr-sizeof-expression"),
+		      ).bind(bin_op_bind),
 		    binaryOperator(hasAnyOperatorName("+=","-="),
 	              hasLHS(hasType(pointerType())),
 		      hasRHS(sizeOfExpr(expr()))
-		      ).bind("ptr-sizeof-expression")
+		      ).bind(bin_op_bind)
             )),
         this);
 }
 
 void SuspiciousPointerArithmeticsUsingSizeofCheck::check(const MatchFinder::MatchResult &Result) {
     static const char *diag_msg	= "Suspicious pointer arithmetics using sizeof() operator";
-    auto Matched = Result.Nodes.getNodeAs<BinaryOperator>("ptr-sizeof-expression");
+    auto Matched = Result.Nodes.getNodeAs<BinaryOperator>(bin_op_bind);
     diag(Matched->getExprLoc(),diag_msg)<< Matched->getSourceRange();
 }
 

>From 27a2ac5e7e6168c43dd9fec14994176b7ed18ea0 Mon Sep 17 00:00:00 2001
From: zporky <zporky at gmail.com>
Date: Thu, 9 Nov 2023 20:02:38 +0100
Subject: [PATCH 04/11] filtering out char* like matches

---
 ...iousPointerArithmeticsUsingSizeofCheck.cpp | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
index 90a8d308596cf6..02275a2cdfe747 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
@@ -15,11 +15,13 @@
 
 using namespace clang::ast_matchers;
 
-namespace {
-  static const char *bin_op_bind = "ptr-sizeof-expression";	
-}
 namespace clang::tidy::bugprone {
 
+//static const char *bin_op_bind = "ptr-sizeof-expression";	
+static constexpr llvm::StringLiteral BinOp{"bin-op"};
+static const auto IgnoredType = qualType(anyOf(asString("char"),asString("unsigned char"),asString("signed char"),asString("int8_t"),asString("uint8_t"),asString("std::byte"),asString("const char"),asString("const unsigned char"),asString("const signed char"),asString("const int8_t"),asString("const uint8_t"),asString("const std::byte")));
+static const auto InterestingPointer = pointerType(unless(pointee(IgnoredType)));
+
 SuspiciousPointerArithmeticsUsingSizeofCheck::SuspiciousPointerArithmeticsUsingSizeofCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context) {
@@ -28,7 +30,7 @@ SuspiciousPointerArithmeticsUsingSizeofCheck::SuspiciousPointerArithmeticsUsingS
 void SuspiciousPointerArithmeticsUsingSizeofCheck::registerMatchers(MatchFinder *Finder) {
     Finder->addMatcher(
 	     expr(anyOf(
-                    binaryOperator(hasAnyOperatorName("+","-"),
+/*                    binaryOperator(hasAnyOperatorName("+","-"),
                       hasEitherOperand(hasType(pointerType())),
 		      hasEitherOperand(sizeOfExpr(expr())),
 		      unless(allOf(hasLHS(hasType(pointerType())),
@@ -38,13 +40,20 @@ void SuspiciousPointerArithmeticsUsingSizeofCheck::registerMatchers(MatchFinder
 	              hasLHS(hasType(pointerType())),
 		      hasRHS(sizeOfExpr(expr()))
 		      ).bind(bin_op_bind)
+*/
+		    binaryOperator(hasAnyOperatorName("+=","-=","+","-" ),
+	              hasLHS(hasType(InterestingPointer)),
+		      hasRHS(sizeOfExpr(expr()))).bind(BinOp),
+		    binaryOperator(hasAnyOperatorName("+","-" ),
+	              hasRHS(hasType(InterestingPointer)),
+		      hasLHS(sizeOfExpr(expr()))).bind(BinOp)
             )),
         this);
 }
 
 void SuspiciousPointerArithmeticsUsingSizeofCheck::check(const MatchFinder::MatchResult &Result) {
     static const char *diag_msg	= "Suspicious pointer arithmetics using sizeof() operator";
-    auto Matched = Result.Nodes.getNodeAs<BinaryOperator>(bin_op_bind);
+    auto Matched = Result.Nodes.getNodeAs<BinaryOperator>(BinOp);
     diag(Matched->getExprLoc(),diag_msg)<< Matched->getSourceRange();
 }
 

>From 3ed1e4fd39e9efd88e509637a345118a9a7b3300 Mon Sep 17 00:00:00 2001
From: zporky <zporky at gmail.com>
Date: Wed, 15 Nov 2023 23:03:45 +0100
Subject: [PATCH 05/11] Filtering out the uninteresting types in imperative
 way.

---
 ...iousPointerArithmeticsUsingSizeofCheck.cpp | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
index 02275a2cdfe747..91f5ac55c508b6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
@@ -19,6 +19,7 @@ namespace clang::tidy::bugprone {
 
 //static const char *bin_op_bind = "ptr-sizeof-expression";	
 static constexpr llvm::StringLiteral BinOp{"bin-op"};
+static constexpr llvm::StringLiteral PointedType{"pointed-type"};
 static const auto IgnoredType = qualType(anyOf(asString("char"),asString("unsigned char"),asString("signed char"),asString("int8_t"),asString("uint8_t"),asString("std::byte"),asString("const char"),asString("const unsigned char"),asString("const signed char"),asString("const int8_t"),asString("const uint8_t"),asString("const std::byte")));
 static const auto InterestingPointer = pointerType(unless(pointee(IgnoredType)));
 
@@ -40,21 +41,43 @@ void SuspiciousPointerArithmeticsUsingSizeofCheck::registerMatchers(MatchFinder
 	              hasLHS(hasType(pointerType())),
 		      hasRHS(sizeOfExpr(expr()))
 		      ).bind(bin_op_bind)
-*/
+
 		    binaryOperator(hasAnyOperatorName("+=","-=","+","-" ),
 	              hasLHS(hasType(InterestingPointer)),
 		      hasRHS(sizeOfExpr(expr()))).bind(BinOp),
 		    binaryOperator(hasAnyOperatorName("+","-" ),
 	              hasRHS(hasType(InterestingPointer)),
 		      hasLHS(sizeOfExpr(expr()))).bind(BinOp)
+*/		    
+		    binaryOperator(hasAnyOperatorName("+=","-=","+","-" ),
+	              hasLHS(hasType(pointerType(pointee(qualType().bind(PointedType))))),
+		      hasRHS(sizeOfExpr(expr()))).bind(BinOp),
+		    binaryOperator(hasAnyOperatorName("+","-" ),
+	              hasRHS(hasType(pointerType(pointee(qualType().bind(PointedType))))),
+		      hasLHS(sizeOfExpr(expr()))).bind(BinOp)
             )),
         this);
 }
 
+static CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
+  if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() ||
+      isa<DependentSizedArrayType>(Ty) || !Ty->isConstantSizeType())
+    return CharUnits::Zero();
+  return Ctx.getTypeSizeInChars(Ty);
+}
+
 void SuspiciousPointerArithmeticsUsingSizeofCheck::check(const MatchFinder::MatchResult &Result) {
     static const char *diag_msg	= "Suspicious pointer arithmetics using sizeof() operator";
-    auto Matched = Result.Nodes.getNodeAs<BinaryOperator>(BinOp);
-    diag(Matched->getExprLoc(),diag_msg)<< Matched->getSourceRange();
+    const ASTContext &Ctx = *Result.Context;
+    const auto Matched = Result.Nodes.getNodeAs<BinaryOperator>(BinOp);
+    const auto SuspiciousQualTypePtr = Result.Nodes.getNodeAs<QualType>(PointedType);
+    const auto SuspiciousTypePtr = SuspiciousQualTypePtr->getTypePtr();
+
+    auto sz = getSizeOfType(Ctx,SuspiciousTypePtr).getQuantity();
+    if ( sz > 1 )
+    {
+        diag(Matched->getExprLoc(),diag_msg)<< Matched->getSourceRange();
+    }
 }
 
 } // namespace clang::tidy::bugprone

>From 7e3b35dbf09d99fba55d42d9c4c5c571dd4703a6 Mon Sep 17 00:00:00 2001
From: zporky <zporky at gmail.com>
Date: Thu, 16 Nov 2023 20:35:03 +0100
Subject: [PATCH 06/11] Reporting the suspicious type and its size.

---
 ...SuspiciousPointerArithmeticsUsingSizeofCheck.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
index 91f5ac55c508b6..a1710916f762ff 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
@@ -67,16 +67,17 @@ static CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
 }
 
 void SuspiciousPointerArithmeticsUsingSizeofCheck::check(const MatchFinder::MatchResult &Result) {
-    static const char *diag_msg	= "Suspicious pointer arithmetics using sizeof() operator";
     const ASTContext &Ctx = *Result.Context;
-    const auto Matched = Result.Nodes.getNodeAs<BinaryOperator>(BinOp);
-    const auto SuspiciousQualTypePtr = Result.Nodes.getNodeAs<QualType>(PointedType);
-    const auto SuspiciousTypePtr = SuspiciousQualTypePtr->getTypePtr();
+    const BinaryOperator* Matched = Result.Nodes.getNodeAs<BinaryOperator>(BinOp);
+    const QualType* SuspiciousQualTypePtr = Result.Nodes.getNodeAs<QualType>(PointedType);
+    const Type* SuspiciousTypePtr = SuspiciousQualTypePtr->getTypePtr();
 
-    auto sz = getSizeOfType(Ctx,SuspiciousTypePtr).getQuantity();
+    std::size_t sz = getSizeOfType(Ctx,SuspiciousTypePtr).getQuantity();
     if ( sz > 1 )
     {
-        diag(Matched->getExprLoc(),diag_msg)<< Matched->getSourceRange();
+        diag(Matched->getExprLoc(),"Suspicious pointer arithmetics using sizeof() operator: sizeof(%0) is %1") << SuspiciousQualTypePtr->getAsString(Ctx.getPrintingPolicy())
+	                       << sz
+		               << Matched->getSourceRange();
     }
 }
 

>From fbbc8d2e75472ad8cdfdaef7f97083994f1ad3ef Mon Sep 17 00:00:00 2001
From: zporky <zporky at gmail.com>
Date: Fri, 5 Apr 2024 16:00:00 +0200
Subject: [PATCH 07/11] add initial test file

---
 ...picious-pointer-arithmetics-using-sizeof.c | 209 ++++++++++++++++++
 1 file changed, 209 insertions(+)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-pointer-arithmetics-using-sizeof.c

diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-pointer-arithmetics-using-sizeof.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-pointer-arithmetics-using-sizeof.c
new file mode 100644
index 00000000000000..353cbcc9e683ae
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-pointer-arithmetics-using-sizeof.c
@@ -0,0 +1,209 @@
+
+struct mystruct {
+  long a;
+  long b;
+  long c;
+};
+void noncompliant_f1(void);
+void compliant_f1(void);
+void noncompliant_f3(struct mystruct *msptr); 
+void compliant_f3(struct mystruct *msptr);
+extern void sink(const char *);
+
+enum { bufsize = 1024 };
+
+void noncompliant_f1(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr; 
+  while ( ptr < bptr + sizeof(buffer) ) { // noncompliant
+    *ptr++ = 0;	// compliant  
+  }
+}
+void noncompliant_f1a(void) {
+  typedef int my_int_t;	
+  my_int_t buffer[bufsize]; 
+
+  my_int_t *bptr = &buffer[0]; 
+  my_int_t *ptr  = bptr; 
+  while ( ptr < bptr + sizeof(buffer) ) { // noncompliant
+    *ptr++ = 0;	// compliant  
+  }
+}
+void compliant_f1(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    *ptr++ = 0;	// compliant  
+  }
+}
+
+void noncompliant_f2(void) {
+  int buffer[bufsize];
+  int *ptr = buffer;
+
+  while ( ptr < buffer + sizeof(buffer) ) { // noncompliant
+    *ptr++ = 0;	// compliant
+  }
+}
+
+void compliant_f2(void) {
+  int buffer[bufsize];
+  int *ptr  = buffer; 
+
+  while ( ptr < buffer + bufsize ) { // compliant
+    *ptr++ = 0;	// compliant  
+  }
+}
+
+void memset2(void*, int, unsigned int);
+
+void noncompliant_f3(struct mystruct *msptr) {
+  const unsigned int skip = sizeof(long); // why offsetof is declared?
+  struct mystruct *ptr = msptr;
+  
+  memset2(ptr + skip, // noncompliant, impossible with tidy
+                     0, sizeof(struct mystruct) - skip);
+}
+
+void compliant_f3(struct mystruct *msptr) {
+  const unsigned int skip = sizeof(long);
+  char *ptr = (char*)msptr;
+
+  memset2(ptr + skip, // compliant
+                     0, sizeof(struct mystruct) - skip); 
+}
+
+void noncompliant_f4(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    *ptr = 0;	  
+    ptr += sizeof(*ptr); // noncompliant
+  }
+}
+void noncompliant_f4w(void) { /* accidentally good */
+  char buffer[bufsize]; 
+
+  char *bptr = &buffer[0]; 
+  char *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    *ptr = 0;	  
+    ptr += sizeof(*ptr);  // silenced
+  }
+}
+void compliant_f4w(void) {
+  char buffer[bufsize]; 
+
+  char *bptr = &buffer[0]; 
+  char *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    *ptr = 0;	  
+    ptr += 1;  // compliant
+  }
+}
+
+void noncompliant_f5(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    *ptr = 0;	  
+    ptr = ptr + sizeof(*ptr); // noncompliant
+  }
+}
+void noncompliant_f5w(void) {
+  char buffer[bufsize]; 
+
+  char*bptr = &buffer[0]; 
+  char *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    *ptr = 0;	  
+    ptr = ptr + sizeof(*ptr); // silenced
+  }
+}
+void noncompliant_f5c(void) {
+  char buffer[bufsize]; 
+
+  char*bptr = &buffer[0]; 
+  const char *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    sink(ptr);	  
+    ptr = ptr + sizeof(*ptr); // silenced
+  }
+}
+void compliant_f5c(void) {
+  char buffer[bufsize]; 
+
+  char *bptr = &buffer[0]; 
+  const char *ptr  = bptr; 
+  while ( ptr < bptr + bufsize ) { // compliant
+    sink(ptr);	  
+    ptr = ptr + 1;  // compliant
+  }
+}
+
+void noncompliant_f6(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr + bufsize; // compliant
+  while ( ptr >= bptr ) {
+    *ptr = 0;	  
+    ptr = ptr - sizeof(*ptr); // noncompliant
+  }
+}
+void noncompliant_f6w(void) {
+  char buffer[bufsize]; 
+
+  char *bptr = &buffer[0]; 
+  char *ptr  = bptr + bufsize; // compliant
+  while ( ptr >= bptr ) {
+    *ptr = 0;	  
+    ptr = ptr - sizeof(*ptr); // silenced
+  }
+}
+void compliant_f6(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr + bufsize; // compliant
+  while ( ptr >= bptr ) {
+    *ptr = 0;	  
+    ptr = ptr - 1; // compliant
+  }
+}
+
+void compliant_f7(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr + bufsize; // compliant
+  int i = ptr - bptr; // compliant
+  while ( i >= 0 ) {
+    ptr[i] = 0;	  
+    i = i - 1; // compliant
+  }
+}
+
+void compliant_f8(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr + bufsize; // compliant
+  int i = sizeof(*ptr) - sizeof(*bptr); // compliant
+}
+void compliant_f9(void) {
+  int buffer[bufsize]; 
+
+  int *bptr = &buffer[0]; 
+  int *ptr  = bptr + bufsize; // compliant
+  int i = sizeof(ptr) - sizeof(*bptr); // compliant
+}
+

>From 80c67e581ad2debed748d7142ed72a468936122a Mon Sep 17 00:00:00 2001
From: Whisperity <whisperity at gmail.com>
Date: Mon, 26 Aug 2024 16:38:22 +0200
Subject: [PATCH 08/11] doc: Add documentation for the check

---
 ...iciousPointerArithmeticsUsingSizeofCheck.h | 15 ++++---
 ...cious-pointer-arithmetics-using-sizeof.rst | 44 +++++++++++++++++++
 .../docs/clang-tidy/checks/cert/arr39-c.rst   | 10 +++++
 3 files changed, 62 insertions(+), 7 deletions(-)
 create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-using-sizeof.rst
 create mode 100644 clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h
index d39e15f0ccc3a0..1dbc51dd3df84f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.h
@@ -6,25 +6,26 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSSIZEOFCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSSIZEOFCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSUSINGSIZEOFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSUSINGSIZEOFCHECK_H
 
 #include "../ClangTidyCheck.h"
 
 namespace clang::tidy::bugprone {
 
-/// Find suspicious calls to string compare functions.
+/// Finds suspicious pointer arithmetic calculations where the pointer is
+/// offset by a sizeof() expression.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-sizeof.html
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-using-sizeof.html
 class SuspiciousPointerArithmeticsUsingSizeofCheck : public ClangTidyCheck {
 public:
-  SuspiciousPointerArithmeticsUsingSizeofCheck(StringRef Name, ClangTidyContext *Context);
-//  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  SuspiciousPointerArithmeticsUsingSizeofCheck(StringRef Name,
+                                               ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
 
 } // namespace clang::tidy::bugprone
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSSIZEOFCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSPOINTERARITHMETICSUSINGSIZEOFCHECK_H
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-using-sizeof.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-using-sizeof.rst
new file mode 100644
index 00000000000000..5188f947627ab0
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-using-sizeof.rst
@@ -0,0 +1,44 @@
+.. title:: clang-tidy - bugprone-suspicious-pointer-arithmetics-using-sizeof
+
+bugprone-suspicious-pointer-arithmetics-using-sizeof
+====================================================
+
+Finds suspicious pointer arithmetic calculations where the pointer is offset by a ``sizeof()`` expression.
+
+Pointer arithmetic expressions implicitly scale the offset added to or subtracted from the address by the size of the pointee type.
+Scaling the offset expression manually effectively results in a squared offset, which creates an invalid pointer that points beyond the end of the intended array.
+
+.. code-block:: c++
+
+  void printEveryEvenIndexElement(int *Array, size_t N) {
+    int *P = Array;
+    while (P <= Array + N * sizeof(int)) { // Suspicious pointer arithmetics using sizeof()!
+      printf("%d ", *P);
+
+      P += 2 * sizeof(int); // Suspicious pointer arithmetics using sizeof()!
+    }
+  }
+
+The above example should be in the following, correct form:
+
+.. code-block:: c++
+
+  void printEveryEvenIndexElement(int *Array, size_t N) {
+    int *P = Array;
+    while (P <= Array + N) {
+      printf("%d ", *P);
+
+      P += 2;
+    }
+  }
+
+`cert-arr39-c` redirects here as an alias of this check.
+
+This check corresponds to the CERT C Coding Standard rule
+`ARR39-C. Do not add or subtract a scaled integer to a pointer
+<http://wiki.sei.cmu.edu/confluence/display/c/ARR39-C.+Do+not+add+or+subtract+a+scaled+integer+to+a+pointer>`_.
+
+Limitations
+-----------
+
+While incorrect from a technically rigorous point of view, the check does not warn for pointer arithmetics where the pointee type is ``char`` (``sizeof(char) == 1``, by definition) on purpose.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst
new file mode 100644
index 00000000000000..91e02e39952be9
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-arr39-c
+.. meta::
+   :http-equiv=refresh: 5;URL=../bugprone/suspicious-pointer-arithmetics-using-sizeof.html
+
+cert-arr39-c
+============
+
+The `cert-arr39-c` check is an alias, please see
+:doc:`bugprone-suspicious-pointer-arithmetics-using-sizeof <../bugprone/suspicious-pointer-arithmetics-using-sizeof>`
+for more information.

>From be6c05aa90caa29b7e6a7b9aff5fd3e3c7082c73 Mon Sep 17 00:00:00 2001
From: Whisperity <whisperity at gmail.com>
Date: Mon, 26 Aug 2024 17:09:51 +0200
Subject: [PATCH 09/11] chore(doc): Added release notes entries

---
 clang-tools-extra/docs/ReleaseNotes.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1b025e8f90f7ba..ba2ffb6f8970ff 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,9 +98,19 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-suspicious-pointer-arithmetics-using-sizeof
+  <clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-using-sizeof>`
+  check that finds suspicious pointer arithmetic calculations where the pointer
+  is offset by a ``sizeof()`` expression.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
+- New alias :doc:`cert-arr39-c <clang-tidy/checks/cert/arr39-c>` to
+  :doc:`bugprone-suspicious-pointer-arithmetics-using-sizeof
+  <clang-tidy/checks/bugprone/suspicious-pointer-arithmetics-using-sizeof>`
+  was added.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 

>From cc5de412e09f44df8489a817edc8f219d71f6557 Mon Sep 17 00:00:00 2001
From: Whisperity <whisperity at gmail.com>
Date: Mon, 26 Aug 2024 17:11:50 +0200
Subject: [PATCH 10/11] style: Format out line break from `CERTModule`

---
 clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index 9e90f363367496..39ba55142da4bc 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -283,8 +283,9 @@ class CERTModule : public ClangTidyModule {
 
     // C checkers
     // ARR
-    CheckFactories.registerCheck<bugprone::SuspiciousPointerArithmeticsUsingSizeofCheck>(
-        "cert-arr39-c");		    
+    CheckFactories
+      .registerCheck<bugprone::SuspiciousPointerArithmeticsUsingSizeofCheck>(
+        "cert-arr39-c");
     // CON
     CheckFactories.registerCheck<bugprone::SpuriouslyWakeUpFunctionsCheck>(
         "cert-con36-c");

>From 590a7718fb39c918ca8182ab2782c2d235dc8c07 Mon Sep 17 00:00:00 2001
From: Whisperity <whisperity at gmail.com>
Date: Mon, 26 Aug 2024 17:50:16 +0200
Subject: [PATCH 11/11] style: Apply some formatting fixes

---
 .../SuspiciousPointerArithmeticsUsingSizeofCheck.cpp     | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
index a1710916f762ff..fc9c6eac38f91f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousPointerArithmeticsUsingSizeofCheck.cpp
@@ -1,4 +1,4 @@
-//===--- SuspiciousPointerArithmeticsUsingSizeofCheck.cpp - clang-tidy --===//
+//===--- SuspiciousPointerArithmeticsUsingSizeofCheck.cpp - clang-tidy ----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -68,9 +68,10 @@ static CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
 
 void SuspiciousPointerArithmeticsUsingSizeofCheck::check(const MatchFinder::MatchResult &Result) {
     const ASTContext &Ctx = *Result.Context;
-    const BinaryOperator* Matched = Result.Nodes.getNodeAs<BinaryOperator>(BinOp);
-    const QualType* SuspiciousQualTypePtr = Result.Nodes.getNodeAs<QualType>(PointedType);
-    const Type* SuspiciousTypePtr = SuspiciousQualTypePtr->getTypePtr();
+    const auto *Matched = Result.Nodes.getNodeAs<BinaryOperator>(BinOp);
+    const auto *SuspiciousQualTypePtr =
+        Result.Nodes.getNodeAs<QualType>(PointedType);
+    const auto *SuspiciousTypePtr = SuspiciousQualTypePtr->getTypePtr();
 
     std::size_t sz = getSizeOfType(Ctx,SuspiciousTypePtr).getQuantity();
     if ( sz > 1 )



More information about the cfe-commits mailing list