From c63522e6ba7782c335043893ae7cbd37eca24fe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <1.int32@gmail.com> Date: Mon, 7 Feb 2022 09:56:24 +0100 Subject: [PATCH] [clang-tidy] Add new check 'shared-ptr-array-mismatch'. Reviewed By: LegalizeAdulthood Differential Revision: https://reviews.llvm.org/D117306 --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 2 + .../bugprone/SharedPtrArrayMismatchCheck.cpp | 31 +++++ .../bugprone/SharedPtrArrayMismatchCheck.h | 38 ++++++ .../bugprone/SmartPtrArrayMismatchCheck.cpp | 121 ++++++++++++++++++ .../bugprone/SmartPtrArrayMismatchCheck.h | 52 ++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../bugprone-shared-ptr-array-mismatch.rst | 31 +++++ .../docs/clang-tidy/checks/list.rst | 1 + .../bugprone-shared-ptr-array-mismatch.cpp | 95 ++++++++++++++ 10 files changed, 378 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h create mode 100644 clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 82a807eaa1714..7b1a2c14ed3c1 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -42,6 +42,7 @@ #include "PosixReturnCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" +#include "SharedPtrArrayMismatchCheck.h" #include "SignalHandlerCheck.h" #include "SignedCharMisuseCheck.h" #include "SizeofContainerCheck.h" @@ -143,6 +144,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-posix-return"); CheckFactories.registerCheck( "bugprone-reserved-identifier"); + CheckFactories.registerCheck( + "bugprone-shared-ptr-array-mismatch"); CheckFactories.registerCheck("bugprone-signal-handler"); CheckFactories.registerCheck( "bugprone-signed-char-misuse"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index d1dbaec8358ad..b33072ddf01b8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -37,6 +37,8 @@ add_clang_library(clangTidyBugproneModule PosixReturnCheck.cpp RedundantBranchConditionCheck.cpp ReservedIdentifierCheck.cpp + SharedPtrArrayMismatchCheck.cpp + SmartPtrArrayMismatchCheck.cpp SignalHandlerCheck.cpp SignedCharMisuseCheck.cpp SizeofContainerCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.cpp new file mode 100644 index 0000000000000..b628ebd4ae784 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.cpp @@ -0,0 +1,31 @@ +//===--- SharedPtrArrayMismatchCheck.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 "SharedPtrArrayMismatchCheck.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +SharedPtrArrayMismatchCheck::SharedPtrArrayMismatchCheck( + StringRef Name, ClangTidyContext *Context) + : SmartPtrArrayMismatchCheck(Name, Context, "shared") {} + +SharedPtrArrayMismatchCheck::SmartPtrClassMatcher +SharedPtrArrayMismatchCheck::getSmartPointerClassMatcher() const { + return classTemplateSpecializationDecl( + hasName("::std::shared_ptr"), templateArgumentCountIs(1), + hasTemplateArgument( + 0, templateArgument(refersToType(qualType().bind(PointerTypeN))))); +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h b/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h new file mode 100644 index 0000000000000..f628e02962d27 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h @@ -0,0 +1,38 @@ +//===--- SharedPtrArrayMismatchCheck.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_SHAREDPTRARRAYMISMATCHCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SHAREDPTRARRAYMISMATCHCHECK_H + +#include "SmartPtrArrayMismatchCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Find `std::shared_ptr(new T[...])`, replace it (if applicable) with +/// `std::shared_ptr(new T[...])`. +/// +/// Example: +/// +/// \code +/// std::shared_ptr PtrArr{new int[10]}; +/// \endcode +class SharedPtrArrayMismatchCheck : public SmartPtrArrayMismatchCheck { +public: + SharedPtrArrayMismatchCheck(StringRef Name, ClangTidyContext *Context); + +protected: + virtual SmartPtrClassMatcher getSmartPointerClassMatcher() const; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SHAREDPTRARRAYMISMATCHCHECK_H diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp new file mode 100644 index 0000000000000..a706f6ce36b6e --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp @@ -0,0 +1,121 @@ +//===--- SmartPtrArrayMismatchCheck.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 "SmartPtrArrayMismatchCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +namespace { + +constexpr char ConstructExprN[] = "found_construct_expr"; +constexpr char NewExprN[] = "found_new_expr"; +constexpr char ConstructorN[] = "found_constructor"; + +bool isInSingleDeclStmt(const DeclaratorDecl *D) { + const DynTypedNodeList Parents = + D->getASTContext().getParentMapContext().getParents(*D); + for (const DynTypedNode &PNode : Parents) + if (const auto *PDecl = PNode.get()) + return PDecl->isSingleDecl(); + return false; +} + +const DeclaratorDecl *getConstructedVarOrField(const Expr *FoundConstructExpr, + ASTContext &Ctx) { + const DynTypedNodeList ConstructParents = + Ctx.getParentMapContext().getParents(*FoundConstructExpr); + if (ConstructParents.size() != 1) + return nullptr; + const auto *ParentDecl = ConstructParents.begin()->get(); + if (isa_and_nonnull(ParentDecl)) + return ParentDecl; + + return nullptr; +} + +} // namespace + +const char SmartPtrArrayMismatchCheck::PointerTypeN[] = "pointer_type"; + +SmartPtrArrayMismatchCheck::SmartPtrArrayMismatchCheck( + StringRef Name, ClangTidyContext *Context, StringRef SmartPointerName) + : ClangTidyCheck(Name, Context), SmartPointerName(SmartPointerName) {} + +void SmartPtrArrayMismatchCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) {} + +void SmartPtrArrayMismatchCheck::registerMatchers(MatchFinder *Finder) { + // For both shared and unique pointers, we need to find constructor with + // exactly one parameter that has the pointer type. Other constructors are + // not applicable for this check. + auto FindConstructor = + cxxConstructorDecl(ofClass(getSmartPointerClassMatcher()), + parameterCountIs(1), isExplicit()) + .bind(ConstructorN); + auto FindConstructExpr = + cxxConstructExpr( + hasDeclaration(FindConstructor), argumentCountIs(1), + hasArgument( + 0, cxxNewExpr(isArray(), hasType(pointerType(pointee( + equalsBoundNode(PointerTypeN))))) + .bind(NewExprN))) + .bind(ConstructExprN); + Finder->addMatcher(FindConstructExpr, this); +} + +void SmartPtrArrayMismatchCheck::check(const MatchFinder::MatchResult &Result) { + const auto *FoundNewExpr = Result.Nodes.getNodeAs(NewExprN); + const auto *FoundConstructExpr = + Result.Nodes.getNodeAs(ConstructExprN); + const auto *FoundConstructorDecl = + Result.Nodes.getNodeAs(ConstructorN); + + ASTContext &Ctx = FoundConstructorDecl->getASTContext(); + const DeclaratorDecl *VarOrField = + getConstructedVarOrField(FoundConstructExpr, Ctx); + + auto D = diag(FoundNewExpr->getBeginLoc(), + "%0 pointer to non-array is initialized with array") + << SmartPointerName; + D << FoundNewExpr->getSourceRange(); + + if (VarOrField) { + auto TSTypeLoc = VarOrField->getTypeSourceInfo() + ->getTypeLoc() + .getAsAdjusted(); + assert(TSTypeLoc.getNumArgs() >= 1 && + "Matched type should have at least 1 template argument."); + + SourceRange TemplateArgumentRange = TSTypeLoc.getArgLoc(0) + .getTypeSourceInfo() + ->getTypeLoc() + .getLocalSourceRange(); + D << TemplateArgumentRange; + + if (isInSingleDeclStmt(VarOrField)) { + const SourceManager &SM = Ctx.getSourceManager(); + if (!utils::rangeCanBeFixed(TemplateArgumentRange, &SM)) + return; + + SourceLocation InsertLoc = Lexer::getLocForEndOfToken( + TemplateArgumentRange.getEnd(), 0, SM, Ctx.getLangOpts()); + D << FixItHint::CreateInsertion(InsertLoc, "[]"); + } + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.h new file mode 100644 index 0000000000000..74b46eb7c216f --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.h @@ -0,0 +1,52 @@ +//===--- SharedPtrArrayMismatchCheck.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_SMARTPTRARRAYMISMATCHCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRARRAYMISMATCHCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Find constructions of smart (unique or shared) pointers where the pointer +/// is declared with non-array target type and an array (created with a +/// new-expression) is passed to it. +class SmartPtrArrayMismatchCheck : public ClangTidyCheck { +public: + SmartPtrArrayMismatchCheck(StringRef Name, ClangTidyContext *Context, + StringRef SmartPointerName); + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +protected: + using SmartPtrClassMatcher = ast_matchers::internal::BindableMatcher; + + /// Returns matcher that match with different smart pointer classes. + /// + /// Requires to bind pointer type (qualType) with PointerTypeN string declared + /// in this class. + virtual SmartPtrClassMatcher getSmartPointerClassMatcher() const = 0; + + static const char PointerTypeN[]; + +private: + StringRef const SmartPointerName; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRARRAYMISMATCHCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2f2be2d1ac1dc..9179cb745791f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -99,6 +99,10 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-shared-ptr-array-mismatch ` check. + + Finds initializations of C++ shared pointers to non-array type that are initialized with an array. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst new file mode 100644 index 0000000000000..d15c28272d697 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst @@ -0,0 +1,31 @@ +.. title:: clang-tidy - bugprone-shared-ptr-array-mismatch + +bugprone-shared-ptr-array-mismatch +================================== + +Finds initializations of C++ shared pointers to non-array type that are +initialized with an array. + +If a shared pointer ``std::shared_ptr`` is initialized with a new-expression +``new T[]`` the memory is not deallocated correctly. The pointer uses plain +``delete`` in this case to deallocate the target memory. Instead a ``delete[]`` +call is needed. A ``std::shared_ptr`` calls the correct delete operator. + +The check offers replacement of ``shared_ptr`` to ``shared_ptr`` if it +is used at a single variable declaration (one variable in one statement). + +Example: + +.. code-block:: c++ + + std::shared_ptr x(new Foo[10]); // -> std::shared_ptr x(new Foo[10]); + // ^ warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + std::shared_ptr x1(new Foo), x2(new Foo[10]); // no replacement + // ^ warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + + std::shared_ptr x3(new Foo[10], [](const Foo *ptr) { delete[] ptr; }); // no warning + + struct S { + std::shared_ptr x(new Foo[10]); // no replacement in this case + // ^ warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index fcf661a406959..a67de1c4788f9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -84,6 +84,7 @@ Clang-Tidy Checks `bugprone-posix-return `_, "Yes" `bugprone-redundant-branch-condition `_, "Yes" `bugprone-reserved-identifier `_, "Yes" + `bugprone-shared-ptr-array-mismatch `_, "Yes" `bugprone-signal-handler `_, `bugprone-signed-char-misuse `_, `bugprone-sizeof-container `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp new file mode 100644 index 0000000000000..a232b854530c5 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp @@ -0,0 +1,95 @@ +// RUN: %check_clang_tidy %s bugprone-shared-ptr-array-mismatch %t + +namespace std { + +template +struct shared_ptr { + template + explicit shared_ptr(Y *) {} + template + shared_ptr(Y *, Deleter) {} +}; + +} // namespace std + +struct A {}; + +void f1() { + std::shared_ptr P1{new int}; + std::shared_ptr P2{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr P2{new int[10]}; + // clang-format off + std::shared_ptr< int > P3{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr< int[] > P3{new int[10]}; + // clang-format on + std::shared_ptr P4(new int[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr P4(new int[10]); + new std::shared_ptr(new int[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + std::shared_ptr P5(new int[10]); + std::shared_ptr P6(new int[10], [](const int *Ptr) {}); +} + +void f2() { + std::shared_ptr P1(new A); + std::shared_ptr P2(new A[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr P2(new A[10]); + std::shared_ptr P3(new A[10]); +} + +void f3() { + std::shared_ptr P1{new int}, P2{new int[10]}, P3{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + // CHECK-MESSAGES: :[[@LINE-2]]:57: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] +} + +struct S { + std::shared_ptr P1; + std::shared_ptr P2{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + std::shared_ptr P3{new int}, P4{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + S() : P1{new int[10]} {} + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] +}; + +void f_parm(std::shared_ptr); + +void f4() { + f_parm(std::shared_ptr{new int[10]}); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] +} + +std::shared_ptr f_ret() { + return std::shared_ptr(new int[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] +} + +template +void f_tmpl() { + std::shared_ptr P1{new T[10]}; +} + +void f5() { + f_tmpl(); +} + +#define CHAR_PTR_TYPE std::shared_ptr +#define CHAR_PTR_VAR(X) \ + X { new char[10] } +#define CHAR_PTR_INIT(X, Y) \ + std::shared_ptr X { Y } + +void f6() { + CHAR_PTR_TYPE P1{new char[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + std::shared_ptr CHAR_PTR_VAR(P2); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr CHAR_PTR_VAR(P2); + CHAR_PTR_INIT(P3, new char[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch] +}