https://github.com/whisperity updated https://github.com/llvm/llvm-project/pull/106061
>From 0202caa773928bfc395b850f52191ab15afe0eb4 Mon Sep 17 00:00:00 2001 From: zporky <zpo...@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 <zpo...@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 <zpo...@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 <zpo...@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 <zpo...@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 <zpo...@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 <zpo...@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 <whisper...@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 <whisper...@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 <whisper...@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 <whisper...@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 ) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits